chore: s/cog/rh-ui-settings-fill - #8298
Merged
Merged
Conversation
Contributor
|
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 (2)
✅ Files skipped from review due to trivial changes (2)
WalkthroughReplaced legacy Font Awesome icon identifiers/markup with Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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 |
Collaborator
|
Preview: https://pf-pr-8298.surge.sh A11y report: https://pf-pr-8298-a11y.surge.sh |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/components/MenuToggle/menu-toggle-icon.hbs`:
- Line 13: The menu-toggle component now calls {{pfIcon menu-toggle--icon}} and
fails to render legacy icon names; update callers so menu-toggle--icon uses
valid `@patternfly/react-icons` IDs (replace
"th","question-circle","bookmark","filter" and the typo "rh-ui-settings-fills"
with correct IDs such as "rh-ui-settings-fill" or appropriate
"rh-ui-*/rh-microns-*" names). Edit the listed templates
(masthead-template-content-toolbar-content.hbs,
masthead-template-application-launcher.hbs,
masthead-template-content-icon-group.hbs, toolbar-template-content.hbs,
page-template-context-selector.hbs, toolbar-toggle.hbs,
toolbar-item-search-filter.hbs) to substitute the legacy icon strings with the
supported icon identifiers so {{pfIcon menu-toggle--icon}} resolves correctly.
In `@src/patternfly/demos/Page/page-template-context-selector.hbs`:
- Line 5: Replace the incorrect plural icon token "rh-ui-settings-fills" with
the correct singular token "rh-ui-settings-fill" wherever it's used;
specifically update the menu-toggle--icon attribute instance (currently
menu-toggle--icon="rh-ui-settings-fills") and the empty-state-icon--type
attribute instance (currently empty-state-icon--type="rh-ui-settings-fills") to
use "rh-ui-settings-fill" so they match the rest of the repository.
🪄 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: f0fbd43b-c120-492c-bbbb-4036a5a89fbd
📒 Files selected for processing (15)
src/patternfly/components/ActionList/examples/ActionList.mdsrc/patternfly/components/Alert/examples/Alert.mdsrc/patternfly/components/Icon/examples/Icon.mdsrc/patternfly/components/Menu/examples/Menu.mdsrc/patternfly/components/MenuToggle/examples/MenuToggle.mdsrc/patternfly/components/MenuToggle/menu-toggle-icon.hbssrc/patternfly/components/Toolbar/toolbar-group-action-plain.hbssrc/patternfly/components/TreeView/examples/TreeView.mdsrc/patternfly/components/Wizard/examples/Wizard.mdsrc/patternfly/demos/Masthead/examples/Masthead.mdsrc/patternfly/demos/Masthead/masthead-template-content-icon-group.hbssrc/patternfly/demos/Masthead/masthead-template-content-toolbar-content.hbssrc/patternfly/demos/Masthead/masthead-template-drilldown.hbssrc/patternfly/demos/Page/page-template-context-selector.hbssrc/patternfly/demos/Page/page-template-masthead-drilldown.hbs
| {{else}} | ||
| <i class="fas fa-{{menu-toggle--icon}}" aria-hidden="true"></i> | ||
| {{/ifEquals}} | ||
| {{pfIcon menu-toggle--icon}} |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect all menu-toggle icon assignments
rg -nP --glob '*.{hbs,md,js,jsx,ts,tsx}' 'menu-toggle--icon\s*=\s*"[^"]+"' -C2
# Detect likely legacy Font Awesome-style values
rg -nP --glob '*.{hbs,md,js,jsx,ts,tsx}' 'menu-toggle--icon\s*=\s*"(cog|ellipsis-v|fa-[^"]+|fas[^"]*)"' -C2Repository: patternfly/patternfly
Length of output: 8354
🏁 Script executed:
# Find the pfIcon helper definition
fd -t f 'helpers\.mjs|helpers\.js' --max-one-result
# Also check if there's an icon registry or mapping
fd -t f -i 'icon' --glob '*registry*' --glob '*map*' | head -20Repository: patternfly/patternfly
Length of output: 448
🏁 Script executed:
# Search for pfIcon helper definition and usage
rg -n 'pfIcon\s*[=:]|function pfIcon|export.*pfIcon' --type js --type mjs -C3Repository: patternfly/patternfly
Length of output: 92
🏁 Script executed:
# Find helpers file
fd -type f 'helpers' | grep -E '\.(js|mjs)$'
# Also search for pfIcon definition directly
rg -n 'pfIcon' --type js -C3 | head -100Repository: patternfly/patternfly
Length of output: 235
🏁 Script executed:
# Find helpers file directly
find . -name '*helpers*' -type f | head -10
# Search for pfIcon in all files
rg 'pfIcon' -C3 | head -150Repository: patternfly/patternfly
Length of output: 14100
🏁 Script executed:
# Read the helpers.mjs file to see pfIcon implementation
cat -n ./scripts/helpers.mjs | head -200Repository: patternfly/patternfly
Length of output: 8319
🏁 Script executed:
# Continue reading helpers.mjs to find pfIcon
cat -n ./scripts/helpers.mjs | sed -n '200,400p'Repository: patternfly/patternfly
Length of output: 8607
🏁 Script executed:
# Continue reading the pfIcon implementation
cat -n ./scripts/helpers.mjs | sed -n '397,500p'Repository: patternfly/patternfly
Length of output: 1214
🏁 Script executed:
# Check if `@patternfly/react-icons` is in the repo and look for icon files
find . -path '*/node_modules/@patternfly/react-icons/dist/static' -type d 2>/dev/null | head -5
# Also check package.json to see if it's a dependency
rg '@patternfly/react-icons' package.jsonRepository: patternfly/patternfly
Length of output: 118
🏁 Script executed:
# Check the menu-toggle-icon.hbs file to see the current code
cat -n ./src/patternfly/components/MenuToggle/menu-toggle-icon.hbsRepository: patternfly/patternfly
Length of output: 715
🏁 Script executed:
# Search for icon name patterns used in the codebase to understand the naming convention
rg 'pfIcon\s+["\']' --no-heading -o | sort | uniq -c | sort -rn | head -40Repository: patternfly/patternfly
Length of output: 133
🏁 Script executed:
# Search for icon name patterns used in the codebase to understand the naming convention
rg 'pfIcon' --no-heading | grep -oP 'pfIcon\s+["\x27][^"\x27]+' | sort | uniq -c | sort -rn | head -40Repository: patternfly/patternfly
Length of output: 1444
🏁 Script executed:
# Check for a real issue: look for the typo "rh-ui-settings-fills" (plural)
rg 'rh-ui-settings-fill' --type hbs
# Also look for other potentially problematic icon names
rg 'menu-toggle--icon=' --type hbs -A1 -B1Repository: patternfly/patternfly
Length of output: 5790
🏁 Script executed:
# Check the actual icons available in `@patternfly/react-icons`
find . -path '*/node_modules/@patternfly/react-icons/dist/static*' -type f -name '*.svg' 2>/dev/null | head -20
# Also check if there's a list or index
find . -path '*/node_modules/@patternfly/react-icons*' -name 'index*' 2>/dev/null | head -10Repository: patternfly/patternfly
Length of output: 47
🏁 Script executed:
# Check the menu-toggle.hbs component to see how it passes menu-toggle--icon to menu-toggle-icon
cat -n ./src/patternfly/components/MenuToggle/menu-toggle.hbs | head -50Repository: patternfly/patternfly
Length of output: 2051
🏁 Script executed:
# Continue reading menu-toggle.hbs to see how menu-toggle--icon is used
cat -n ./src/patternfly/components/MenuToggle/menu-toggle.hbs | sed -n '50,150p'Repository: patternfly/patternfly
Length of output: 1922
🏁 Script executed:
# Find all files using these problematic icon names
rg 'menu-toggle--icon="(bookmark|th|question-circle|filter)"' --type hbs
rg 'menu-toggle--icon="rh-ui-settings-fills"' --type hbsRepository: patternfly/patternfly
Length of output: 1352
Fix legacy icon names in menu-toggle callers — they will not render with pfIcon.
Line 13 now requires icon names resolvable by @patternfly/react-icons. Six demo/template files use legacy names that don't exist in that package:
"th","question-circle","bookmark"(Font Awesome-style)"rh-ui-settings-fills"(typo — should be"rh-ui-settings-fill")"filter"(in toolbar components)
These will render as empty HTML comments instead of icons. Migrate to valid icon IDs like "rh-ui-*" or "rh-microns-*" prefixes.
Affected files:
src/patternfly/demos/Masthead/masthead-template-content-toolbar-content.hbssrc/patternfly/demos/Masthead/masthead-template-application-launcher.hbssrc/patternfly/demos/Masthead/masthead-template-content-icon-group.hbssrc/patternfly/demos/Toolbar/toolbar-template-content.hbssrc/patternfly/demos/Page/page-template-context-selector.hbssrc/patternfly/components/Toolbar/toolbar-toggle.hbssrc/patternfly/components/Toolbar/toolbar-item-search-filter.hbs
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/patternfly/components/MenuToggle/menu-toggle-icon.hbs` at line 13, The
menu-toggle component now calls {{pfIcon menu-toggle--icon}} and fails to render
legacy icon names; update callers so menu-toggle--icon uses valid
`@patternfly/react-icons` IDs (replace "th","question-circle","bookmark","filter"
and the typo "rh-ui-settings-fills" with correct IDs such as
"rh-ui-settings-fill" or appropriate "rh-ui-*/rh-microns-*" names). Edit the
listed templates (masthead-template-content-toolbar-content.hbs,
masthead-template-application-launcher.hbs,
masthead-template-content-icon-group.hbs, toolbar-template-content.hbs,
page-template-context-selector.hbs, toolbar-toggle.hbs,
toolbar-item-search-filter.hbs) to substitute the legacy icon strings with the
supported icon identifiers so {{pfIcon menu-toggle--icon}} resolves correctly.
thatblindgeye
approved these changes
Apr 10, 2026
Collaborator
|
🎉 This PR is included in version 6.5.0-prerelease.67 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This was referenced Jul 22, 2026
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
3 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.
fixes #8297
Summary by CodeRabbit