Bugfixes for dynamic tab panels - feedback requested
Summary of the changes
Significant refactor of tab components - internal props removed from IcTabPanel, because they were managed by IcTabContext already.
The ticket identifies a number of issues:
Tab panel appears above the tabs
Fixed - but with the addition of an empty <span> inside the 'Dynamic Rendering' story's IcTabContext.
I think a proper fix to this might require enforcing slots in IcTabContext, so it can enforce the order that TabGroup and TabPanels render in?
The first tab is not selected
Fixed!
If the second tab is selected and another tab is dynamically removed, the second tab loses the selected state and the first tab is selected
Unable to replicate!
All tabPanels are displayed when tabs are dynamically loaded and a tab is selected
Unable to resolve - I spent a long time looking into this. The issue seems to be that newly created tabs aren't given the hidden='true' attribute and that's why they all appear when a new tab is selected.
I think it's down to the IcTabContext not setting them up correctly - but i spent a significant amount of time trying to adjust configureTabs() and linkTabs() - and even hard-setting hidden to true on new tab panels didn't stick, they would still render when a new tab was selected
Related issue
#1799
Checklist
Testing
- [x] Relevant unit tests and visual regression tests added.
- [x] Playground stories in React Storybook up to date, with any prop changes and additions addressed.
- [x] Compare performance of modified components against develop using Performance addon in React Storybook.
Accessibility
- [x] Accessibility Insights FastPass performed.
- [x] A11y plug-in on Storybook yields no issues.
- [x] Manual screen reader testing performed using NVDA and VoiceOver.
- [x] Manual keyboard testing for keyboard controls and logical focus order.
- [x] Correct roles used and ARIA attributes used correctly where required.
- [x] Logical heading structure is maintained, and the HTML elements used for headings can be changed to fit within the wider page structure.
Testing content extremes
- [x] Min/max content examples tested with no loss of content or overflow.
- [x] All prop combinations work without issue.
- [x] Tested for FOUC (Flash of Unstyled Content) in both SSR (Server-Side Rendering) and SSG (Static Site Generation) settings.
- [x] Controlled and uncontrolled input components tested.
- [x] Props/slots can be updated after initial render.
View your branch deployment here: https://mi6.github.io/ic-ui-kit/branches/1799-dynamic-tab-panels/web-components View your React branch deployment here: https://mi6.github.io/ic-ui-kit/branches/1799-dynamic-tab-panels/react
Tab panel appears above the tabs yep, appears fixed. seems reasonable as otherwise we need a breaking change i guess to resolve as you suggest
The first tab is not selected Fixed
If the second tab is selected and another tab is dynamically removed, the second tab loses the selected state and the first tab is selected Also unable to replicate!
All tabPanels are displayed when tabs are dynamically loaded and a tab is selected Having looked as this too (a while ago). It seems to be a timing issue with iterating over the same array of elements to render tab and the tap-panel. I'm not really sure what can be done to resolve this
A few comments - generally agree with @ad9242:
Tab panel appears above the tabs I agree that adding slots for the TabGroup and TabPanels would be a way to fix this properly in future, and that the current fix is reasonable.
The first tab is not selected Yep, agree that this is fixed.
If the second tab is selected and another tab is dynamically removed, the second tab loses the selected state and the first tab is selected I am able to replicate this in the StackBlitz provided in the ticket (although only after refreshing the whole page, and only sometimes!). But I think the changes in this PR have fixed it - I am not able to replicate it in the story that has been added.
All tabPanels are displayed when tabs are dynamically loaded and a tab is selected Not sure what can be done about this either - I would need to spend a lot more time looking into it in detail before I can suggest anything really