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(Tabs): keep selected tab in view #7045

Conversation

emyarod
Copy link
Member

@emyarod emyarod commented Oct 14, 2020

Closes #6917

This PR makes sure the selected tab in the component state is visible (component scrolls to the selected tab) when the component updates

Changelog

Changed

  • scroll selected tab into view on component update

Testing / Reviewing

Change the selected tab and confirm that the tab is in view on initial render in addition to subsequent renders

@netlify
Copy link

netlify bot commented Oct 14, 2020

Deploy preview for carbon-elements ready!

Built with commit 2011d31

https://deploy-preview-7045--carbon-elements.netlify.app

@netlify
Copy link

netlify bot commented Oct 14, 2020

Deploy preview for carbon-components-react ready!

Built without sensitive environment variables with commit 2011d31

https://deploy-preview-7045--carbon-components-react.netlify.app

@joshblack
Copy link
Contributor

@emyarod random question, is there a chance that this cDU call could be invoked if Tabs is not in the viewport? If so, would this change cause the page to scroll the selected tab into view (in the y direction)?

@emyarod
Copy link
Member Author

emyarod commented Oct 14, 2020

no it shouldn't be scrolled in the y direction (related #6974)

edit: I am assuming you are not referring to the default behavior where changing focus also scrolls the focused element into view

@tw15egan
Copy link
Collaborator

Seeing an issue where you are not able to click to scroll to other tabs since it seems like it wants to keep the focus on the first tab

focus-click

@emyarod
Copy link
Member Author

emyarod commented Oct 19, 2020

@tw15egan I'm unable to replicate that issue. Can you provide more details about how you're getting that to occur?

@dakahn
Copy link
Contributor

dakahn commented Oct 19, 2020

Seeing the same bug as @tw15egan on Firefox latest Windows 10

@emyarod emyarod force-pushed the 6917-scroll-to-selected-tab-on-mount branch from 18103e2 to cd0702d Compare October 21, 2020 16:48
@emyarod
Copy link
Member Author

emyarod commented Oct 21, 2020

can you take another look with the latest PR changes?

@emyarod emyarod force-pushed the 6917-scroll-to-selected-tab-on-mount branch 2 times, most recently from a093c0c to 15b43b8 Compare October 21, 2020 17:34
@emyarod emyarod force-pushed the 6917-scroll-to-selected-tab-on-mount branch from 15b43b8 to f5b1cec Compare October 21, 2020 18:32
@joshblack
Copy link
Contributor

bump @dakahn based on comment above

@dakahn
Copy link
Contributor

dakahn commented Oct 22, 2020

Scrolling works swell! 👍

@kodiakhq kodiakhq bot merged commit d27bcf1 into carbon-design-system:master Oct 22, 2020
@emyarod emyarod deleted the 6917-scroll-to-selected-tab-on-mount branch October 23, 2020 15:02
emyarod added a commit to emyarod/carbon that referenced this pull request Oct 23, 2020
kodiakhq bot added a commit that referenced this pull request Oct 23, 2020
* fix(Tab): update role

* fix(Tab): place aria attributes on inner anchor instead of li

* test(Tabs): update tab role test

* fix(Tab): convert anchor to button

* test(Tabs): update inner anchor tests

* chore: update snapshots

* test(Tab): update test descriptions

* test(Tab): remove obsolete test

* fix(Tab): remove tabIndex from wrapper li

* fix(Tab): add role to button

* fix(Tab): deprecate required href prop

* fix(TabContent): remove `aria-live` and `aria-hidden`

* chore: update snapshots

* test(Tab): update test description

* chore(Tab): deprecate tabIndex prop

* docs(Tabs): add tabpanel role to custom tab content wrapper

* chore: update snapshots

* fix(Tabs): activate scroll buttons with LMB only

* fix(Tabs): prevent keyboard focus of scroll buttons

* fix(Tabs): prevent ul from being focusable

* refactor(Tabs): carry over changes from #7045

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tabs component: need ability to scroll to the selected tab
4 participants