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: Tabs scroll snapping #3143

Merged
merged 4 commits into from
Dec 18, 2024
Merged

feat: Tabs scroll snapping #3143

merged 4 commits into from
Dec 18, 2024

Conversation

georgylobko
Copy link
Contributor

Description

Reintroduced tabs scroll snapping reverted in this commit.

The main issue with the previous implementation was that the custom scrollIntoView could scroll in a way that pushed the active element out of view.

This PR adjusts the scrollIntoView logic to always center the active element in the container, which should fix the problem.

Related links, issue #, if available: n/a

How has this been tested?

Review checklist

The following items are to be evaluated by the author(s) and the reviewer(s).

Correctness

  • Changes include appropriate documentation updates.
  • Changes are backward-compatible if not indicated, see CONTRIBUTING.md.
  • Changes do not include unsupported browser features, see CONTRIBUTING.md.
  • Changes were manually tested for accessibility, see accessibility guidelines.

Security

Testing

  • Changes are covered with new/existing unit tests?
  • Changes are covered with new/existing integration tests?

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Copy link

codecov bot commented Dec 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.40%. Comparing base (3bf2183) to head (5cc60fa).
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3143   +/-   ##
=======================================
  Coverage   96.40%   96.40%           
=======================================
  Files         784      784           
  Lines       22130    22130           
  Branches     7528     7586   +58     
=======================================
  Hits        21335    21335           
- Misses        743      788   +45     
+ Partials       52        7   -45     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…dth to make its center position to be aligned with a center of the container
@georgylobko georgylobko marked this pull request as ready for review December 17, 2024 10:11
@georgylobko georgylobko requested a review from a team as a code owner December 17, 2024 10:11
@georgylobko georgylobko requested review from orangevolon and gethinwebster and removed request for a team December 17, 2024 10:11
@@ -223,13 +223,25 @@ test(
await page.setWindowSize({ width: 500, height: 1000 });
await page.click('#add-tab');
await page.click(page.paginationButton('right', true));
await page.click(page.paginationButton('right', true));
Copy link
Member

Choose a reason for hiding this comment

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

Why is it necessary to click once more in the right arrow now? Actually, it looks like the component scrolls a bit further now than before, making the seventh tab visible with one single click whereas before it wasn't.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The component actually scrolls slightly less than before, which is why this condition is necessary

@georgylobko georgylobko requested a review from jperals December 18, 2024 09:54
@georgylobko georgylobko added this pull request to the merge queue Dec 18, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Dec 18, 2024
@georgylobko georgylobko added this pull request to the merge queue Dec 18, 2024
Merged via the queue into main with commit 6310aca Dec 18, 2024
38 checks passed
@georgylobko georgylobko deleted the feat/tabs-scroll-snapping-v3 branch December 18, 2024 11:42
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.

3 participants