Skip to content

Adopt Vue composables for stateful non-rendering logic, starting with TemplateSection's scroll-arrows #134

Description

@moodyjmz

TL;DR: This app has no composables yet — stateful logic that doesn't render anything (dialog state, view-mode persistence, scroll-arrow tracking) currently lives inline in <script setup> blocks alongside markup-driving state. Two PRs, not one: a proof-of-concept extraction first, then — once that's proven out and the conventions hold up in practice — a second PR that deliberately extends the same model to the rest of the identified clusters. Not a single big-bang PR covering everything at once, and not left to happen opportunistically one at a time either.

Detail

Why now

Found while investigating a recurring test-coverage gap across #126/#127/#130 (tracked separately in #133): the same file keeps absorbing new interactive elements and derived state without ever being split up. Composables are the piece "extract it" doesn't cover on its own — component extraction splits render output, composables split stateful logic with no direct template dependency. This project is small (5 Vue files, ~1000 lines total) and young, so establishing the convention now is cheap; it gets more expensive the more code lands on the current shape.

PR 1 — proof of concept: TemplateSection.vue's scroll-arrows cluster

list (element ref) + canScrollLeft/canScrollRight + updateArrows() + scrollByStep() + the onMounted/onUnmounted wiring is a self-contained "scrollable region with arrow buttons" state machine — reusable, no dependency on other in-flight work, and the same region already has an open a11y gap (#59: no aria-label identifying it as a scrollable region) worth closing while in there.

Check @vueuse/core before hand-rolling this one. Verified directly against its shipped type declarations: useScroll(element) already returns arrivedState.{left,right,top,bottom} (the inverse of canScrollLeft/canScrollRight) plus reactive x/y, and handles the listener attach/detach and resize-observation internally. The extraction should be a thin wrapper over useScroll, not new hand-rolled logic — this is the same "reuse before you write" ladder AGENTS.md already applies to @nextcloud/* packages and @nextcloud/vue components, extended to general-purpose Vue utilities. @vueuse/core@14.4.0's only peer dependency is vue@^3.5.0, which this app already satisfies.

Conventions to establish with this PR, not invent per-composable later:

  1. Check @vueuse/core first. Before hand-rolling a composable, check whether it already exists there (as with useScroll above). Prefer it the same way an existing @nextcloud/vue component or design token is preferred over a hand-rolled equivalent.
  2. Return individual refs, not a reactive() object. A composable that returns reactive({...}) breaks reactivity silently if the caller destructures it. Return individual refs (or toRefs() on an internal reactive object) so destructuring is always safe.
  3. A shared test helper for composables using lifecycle hooks. onMounted/onUnmounted throw outside an active component instance, so testing needs a small reusable helper — a trivial host component whose setup() just returns the composable — living in src/test-utils/, written once here rather than reinvented slightly differently by the second and third composable.
  4. Same new-unit-plus-sibling-spec rule as src/utils//src/components/ today. src/composables/useScrollArrows.ts lands with src/composables/useScrollArrows.spec.ts in the same commit — this is also where the src/composables/ directory convention itself gets established.

PR 2 — once PR 1 is proven, extend deliberately to the rest

Not opportunistic, not "whoever next touches that region" — a committed second PR, once PR 1's shape has held up in review and in practice. Targets:

  • OfficeOverview.vue: the create-from-template dialog cluster (showCreateDialog, newFileName, pendingCreator, pendingTemplate, creating, createError, createInput)
  • OfficeOverview.vue: the view-mode toggle + persistence cluster (viewMode, toggleViewMode(), getOverviewGridView/setOverviewGridView)
  • OfficeOverview.vue: the active-creator/routing-sync cluster landing via feat: add navigation actions for office #130 (activeCreator, routeCreatorId, the URL-sync watch)

Each of these is its own commit within PR 2, one concern per commit, same as always — but landing as one deliberate pass rather than trickling in over unrelated future feature work.

After both PRs: documenting the pattern

Once PR 2 lands and the model has held up across all four clusters — not before — #139 writes it into AGENTS.md as an established pattern. Kept as a separate issue rather than folded into this one, so this issue stays scoped to the code work.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions