fix(icons): replace close icon with rh micron close icon - #8174
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (117)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughReplaces Font Awesome "times" close icons with the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/Alert/examples/Alert.md`:
- Line 469: Two toast examples use the success-specific aria-label in the
close-button partial invocation (the string starting with {{> button
button--IsPlain=true button--attribute='aria-label="Close success alert: Success
alert title"'}}); update those two occurrences so the aria-label matches the
toast type — e.g., change to aria-label="Close danger alert: Danger alert title"
for the danger toast and aria-label="Close info alert: Info alert title" for the
info toast — leaving other attributes (button--IsPlain, button--IsIcon,
button--icon) unchanged.
In `@src/patternfly/components/TextInputGroup/examples/TextInputGroup.md`:
- Around line 109-113: The icon class "fas fa-rh-microns-close" used inside the
text-input-group-utilities partial is invalid and will not render; update the
<i> element inside the {{#> text-input-group-utilities}} block (the button
partial invocation with button--modifier="pf-m-plain" and aria-label="Clear
input") to use a valid icon class (e.g., a correct Font Awesome close icon class
or the project’s PatternFly icon class) or swap to the button partial pattern
suggested in the comment so the clear icon renders correctly.
- Around line 94-98: The icon class in the text-input-group-utilities example is
invalid (uses Font Awesome prefix with a PatternFly icon) and prevents
rendering; update the nested button (the {{#> button ...}} block with
aria-label="Clear error validation example") to use the same PatternFly icon
pattern used elsewhere in this repo—remove the "fas fa-rh-microns-close" class
and replace it with the PatternFly icon usage (either the icon partial/component
or the correct PatternFly icon class used in other examples) so the icon renders
consistently with other examples.
ℹ️ Review info
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (21)
src/patternfly/components/AboutModalBox/examples/AboutModalBox.mdsrc/patternfly/components/ActionList/examples/ActionList.mdsrc/patternfly/components/Alert/examples/Alert.mdsrc/patternfly/components/Drawer/drawer-close.hbssrc/patternfly/components/InlineEdit/inline-edit-action-cancel.hbssrc/patternfly/components/Label/label-group-close.hbssrc/patternfly/components/Label/label.hbssrc/patternfly/components/MenuToggle/examples/MenuToggle.mdsrc/patternfly/components/MenuToggle/menu-toggle-clear.hbssrc/patternfly/components/ModalBox/modal-box-close.hbssrc/patternfly/components/MultipleFileUpload/multiple-file-upload-status-item-close.hbssrc/patternfly/components/NotificationDrawer/notification-drawer-header-action-close.hbssrc/patternfly/components/Popover/popover-close.hbssrc/patternfly/components/Tabs/tabs-item-action.hbssrc/patternfly/components/TextInputGroup/examples/TextInputGroup.mdsrc/patternfly/components/TextInputGroup/text-input-group--search-input.hbssrc/patternfly/components/Toolbar/templates/toolbar--item-label-group.hbssrc/patternfly/components/Wizard/wizard-close.hbssrc/patternfly/demos/AboutModal/about-modal-template.hbssrc/patternfly/demos/Alert/examples/Alert.mdsrc/patternfly/demos/Masthead/examples/Masthead.md
|
Preview: https://pf-pr-8174.surge.sh A11y report: https://pf-pr-8174-a11y.surge.sh |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/patternfly/components/TextInputGroup/examples/TextInputGroup.md (1)
109-113:⚠️ Potential issue | 🟠 MajorSame inconsistency: use button partial pattern.
This has the same issue as lines 94-98. The nested button block with raw
<i>element should be replaced with the consistent button partial pattern.♻️ Proposed fix
- {{#> text-input-group-utilities}} - {{#> button button--modifier="pf-m-plain" button--attribute='aria-label="Clear input"'}} - <i class="rh-microns-close" aria-hidden="true"></i> - {{/button}} - {{/text-input-group-utilities}} + {{#> text-input-group-utilities}} + {{> button button--IsPlain=true button--IsIcon=true button--icon="rh-microns-close" button--attribute='aria-label="Clear input"'}} + {{/text-input-group-utilities}}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/patternfly/components/TextInputGroup/examples/TextInputGroup.md` around lines 109 - 113, Replace the nested raw <i> element inside the text-input-group-utilities block with the consistent button partial pattern used elsewhere: update the block under {{#> text-input-group-utilities}} to invoke the {{#> button}} partial (same button--modifier and aria-label attributes) but use the button partial's icon slot/partial instead of embedding raw HTML; locate the occurrences of the text-input-group-utilities and button partials in this file and mirror the implementation used around lines 94-98 so the icon is rendered via the button partial pattern rather than a raw <i> tag.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/patternfly/components/TextInputGroup/examples/TextInputGroup.md`:
- Around line 109-113: Replace the nested raw <i> element inside the
text-input-group-utilities block with the consistent button partial pattern used
elsewhere: update the block under {{#> text-input-group-utilities}} to invoke
the {{#> button}} partial (same button--modifier and aria-label attributes) but
use the button partial's icon slot/partial instead of embedding raw HTML; locate
the occurrences of the text-input-group-utilities and button partials in this
file and mirror the implementation used around lines 94-98 so the icon is
rendered via the button partial pattern rather than a raw <i> tag.
ℹ️ Review info
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/patternfly/components/Alert/examples/Alert.mdsrc/patternfly/components/TextInputGroup/examples/TextInputGroup.md
ebfaa7f to
6add807
Compare
|
Fixed up a couple more, re-ran and updated visual regressions. Full report - https://drive.google.com/file/d/1PSjE4rJz6RhAvEWOoYiMQhBOwF9u8pdc/view?usp=sharing |
|
🎉 This PR is included in version 6.5.0-prerelease.46 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Fixes #8172
Summary by CodeRabbit