fix(Tabs): select nav tab from initial hash#12297
fix(Tabs): select nav tab from initial hash#12297tarunvashishth wants to merge 1 commit intopatternfly:mainfrom
Conversation
WalkthroughThe Tabs component now derives the active tab from URL hash fragments for nav-style tabs, addressing a bug where direct URL access with hash anchors did not highlight the corresponding tab. This includes state management for hash tracking, event listener initialization, and fallback logic preserving existing controlled/uncontrolled behavior when no matching hash is present. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Preview: https://pf-react-pr-12297.surge.sh A11y report: https://pf-react-pr-12297-a11y.surge.sh |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/react-core/src/components/Tabs/Tabs.tsx`:
- Around line 217-229: Tabs currently lets the URL hash override props.activeKey
even in controlled mode: change Tabs.getActiveKeyFromProps to only consider
Tabs.getActiveKeyFromCurrentUrl when the component is uncontrolled (i.e.,
props.activeKey is undefined), so controlled consumers remain authoritative;
likewise, in the internal routine that writes the hash (the code that updates
the URL hash without notifying the parent), either skip updating the URL when
props.activeKey is provided or invoke the owner callback (e.g.,
onSelect/onChange) so the parent can sync before the hash becomes authoritative.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 11069d60-5d7e-45ac-a245-2d32b5b7c58d
📒 Files selected for processing (4)
packages/react-core/src/components/Tabs/Tabs.tsxpackages/react-core/src/components/Tabs/__tests__/Tabs.test.tsxpackages/react-core/src/components/Tabs/examples/Tabs.mdpackages/react-core/src/components/Tabs/examples/TabsNavInitialHash.tsx
| static getActiveKeyFromProps = ( | ||
| props: TabsProps, | ||
| uncontrolledActiveKey: TabsState['uncontrolledActiveKey'], | ||
| currentUrlHash?: string | ||
| ) => { | ||
| const activeKeyFromCurrentUrl = Tabs.getActiveKeyFromCurrentUrl(props, currentUrlHash); | ||
|
|
||
| if (activeKeyFromCurrentUrl !== undefined) { | ||
| return activeKeyFromCurrentUrl; | ||
| } | ||
|
|
||
| return props.defaultActiveKey !== undefined ? uncontrolledActiveKey : props.activeKey; | ||
| }; |
There was a problem hiding this comment.
Keep activeKey authoritative in controlled mode.
Lines 224-228 make the hash match win over props.activeKey, and Lines 411-420 update that hash internally without notifying the owner. A controlled <Tabs activeKey={...}> can now render a different selected tab than the parent state on initial load or after hashchange, which breaks separate-content patterns and any parent UI derived from activeKey. Either scope hash resolution to uncontrolled tabs, or add a callback path that keeps the owner in sync before the hash becomes authoritative.
Also applies to: 411-420
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/react-core/src/components/Tabs/Tabs.tsx` around lines 217 - 229,
Tabs currently lets the URL hash override props.activeKey even in controlled
mode: change Tabs.getActiveKeyFromProps to only consider
Tabs.getActiveKeyFromCurrentUrl when the component is uncontrolled (i.e.,
props.activeKey is undefined), so controlled consumers remain authoritative;
likewise, in the internal routine that writes the hash (the code that updates
the URL hash without notifying the parent), either skip updating the URL when
props.activeKey is provided or invoke the owner callback (e.g.,
onSelect/onChange) so the parent can sync before the hash becomes authoritative.
2 participants
closes #12094
Summary by CodeRabbit
New Features
Tests
Documentation