feat(Page): added responsive docked nav#12327
Draft
thatblindgeye wants to merge 1 commit intopatternfly:mainfrom
Draft
feat(Page): added responsive docked nav#12327thatblindgeye wants to merge 1 commit intopatternfly:mainfrom
thatblindgeye wants to merge 1 commit intopatternfly:mainfrom
Conversation
WalkthroughThis PR implements responsive docked navigation by introducing dock and text-expanded styling variants across core components (Button, MenuToggle, Nav), adding a compact masthead logo option, restructuring the Page component's docked layout to support separate mobile and docked mastheads, and providing a complete example implementation with expandable dock state management. Changes
Sequence DiagramsequenceDiagram
actor User
participant PageDockedNav
participant Page
participant Nav
participant Button
participant MenuToggle
User->>PageDockedNav: Click dock toggle (hamburger)
PageDockedNav->>PageDockedNav: Update isDockTextExpanded state
PageDockedNav->>Page: Pass isDockExpanded, isDockTextExpanded, dockedMasthead props
Page->>Page: Apply dock expanded/textExpanded modifiers
PageDockedNav->>Nav: Pass isTextExpanded prop
Nav->>Nav: Conditionally apply textExpanded class
Nav->>Nav: Show/hide nav item text based on isTextExpanded
PageDockedNav->>Button: Render dock variant (conditionally show/hide)
Button->>Button: Apply dock and textExpanded modifiers
PageDockedNav->>MenuToggle: Render dock variant (conditionally show/hide)
MenuToggle->>MenuToggle: Apply dock and textExpanded modifiers
PageDockedNav->>User: Render updated dock UI with expanded/collapsed state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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)
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 |
Collaborator
|
Preview: https://pf-react-pr-12327.surge.sh A11y report: https://pf-react-pr-12327-a11y.surge.sh |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/react-core/src/demos/examples/Page/PageDockedNav.tsx (1)
427-427: Remove unnecessary fragment wrapper.The
Pagecomponent is the only child element, so the wrapping fragment (<>...</>) is unnecessary.♻️ Proposed fix
return ( - <> - <Page + <Page variant="docked" isDockExpanded={isDockExpanded} ... - </Page> - </> + </Page> );Also applies to: 460-461
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react-core/src/demos/examples/Page/PageDockedNav.tsx` at line 427, The fragment wrapper around the single child Page component is unnecessary; remove the surrounding <>...</> so that Page is returned/rendered directly. Update both locations where a fragment encloses Page in PageDockedNav.tsx (the JSX that currently wraps <Page ... />) to eliminate the fragment and leave just the Page element, ensuring any props or surrounding JSX remain intact.packages/react-core/src/components/Page/Page.tsx (1)
31-31: Add JSDoc description and@betatag fordockedMastheadprop.The
dockedMastheadprop is missing both a JSDoc description and the@betatag, unlike the other new dock-related props (isDockExpanded,isDockTextExpanded).📝 Proposed fix
- dockedMasthead?: React.ReactNode; + /** `@beta` Docked masthead component rendered inside the dock container. Only applies when variant is docked. */ + dockedMasthead?: React.ReactNode;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react-core/src/components/Page/Page.tsx` at line 31, The prop declaration dockedMasthead?: React.ReactNode is missing a JSDoc comment and the `@beta` tag; add a short JSDoc description explaining it renders the masthead when the dock is anchored (matching style of isDockExpanded/isDockTextExpanded) and prepend `@beta` so it appears in generated docs, ensuring formatting/comments follow the existing JSDoc conventions used in Page.tsx.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/react-core/src/components/Page/Page.tsx`:
- Line 31: The prop declaration dockedMasthead?: React.ReactNode is missing a
JSDoc comment and the `@beta` tag; add a short JSDoc description explaining it
renders the masthead when the dock is anchored (matching style of
isDockExpanded/isDockTextExpanded) and prepend `@beta` so it appears in generated
docs, ensuring formatting/comments follow the existing JSDoc conventions used in
Page.tsx.
In `@packages/react-core/src/demos/examples/Page/PageDockedNav.tsx`:
- Line 427: The fragment wrapper around the single child Page component is
unnecessary; remove the surrounding <>...</> so that Page is returned/rendered
directly. Update both locations where a fragment encloses Page in
PageDockedNav.tsx (the JSX that currently wraps <Page ... />) to eliminate the
fragment and leave just the Page element, ensuring any props or surrounding JSX
remain intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d29e4335-40ec-48f1-a7ba-77f905c7e5a5
📒 Files selected for processing (7)
packages/react-core/src/components/Button/Button.tsxpackages/react-core/src/components/Masthead/MastheadLogo.tsxpackages/react-core/src/components/MenuToggle/MenuToggle.tsxpackages/react-core/src/components/Nav/Nav.tsxpackages/react-core/src/components/Page/Page.tsxpackages/react-core/src/demos/Page.mdpackages/react-core/src/demos/examples/Page/PageDockedNav.tsx
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
What: Closes #12195
Some notes:
Additional issues:
Summary by CodeRabbit
New Features
Documentation