feat(Wizard): added plain styling#12289
feat(Wizard): added plain styling#12289thatblindgeye wants to merge 5 commits intopatternfly:mainfrom
Conversation
WalkthroughAdds two optional props, Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ 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 |
|
Preview: https://pf-react-pr-12289.surge.sh A11y report: https://pf-react-pr-12289-a11y.surge.sh |
| */ | ||
| shouldFocusContent?: boolean; | ||
| /** Adds plain styling to the wizard. */ | ||
| isPlain?: boolean; |
There was a problem hiding this comment.
Curious, does this prop do nothing? Or are style changes applied with pf-m-plain. I ask because it is hard coded below so I am assuming it has no styles associated with it.
If the prop does not do anything, do we need to add it.
There was a problem hiding this comment.
This prop will dictate whether the pf-m-plain class is applied
![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/Wizard/Wizard.tsx`:
- Around line 62-65: The public prop should be renamed to isNoPlain while
keeping isNoPlainOnGlass as a deprecated alias: update the Wizard component
props (the isPlain/isNoPlainOnGlass declarations) to accept isNoPlain?: boolean
and treat isNoPlainOnGlass as optional legacy input (e.g., const
effectiveIsNoPlain = isNoPlain ?? isNoPlainOnGlass) so existing behavior is
preserved; update all internal usages in Wizard (and the other occurrences
referenced around lines ~84-85 and ~190-195) to read effectiveIsNoPlain, and add
a short deprecation note/comment for isNoPlainOnGlass so consumers know to
migrate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 05040312-0989-4c99-b9e7-989d6905b328
📒 Files selected for processing (1)
packages/react-core/src/components/Wizard/Wizard.tsx
| /** Adds plain styling to the wizard. */ | ||
| isPlain?: boolean; | ||
| /** @beta Prevents the wizard from automatically applying plain styling when glass theme is enabled. */ | ||
| isNoPlainOnGlass?: boolean; |
There was a problem hiding this comment.
Expose isNoPlain as the public prop (keep isNoPlainOnGlass as alias/deprecated).
The behavior is wired correctly, but the public API currently diverges from the requested isNoPlain prop. This can create avoidable churn for consumers and docs.
Proposed compatibility update
export interface WizardProps extends React.HTMLProps<HTMLDivElement> {
/** Adds plain styling to the wizard. */
isPlain?: boolean;
+ /** Prevents the wizard from automatically applying plain styling when glass theme is enabled. */
+ isNoPlain?: boolean;
/** `@beta` Prevents the wizard from automatically applying plain styling when glass theme is enabled. */
isNoPlainOnGlass?: boolean;
}
export const Wizard = ({
...
isPlain = false,
+ isNoPlain = false,
isNoPlainOnGlass = false,
...wrapperProps
}: WizardProps) => {
+ const noPlain = isNoPlain || isNoPlainOnGlass;
...
<div
className={css(
styles.wizard,
isPlain && styles.modifiers.plain,
- isNoPlainOnGlass && styles.modifiers.noPlain,
+ noPlain && styles.modifiers.noPlain,
className
)}Also applies to: 84-85, 190-195
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/react-core/src/components/Wizard/Wizard.tsx` around lines 62 - 65,
The public prop should be renamed to isNoPlain while keeping isNoPlainOnGlass as
a deprecated alias: update the Wizard component props (the
isPlain/isNoPlainOnGlass declarations) to accept isNoPlain?: boolean and treat
isNoPlainOnGlass as optional legacy input (e.g., const effectiveIsNoPlain =
isNoPlain ?? isNoPlainOnGlass) so existing behavior is preserved; update all
internal usages in Wizard (and the other occurrences referenced around lines
~84-85 and ~190-195) to read effectiveIsNoPlain, and add a short deprecation
note/comment for isNoPlainOnGlass so consumers know to migrate.
| */ | ||
| shouldFocusContent?: boolean; | ||
| /** Adds plain styling to the wizard. */ | ||
| isPlain?: boolean; |
There was a problem hiding this comment.
Just curious - why is isNoPlainOnGlass beta but isPlain isn't? I'm not suggesting updating it, I just hadn't thought of these being beta.
There was a problem hiding this comment.
We probably could make both beta; I think at least having the isNo... prop as beta gives us an easier means to remove the prop if needed down the line (without having to deprecate then wait for a breaking release)
mcoker
left a comment
mcoker
left a comment
There was a problem hiding this comment.
LGTM. Only comment is that the example doesn't really show any difference. IMO it's fine but we could also use a wizard with a header so it changes a little.
4 participants
What: Closes #12277
Additional issues:
Summary by CodeRabbit
New Features
Tests
Documentation