Adopt Vue composables for stateful non-rendering logic, starting with TemplateSection's scroll-arrows
Nobody has claimed this yet.
- Dominant language
- JavaScript
- Stars
- 6
- Forks
- 1
- Avg merge
- 1d 3h
- Merged PRs (30d)
- 10
Description
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:
- Check
@vueuse/corefirst. Before hand-rolling a composable, check whether it already exists there (as withuseScrollabove). Prefer it the same way an existing@nextcloud/vuecomponent or design token is preferred over a hand-rolled equivalent. - Return individual refs, not a
reactive()object. A composable that returnsreactive({...})breaks reactivity silently if the caller destructures it. Return individualrefs (ortoRefs()on an internal reactive object) so destructuring is always safe. - A shared test helper for composables using lifecycle hooks.
onMounted/onUnmountedthrow outside an active component instance, so testing needs a small reusable helper — a trivial host component whosesetup()just returns the composable — living insrc/test-utils/, written once here rather than reinvented slightly differently by the second and third composable. - Same new-unit-plus-sibling-spec rule as
src/utils//src/components/today.src/composables/useScrollArrows.tslands withsrc/composables/useScrollArrows.spec.tsin the same commit — this is also where thesrc/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 #130 (activeCreator,routeCreatorId, the URL-syncwatch)
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.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with TemplateSection.vue and check @vueuse/core's useScroll before extracting the scroll-arrows cluster. Add src/composables/useScrollArrows.ts, its sibling spec, and the lifecycle-hook test helper in src/test-utils/. The first PR is done when this proof of concept is tested and the accessibility gap is addressed; only then should the separate PR extend the pattern to the listed OfficeOverview.vue clusters.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript, typescript
- Domain
- frontend
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 58/100