feat(icon): add rh-standard icon styling - #8280
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughAdded a new PatternFly SVG icon CSS class selector Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
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 `@src/patternfly/base/patternfly-svg-icons.scss`:
- Around line 24-30: The .#{$pf-prefix}icon-rh-standard rule uses incorrect
sizing and property: change width and height from 3em to 0.75em (3/4em) and
replace the color declaration with an SVG fill using the brand token (use fill:
var(--pf-t--global--color--brand--accent--default)) so the selector
.#{$pf-prefix}icon-rh-standard sets width: 0.75em; height: 0.75em; and fill:
var(--pf-t--global--color--brand--accent--default).
🪄 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: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 56a51542-90b8-427d-aeba-8077db4cf687
📒 Files selected for processing (1)
src/patternfly/base/patternfly-svg-icons.scss
| .#{$pf-prefix}icon-rh-standard { | ||
| width: 3em; | ||
| height: 3em; | ||
|
|
||
| // color: var(--pf-t--global--icon--color--brand--default); | ||
| color: var(--pf-t--global--color--brand--accent--default); | ||
| } |
There was a problem hiding this comment.
Requirement mismatch: RH standard icon variant uses wrong size and color property
Line 25 and Line 26 set 3em, but the objective calls for 3/4em (0.75em). Also, Line 29 sets color, while the requirement asks for SVG fill using the brand token.
Proposed fix
.#{$pf-prefix}icon-rh-standard {
- width: 3em;
- height: 3em;
+ width: .75em;
+ height: .75em;
- // color: var(--pf-t--global--icon--color--brand--default);
- color: var(--pf-t--global--color--brand--accent--default);
+ fill: var(--pf-t--global--icon--color--brand--accent--default);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/patternfly/base/patternfly-svg-icons.scss` around lines 24 - 30, The
.#{$pf-prefix}icon-rh-standard rule uses incorrect sizing and property: change
width and height from 3em to 0.75em (3/4em) and replace the color declaration
with an SVG fill using the brand token (use fill:
var(--pf-t--global--color--brand--accent--default)) so the selector
.#{$pf-prefix}icon-rh-standard sets width: 0.75em; height: 0.75em; and fill:
var(--pf-t--global--color--brand--accent--default).
|
Preview: https://pf-pr-8280.surge.sh A11y report: https://pf-pr-8280-a11y.surge.sh |
|
🎉 This PR is included in version 6.5.0-prerelease.65 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Closes #8071.
Add
.pf-v6-icon-rh-standardto any icon to test the styling, as I don't believe core uses any RH standard set icons atm.--pf-t--global--icon--color--brand--defaultfeels like the more correct token to use here, but it currently does not change in the redhat theme so I'm currently using the accent color token instead.Summary by CodeRabbit