feat(Panel, CompassPanel): Removed CompassPanel component. Updated Panel component for use in with compass#12372
Open
tlabaj wants to merge 1 commit intopatternfly:mainfrom
Open
feat(Panel, CompassPanel): Removed CompassPanel component. Updated Panel component for use in with compass#12372tlabaj wants to merge 1 commit intopatternfly:mainfrom
tlabaj wants to merge 1 commit intopatternfly:mainfrom
Conversation
…el component for use in with compass
tlabaj
requested review from
a team,
mcoker,
nicolethoen and
wise-king-sullyman
and removed request for
a team
WalkthroughThe PR removes the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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-12372.surge.sh A11y report: https://pf-react-pr-12372-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 (1)
packages/react-core/src/components/Compass/CompassMainHeader.tsx (1)
21-24: Consider adding aPanelMainBodyprop passthrough for migration parity.
compassPanelPropsnow only reaches the outerPanel, but the no-padding replacement lives onPanelMainBody. Consumers using thetitle/toolbarshortcut can’t apply that without switching to customchildrenand recreating the title/toolbar wrappers.Possible API extension
export interface CompassMainHeaderProps extends Omit<React.HTMLProps<HTMLDivElement>, 'title'> { @@ compassPanelProps?: Omit<PanelProps, 'children'>; + /** Additional props passed to the PanelMainBody that wraps the main header content when using the title or toolbar props. */ + compassPanelMainBodyProps?: Omit<React.ComponentProps<typeof PanelMainBody>, 'children'>; } export const CompassMainHeader: React.FunctionComponent<CompassMainHeaderProps> = ({ @@ children, compassPanelProps, + compassPanelMainBodyProps, ...props }: CompassMainHeaderProps) => { @@ <Panel {...compassPanelProps}> <PanelMain> - <PanelMainBody> + <PanelMainBody {...compassPanelMainBodyProps}> <CompassMainHeaderContent>Also applies to: 37-40
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react-core/src/components/Compass/CompassMainHeader.tsx` around lines 21 - 24, The current CompassMainHeader component only exposes compassPanelProps to the outer Panel but not to the inner PanelMainBody, preventing consumers from toggling things like no-padding when using title/toolbar; add a new prop (e.g., compassPanelMainBodyProps?: Omit<PanelMainBodyProps,'children'>) to the CompassMainHeader props, thread that prop into the JSX where PanelMainBody is rendered inside CompassMainHeader (apply the props to the PanelMainBody element), and update the component prop typing and any usages or tests (also replicate the same change at the other related spot referenced in the comment) so consumers can pass no-padding and other PanelMainBody-specific options without needing to provide children.
🤖 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/Compass/CompassMainHeader.tsx`:
- Around line 21-24: The current CompassMainHeader component only exposes
compassPanelProps to the outer Panel but not to the inner PanelMainBody,
preventing consumers from toggling things like no-padding when using
title/toolbar; add a new prop (e.g., compassPanelMainBodyProps?:
Omit<PanelMainBodyProps,'children'>) to the CompassMainHeader props, thread that
prop into the JSX where PanelMainBody is rendered inside CompassMainHeader
(apply the props to the PanelMainBody element), and update the component prop
typing and any usages or tests (also replicate the same change at the other
related spot referenced in the comment) so consumers can pass no-padding and
other PanelMainBody-specific options without needing to provide children.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cbf54911-dac2-4c40-a44d-9489cf0ba7c3
⛔ Files ignored due to path filters (4)
packages/react-core/src/components/Compass/__tests__/__snapshots__/CompassMainHeader.test.tsx.snapis excluded by!**/*.snappackages/react-core/src/components/Compass/__tests__/__snapshots__/CompassPanel.test.tsx.snapis excluded by!**/*.snappackages/react-core/src/components/Panel/__tests__/__snapshots__/Panel.test.tsx.snapis excluded by!**/*.snapyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (21)
packages/react-core/package.jsonpackages/react-core/src/components/Compass/CompassContent.tsxpackages/react-core/src/components/Compass/CompassMainHeader.tsxpackages/react-core/src/components/Compass/CompassMainHeaderContent.tsxpackages/react-core/src/components/Compass/CompassPanel.tsxpackages/react-core/src/components/Compass/__tests__/CompassMainHeader.test.tsxpackages/react-core/src/components/Compass/__tests__/CompassPanel.test.tsxpackages/react-core/src/components/Compass/examples/Compass.mdpackages/react-core/src/components/Compass/examples/CompassBasic.tsxpackages/react-core/src/components/Compass/examples/CompassDockLayout.tsxpackages/react-core/src/components/Compass/index.tspackages/react-core/src/components/Panel/Panel.tsxpackages/react-core/src/components/Panel/__tests__/Panel.test.tsxpackages/react-core/src/components/Panel/examples/Panel.mdpackages/react-core/src/components/Panel/examples/PanelPill.tsxpackages/react-core/src/components/Panel/examples/PanelScrollableWithAutoHeight.tsxpackages/react-core/src/demos/Compass/examples/CompassDemo.tsxpackages/react-core/src/demos/Compass/examples/CompassDockDemo.tsxpackages/react-docs/package.jsonpackages/react-styles/package.jsonpackages/react-tokens/package.json
💤 Files with no reviewable changes (3)
- packages/react-core/src/components/Compass/index.ts
- packages/react-core/src/components/Compass/tests/CompassPanel.test.tsx
- packages/react-core/src/components/Compass/CompassPanel.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 #
Summary
Replaces the Compass-specific
CompassPanelwrapper with the sharedPanelAPI and extendsPanelwith several layout/style modifiers. Compass examples and theCompassMainHeadertitle/toolbar path now usePanel→PanelMain→PanelMainBody. Panel docs and tests are updated to match.Compass
CompassPaneland its unit tests / snapshots; dropped its export fromCompass/ package surface.CompassMainHeader: Renders the title/toolbar layout withPanel,PanelMain, andPanelMainBodyaroundCompassMainHeaderContent. ThecompassPanelPropsobject is typed asOmit<PanelProps, 'children'>(replacesCompassPanelProps).Compass.mdand structure examples now describePanel/PanelMain/PanelMainBodyinstead ofCompassPanel.CompassContentandCompassMainHeaderContenttext updated to reference thePanelstack.CompassMainHeadertests and snapshots expect thePanelroot (e.g.pf-v6-c-panel) instead of the old compass panel class.CompassBasic,CompassDockLayout,CompassDemo, andCompassDockDemouse thePanelstack.CompassDemo: Restores priorCompassPanelbehavior by passingisPillonPanelwhere the old code usedisPill, and usingstyle={{ padding: 0 }}onPanelMainBodywhere the old code usedhasNoPadding(nohasNoPaddingonPanelin core today).Panel
New optional props (JSDoc
@beta) onPanel, applied viacss()and existing PatternFlypf-m-*modifiers in@patternfly/react-styles:hasNoBorderpf-m-no-borderisFullHeightpf-m-full-heightisGlasspf-m-glassisAutoHeightpf-m-scrollable-auto-heightisPillpf-m-pill(
isScrollable/variantremain as before.)Panel — docs & examples
PanelPill.tsx: PillPanelwith a single “Main content” line in the body.PanelScrollableWithAutoHeight.tsx: Scrollable panel with auto height in a short parent container.Panel.md: New sections for Pill and Scrollable with auto height (and ordering adjusted vs. “Scrollable” / “Scrollable with header and footer” as implemented).Panel — tests
import '@testing-library/jest-dom'sotoBeVisible/toHaveClass/ etc. are typed and available.false/ undefined.hasNoBorder,isFullHeight,isGlass,isAutoHeightwithisScrollable,isPill).Breaking changes
CompassPanelis removed; consumers should usePanel+PanelMain+PanelMainBody(andcompassPanelPropsnow matchesPanelonly — oldCompassPanel-only flags likeisPill/hasNoBorderon that API are now expressed withPanelprops or, for “no body padding,” viaPanelMainBody/ layout as needed).Summary by CodeRabbit
New Features
isAutoHeight,hasNoBorder,isFullHeight,isGlass, andisPillfor additional visual variants.Breaking Changes
Documentation