feat(responsive-tabs): allow scroll on components with tabs #4067
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…eat/responsive-tabs
|
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
🦋 Changeset detectedLatest commit: ecdbdd4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
dosubot
bot
added
the
size:XL
github-actions
bot
added
the
Status: Do Not Merge
github-actions
bot
added
Area: Components
☁️ Nx Cloud ReportCI is running/has finished running commands for commit ecdbdd4. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 1 targetSent with 💌 from NxCloud. |
|
Size Change: +4.3 kB (0%) Total Size: 1.1 MB
ℹ️ View Unchanged
|
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit ecdbdd4:
|
Paste
|
||||||||||||||||||||||||
| Project |
Paste
|
| Run status |
|
| Run duration | 02m 07s |
| Commit |
|
| Committer | Kristian Antrobus |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
67
|
serifluous
reviewed
Oct 14, 2024
packages/paste-core/components/in-page-navigation/src/OverflowButton.tsx
Outdated
Show resolved
Hide resolved
serifluous
reviewed
Oct 14, 2024
packages/paste-core/components/code-block/src/OverflowButton.tsx
Outdated
Show resolved
Hide resolved
serifluous
reviewed
Oct 14, 2024
Co-authored-by: Sarah <[email protected]>
serifluous
previously approved these changes
Oct 15, 2024
nkrantz
approved these changes
Oct 16, 2024
serifluous
approved these changes
Oct 16, 2024
krisantrobus
added
🚀 merge it!
and removed
Status: Do Not Merge
Merged
| .getElementById(selectedId) | ||
| // eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
| // @ts-ignore - behavior is typed incorrectly in Typescript v4, fixed in v5+ | ||
| ?.scrollIntoView({ behavior: prevSelectedTab === undefined ? "instant" : "smooth" }); |
There was a problem hiding this comment.
Hi @krisantrobus,
This PR introduced some weird behavior since it applies to all Tabs variations including vertical ones. When used with vertical tabs scrollIntoView obviously scrolls both vertically and horizontally until a tab header is visible which causes an annoying experience when you need to scroll up to revert this unintended view change.
Could this be please fixed?
There was a problem hiding this comment.
Hey @dmitriy-kudelko apologies for this. Working on a fix and will update once we have a PR up
There was a problem hiding this comment.
PR is up and we are reviewing internally: #4143
There was a problem hiding this comment.
Hi @dmitriy-kudelko we have rolled out the fix for this and it is now available in version 20.18.0. I hope this resolves all issues you were seeing.
There was a problem hiding this comment.
@krisantrobus , great news. Thank you for implementing this feature and for quickly releasing a fix! 👍
We've been waiting for it and have already applied it to some places on the UI.
BTW, there seems to be also an issue with the inability to scroll tabs till the very end. As a result the last tab is partially cut. Clicking on the right arrow doesn't seem to help either. The left boundary doesn't have this problem. Not sure if it's something on our end, but we don't have any margin/padding customizations for the Tabs component which could explain that. It doesn't seem to depend on the window width either. I attached a video:
tabs.mov
There was a problem hiding this comment.
Hmm this is very strange and something we have not encountered. Could you share more details. What is the viewport width you are using and number of tabs. If you could replicate in a codesandbox that would be be very helpful. Also are you able to use the tab key to highlight a tab and use arrow keyboard buttons to get to the very last item?
There was a problem hiding this comment.
@dmitriy-kudelko I could not replicate it in our strorybooks when making some page compositions, however I did find it in a deploy preview for our Paste upgrade in one of our internal tools. I will be working on this again tomorrow and we should see another path. Again, so sorry for these issues
There was a problem hiding this comment.
Hi @dmitriy-kudelko I have validated the fix in 20.18.1 and I can confirm the issue has been resolved.
There was a problem hiding this comment.
Hi @krisantrobus , nice! Thank you so much!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added scroll functionality to components with tabs: