Skip to content
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

feat: scroll active sidebar link into view on page load #3654

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

BobbieGoede
Copy link

I find myself searching in the sidebar in the VueUse docs for related composables, the current page link is often out of view in the sidebar.

Not too happy with my solution in VPSidebar.vue, I guess this won't scroll the relevant link into view when navigating using in page links, also not sure if querySelector is the appropriate way to find the relevant link. Should a router hook be used instead?

I can add tests to confirm the changes work as expected, let me know if changes should be made!

@brc-dd brc-dd self-assigned this Apr 26, 2024

// scroll active link into view
onMounted(() => {
nextTick(() => navEl.value?.querySelector('.is-active')?.scrollIntoView({ block: 'center'}))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And if the same route or pattern is placed 2+ times in the sidebar, is it working fine? also I'd calculate to scroll only when it is out of the viewport

@@ -197,6 +197,7 @@ export function useActiveAnchor(

if (activeLink) {
activeLink.classList.add('active')
activeLink.scrollIntoView({ behavior: 'smooth', block: 'center' })

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR preview
image

Fixed
image

we also need to fix some edge cases like scroll to 0 when there is no active link, I'd also scroll only when it is out of viewport

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Keep aside marker visible
3 participants