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

fix(1-3375): Fix unintended scroll on dashboard #9316

Merged
merged 1 commit into from
Feb 17, 2025

Conversation

thomasheartman
Copy link
Contributor

Fixes a bug where the dashboard would scroll you down from the top of
the page on load if your window was too short too see both the
selected flag and the selected project.

This solves it by immediately scrolling to the top of the page after
scrolling your selected element into view. Because this hook only runs
on page load, it shouldn't be safe. (At least I couldn't make this
misbehave with manual testing).

It also changes the list scroll behavior to scroll your selected item
to the top of the list instead of to the bottom (effectively). During
testing, that seems like a better solution to me.

Background (or why do we auto-scroll here?)

The dashboard's flag and projects panels stores your last selection,
so that when you return to the page you'll be shown what you were
looking at last. This is especially useful if you have a lot of flags
but you're focusing on one in particular.

However, if you do have a lot of flags, then it's also quite
likely that your selection will be "below the fold" of the panel, and
you won't see your selected flag/project immediately in the
list (without scrolling).

It seemed like a nice UI affordance to automatically bring your
selected item into view (especially because without it, there's no way
to see what flag/project) you're looking at, so I added the
scrollIntoView hook.

What I didn't realize, however, is that it scrolls all scrollable
ancestor containers, which means that if your screen is too short,
it'll scroll you down the page.

From my reading of the docs and some local testing, I don't think
there is a way to limit the scrolling to only the nearest ancestor, so
the easiest way to ensure that we're always at the top seemed to be to
just scroll to the immediately after.

Fixes a bug where the dashboard would scroll you down from the top of
the page on load if your window was too short too see both the
selected flag and the selected project.

This solves it by immediately scrolling to the top of the page after
scrolling your selected element into view. Because this hook only runs
on page load, it shouldn't be safe. (At least I couldn't make this
misbehave with manual testing).

It also changes the list scroll behavior to scroll your selected item
to the top of the list instead of to the bottom (effectively). During
testing, that seems like a better solution to me.

## Background (or why do we auto-scroll here?)

The dashboard's flag and projects panels stores your last selection,
so that when you return to the page you'll be shown what you were
looking at last. This is especially useful if you have a lot of flags
but you're focusing on one in particular.

However, if you **do** have a lot of flags, then it's also quite
likely that your selection will be "below the fold" of the panel, and
you won't see your selected flag/project immediately in the
list (without scrolling).

It seemed like a nice UI affordance to automatically bring your
selected item into view (especially because without it, there's no way
to see what flag/project) you're looking at, so I added the
[`scrollIntoView`](https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollIntoView) hook.

What I didn't realize, however, is that it scrolls all scrollable
ancestor containers, which means that if your screen is too short,
it'll scroll you down the page.

From my reading of the docs and some local testing, I don't think
there is a way to limit the scrolling to only the nearest ancestor, so
the easiest way to ensure that we're always at the top seemed to be to
just scroll to the immediately after.
Copy link

vercel bot commented Feb 17, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
unleash-monorepo-frontend ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 17, 2025 10:49am
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
unleash-docs ⬜️ Ignored (Inspect) Visit Preview Feb 17, 2025 10:49am

Copy link
Contributor

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

OpenSSF Scorecard

PackageVersionScoreDetails

Scanned Files

Copy link
Contributor

@FredrikOseberg FredrikOseberg left a comment

Choose a reason for hiding this comment

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

LGTM

@thomasheartman thomasheartman merged commit 134c325 into main Feb 17, 2025
13 checks passed
@thomasheartman thomasheartman deleted the fix(1-3375)/unintended-scroll-on-dashboard branch February 17, 2025 11:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants