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

Implements relative position scrolling #3

Closed
wants to merge 2 commits into from

Conversation

TheOtterlord
Copy link

E.g. The first group of tabs on a page have tabs of different sizes. When a user clicks a second set of tabs, this will anchor their viewpoint based on the set of tabs they interacted with.

@HiDeoo
Copy link
Owner

HiDeoo commented Dec 14, 2023

I did not have the time yet to review the code altho I did try it out locally. It looks like this is mostly working fine altho I noticed a few issues.

In case this is relevant, I used the getting started page to test this out which I slightly modified with this content.

  • When switching tabs, it seems the entire page can slightly shift.
shift.mp4
  • When at the bottom of the page, switching tabs using the mouse will always work but using the keyboard will only work once, after that it will not work anymore.
bug.mp4

I wonder if we should also add tests for this?

@TheOtterlord
Copy link
Author

It looks like the bounding height of elements changes when the tab switches. I'm not sure what the cause of this is, but I'll take a look 🤔

I'm not able to replicate the second bug. Are you using any specific browser?

+1 on adding tests

@HiDeoo
Copy link
Owner

HiDeoo commented Dec 14, 2023

I'm not able to replicate the second bug. Are you using any specific browser?

Basic Chrome, but I can investigate when I get a bit more time to see if it's in all browsers and see if I can find the issue 👍

@TheOtterlord
Copy link
Author

Commit above solves repetitive page shifting, preventing the page from scrolling over many tab interactions, but it does still show a shift either way. It comes down to the bounding box of content components changing, and I'm not entirely sure why they do.

@HiDeoo
Copy link
Owner

HiDeoo commented Dec 14, 2023

I pushed in https://github.com/HiDeoo/starlight/pull/4/files how I would have naively implemented the feature. It still has the same issue as I have no idea yet how to work around it, altho I am wondering if your approach handles some cases that mine doesn't or that I may not have thought of or if we could just use a simpler approach like I did?

@TheOtterlord
Copy link
Author

did some testing, and looks like it still handles everything I've tested it with, and it saves so much hassle in implementation! I guess we can either move conversation over to #4 or merge it and deal with everything in the larger PR. I've still found no lead on the shifting, either why it happens or how we can fix it 🤔 the weird thing is, it's not the scroll behaviour specifically, but just what happens when a tab height is changed

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

Successfully merging this pull request may close these issues.

2 participants