-
-
Notifications
You must be signed in to change notification settings - Fork 252
feat: improve scroll-to-top behavior #1453
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -6,41 +6,44 @@ const excludedRoutes = new Set(['index', 'code']) | |||||
|
|
||||||
| const isActive = computed(() => !excludedRoutes.has(route.name as string)) | ||||||
|
|
||||||
| const SCROLL_TO_TOP_DURATION = 500 | ||||||
|
|
||||||
| const isMounted = useMounted() | ||||||
| const isTouchDeviceClient = shallowRef(false) | ||||||
| const isVisible = shallowRef(false) | ||||||
| const scrollThreshold = 300 | ||||||
| const { isSupported: supportsScrollStateQueries } = useCssSupports( | ||||||
| 'container-type', | ||||||
| 'scroll-state', | ||||||
| { ssrValue: false }, | ||||||
| ) | ||||||
| const shouldShowButton = computed(() => isActive.value && isTouchDeviceClient.value) | ||||||
|
|
||||||
| function onScroll() { | ||||||
| if (!supportsScrollStateQueries.value) { | ||||||
| if (supportsScrollStateQueries.value) { | ||||||
| return | ||||||
| } | ||||||
| isVisible.value = window.scrollY > scrollThreshold | ||||||
| } | ||||||
|
|
||||||
| function scrollToTop() { | ||||||
| window.scrollTo({ top: 0, behavior: 'smooth' }) | ||||||
| } | ||||||
| const { scrollToTop } = useScrollToTop({ duration: SCROLL_TO_TOP_DURATION }) | ||||||
|
|
||||||
| useEventListener('scroll', onScroll, { passive: true }) | ||||||
|
|
||||||
| onMounted(() => { | ||||||
| isTouchDeviceClient.value = isTouchDevice() | ||||||
| onScroll() | ||||||
| }) | ||||||
| </script> | ||||||
|
|
||||||
| <template> | ||||||
| <!-- When CSS scroll-state is supported, use CSS-only visibility --> | ||||||
| <button | ||||||
| v-if="isActive && supportsScrollStateQueries" | ||||||
| v-if="shouldShowButton && supportsScrollStateQueries" | ||||||
| type="button" | ||||||
| class="scroll-to-top-css fixed bottom-4 inset-ie-4 z-50 w-12 h-12 bg-bg-elevated border border-border rounded-full shadow-lg md:hidden flex items-center justify-center text-fg-muted hover:text-fg transition-colors active:scale-95" | ||||||
| class="scroll-to-top-css fixed bottom-4 inset-ie-4 z-50 w-12 h-12 bg-bg-elevated border border-border rounded-full shadow-lg flex items-center justify-center text-fg-muted hover:text-fg transition-colors active:scale-95" | ||||||
| :aria-label="$t('common.scroll_to_top')" | ||||||
| @click="scrollToTop" | ||||||
| @click="scrollToTop()" | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| > | ||||||
| <span class="i-lucide:arrow-up w-5 h-5" aria-hidden="true" /> | ||||||
| </button> | ||||||
|
|
@@ -56,11 +59,11 @@ onMounted(() => { | |||||
| leave-to-class="opacity-0 translate-y-2" | ||||||
| > | ||||||
| <button | ||||||
| v-if="isActive && isMounted && isVisible" | ||||||
| v-if="shouldShowButton && isMounted && isVisible" | ||||||
| type="button" | ||||||
| class="fixed bottom-4 inset-ie-4 z-50 w-12 h-12 bg-bg-elevated border border-border rounded-full shadow-lg md:hidden flex items-center justify-center text-fg-muted hover:text-fg transition-colors active:scale-95" | ||||||
| class="fixed bottom-4 inset-ie-4 z-50 w-12 h-12 bg-bg-elevated border border-border rounded-full shadow-lg flex items-center justify-center text-fg-muted hover:text-fg transition-colors active:scale-95" | ||||||
| :aria-label="$t('common.scroll_to_top')" | ||||||
| @click="scrollToTop" | ||||||
| @click="scrollToTop()" | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| > | ||||||
| <span class="i-lucide:arrow-up w-5 h-5" aria-hidden="true" /> | ||||||
| </button> | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,96 @@ | ||
| interface UseScrollToTopOptions { | ||
| /** | ||
| * Duration of the scroll animation in milliseconds. | ||
| */ | ||
| duration?: number | ||
| } | ||
|
|
||
| /** | ||
| * Scroll to the top of the page with a smooth animation. | ||
| * @param options - Configuration options for the scroll animation. | ||
| * @returns An object containing the scrollToTop function and a cancel function. | ||
| */ | ||
| export function useScrollToTop(options: UseScrollToTopOptions) { | ||
| const { duration = 500 } = options | ||
|
|
||
| // Check if prefers-reduced-motion is enabled | ||
| const preferReducedMotion = useMediaQuery('(prefers-reduced-motion: reduce)') | ||
|
|
||
| // Easing function for the scroll animation | ||
| const easeOutQuad = (t: number) => t * (2 - t) | ||
|
|
||
| /** | ||
| * Active requestAnimationFrame id for the current auto-scroll animation | ||
| */ | ||
| let rafId: number | null = null | ||
| /** | ||
| * Disposer for temporary interaction listeners attached during auto-scroll | ||
| */ | ||
| let stopInteractionListeners: (() => void) | null = null | ||
|
|
||
| function cleanupInteractionListeners() { | ||
| if (stopInteractionListeners) { | ||
| stopInteractionListeners() | ||
| stopInteractionListeners = null | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Stop any in-flight auto-scroll before starting a new one. | ||
| */ | ||
| function cancel() { | ||
| if (rafId !== null) { | ||
| cancelAnimationFrame(rafId) | ||
| rafId = null | ||
| } | ||
|
|
||
| cleanupInteractionListeners() | ||
| } | ||
|
|
||
| function scrollToTop() { | ||
| cancel() | ||
|
|
||
| if (preferReducedMotion.value) { | ||
| window.scrollTo({ top: 0, behavior: 'instant' }) | ||
| return | ||
| } | ||
|
|
||
| const start = window.scrollY | ||
| if (start <= 0) return | ||
|
|
||
| const startTime = performance.now() | ||
| const change = -start | ||
|
|
||
| const cleanup = [ | ||
| useEventListener(window, 'wheel', cancel, { passive: true }), | ||
| useEventListener(window, 'touchstart', cancel, { passive: true }), | ||
| useEventListener(window, 'mousedown', cancel, { passive: true }), | ||
| ] | ||
|
Comment on lines
+64
to
+68
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we should not add these inside of |
||
|
|
||
| stopInteractionListeners = () => cleanup.forEach(stop => stop()) | ||
|
|
||
| // Start the frame-by-frame scroll animation. | ||
| function animate() { | ||
| const elapsed = performance.now() - startTime | ||
| const t = Math.min(elapsed / duration, 1) | ||
| const y = start + change * easeOutQuad(t) | ||
|
|
||
| window.scrollTo({ top: y }) | ||
|
|
||
| if (t < 1) { | ||
| rafId = requestAnimationFrame(animate) | ||
| } else { | ||
| cancel() | ||
| } | ||
| } | ||
|
|
||
| rafId = requestAnimationFrame(animate) | ||
| } | ||
|
|
||
| onBeforeUnmount(cancel) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should prefer the |
||
|
|
||
| return { | ||
| scrollToTop, | ||
| cancel, | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Desktop visibility still blocked by touch-only gating.
shouldShowButtonrequiresisTouchDeviceClient, so the button never appears on non-touch desktops. That conflicts with issue#1379’s requirement to show the control on desktop. If the requirement still stands, drop the touch-only gate (or make it configurable). If the requirement has changed, please update the issue/PR description to match.🛠️ Suggested fix if desktop should be supported
Also applies to: 34-34
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is ok, requirements changed to only show the button on touch decices
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@graphierosThank you for clarifying! I understand the requirements have shifted to show the button only on touch devices. The current implementation correctly achieves that.Since issue
#1379is titled "Back-to-top button on desktop" and the PR description mentions "Desktop Support: Enabled the button on desktop", you may want to update both to reflect this new direction—perhaps explaining that after discussion, the button was decided to remain touch-only to avoid cluttering desktop UI (where keyboard shortcuts like Home/End are available).✏️ Learnings added