feat(templates): extract scroll-arrows state into useScrollArrows - #145
Open
moodyjmz wants to merge 2 commits into
Open
feat(templates): extract scroll-arrows state into useScrollArrows#145moodyjmz wants to merge 2 commits into
moodyjmz wants to merge 2 commits into
Conversation
Already present transitively via @nextcloud/vue, but the app is about to import from it directly (useScrollArrows), so it needs to be declared rather than relied on implicitly. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
TemplateSection.vue's list ref, canScrollLeft/canScrollRight, updateArrows(), scrollByStep(), and the ResizeObserver onMounted/ onUnmounted wiring were stateful logic with no direct template dependency, inline in the component. Pulled into a composable built on @vueuse/core's useScroll/useResizeObserver instead of hand-rolled scroll tracking, following the reuse-before-you-write ladder. Establishes src/composables/ and its conventions for later extractions: individual refs returned (not a reactive() object, which breaks destructuring), and a shared withSetup() test helper in src/test-utils/ for composables that rely on onMounted/onUnmounted. Also adds an aria-label identifying the scrollable region, closing the a11y gap noted in #134 while touching this code (mis-cited there as #59 — that's OfficeOverview's favourite-indicator issue instead; #134 should be corrected separately). PR 1 of #134. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PR 1 of #134 — proof of concept for adopting Vue composables as the layer between
src/services/(data) andsrc/components/(render): stateful logic with no direct template dependency.TemplateSection.vue's scroll-arrow state (listref,canScrollLeft/canScrollRight,updateArrows(),scrollByStep(), theResizeObservermount/unmount wiring) intosrc/composables/useScrollArrows.ts, built on@vueuse/core'suseScroll/useResizeObserverrather than hand-rolled — reuse-before-you-write, extended from@nextcloud/*packages to general-purpose Vue utilities.@vueuse/coreas a direct dependency (previously only present transitively via@nextcloud/vue; both now resolve to the same 14.3.0, no duplicate copy in the tree).src/composables/and its conventions for PR 2 and beyond: return individual refs (not areactive()object — breaks silently on destructuring), and a sharedsrc/test-utils/withSetup.tshelper for composables that rely ononMounted/onUnmounted(Vue's lifecycle hooks throw outside a real component instance).aria-labelidentifying it as a scrollable region, per creator.Note: #134 cites this gap as issue #59 — that's actually OfficeOverview's favourite-indicator issue, an unrelated a11y gap. Will correct the citation on #134 separately; not blocking this PR.
Reuse before you write
@vueuse/core'suseScroll/useResizeObserver— see commit message and #134 for the ladder rationale.Test plan
npm run lintnpm run stylelintnpm run test:unit(129 passing, includes 8 new composable tests + a new a11y test)npm run build/compileneeded once merged-ready — will note in a follow-up commentRefs #134.
🤖 Generated with Claude Code