Control the selectedTabIndex from the consuming application #1654
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
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
MiniHeyd
commented
Sep 21, 2023
| // Set CSS custom properties for indicator | ||
| tabsParentElem.style.setProperty('--indicator-left-pos', tabLeftPos + 'px'); | ||
| tabsParentElem.style.setProperty('--indicator-width', tabWidth + 'px'); | ||
| next(() => { |
There was a problem hiding this comment.
using next here allows for the nested tabs to properly set the indicator bar
MiniHeyd
commented
Sep 21, 2023
| @@ -69,7 +80,6 @@ export default class HdsTabsIndexComponent extends Component { | |||
| @action | |||
| onClick(tabIndex, event) { | |||
| this.selectedTabIndex = tabIndex; | |||
| this.setTabIndicator(tabIndex); | |||
There was a problem hiding this comment.
This no longer needs to manually set since the component is watching for changes to this.selectedIndex via the did-update modifier and will fire this method when it changes.
MiniHeyd
commented
Sep 21, 2023
|
|
||
| schedule('afterRender', () => { | ||
| this.setTabIndicator(initialTabIndex); | ||
| this.setTabIndicator(); |
There was a problem hiding this comment.
did-update does not fire when the component is initially added so we need to fire this here to set the indicator on render.
|
Percy snapshot is good to go but I can not approve the build due to permissions |
… this runs. This helps to ensure that during a route transition we wait until the new tabs are ready (the previous runloop will have ended)
3 tasks
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.
📌 Summary
This PR will allow for the consuming application to manage the selected tab index state, in turn allowing for selected tab to be set after render by external controls.
🛠️ Detailed description
Currently the tab index is controlled by the
didInsertmethod and can be initially set to any of the tabs through use of the@isSelectedtab arg. Subsequently the tab can only be changed by controls internal to the component, limiting more dynamic use cases like tracking selected tab as a query parameter and navigating with the browser back and forward buttons. This PR introduces the ability to pass in the selectedTabIndex as an arg on the parent component which will override the internal tracking of the selected tab index. This also moves the tab indicator logic to thedid-updatemodifier, which is set up to fire when the selected index changes.Inspiration was taken from https://github.com/concordnow/ember-aria-tabs
📸 Screenshots
Screen.Recording.2023-09-21.at.8.59.41.AM.mov
🔗 External links
Jira ticket: HDS-XXX
Figma file: [if it applies]
👀 Component checklist
💬 Please consider using conventional comments when reviewing this PR.