feat(icons): updated caret icons to RH microns#12298
feat(icons): updated caret icons to RH microns#12298kmcfaul merged 3 commits intopatternfly:mainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (21)
📒 Files selected for processing (43)
✅ Files skipped from review due to trivial changes (39)
🚧 Files skipped from review as they are similar to previous changes (4)
WalkthroughThis PR systematically replaces Angle* caret icons with RhMicronsCaret* icons across many PatternFly React components and examples; no public APIs or component behavior are changed. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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-12298.surge.sh A11y report: https://pf-react-pr-12298-a11y.surge.sh |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
packages/react-core/src/components/ExpandableSection/ExpandableSection.tsx (1)
220-220: Consider centralizing the default expandable caret icon.
ExpandableSectionnow defaultstoggleIconvia props, whileExpandableSectionToggle(detached mode) still hardcodes its own icon. Keeping both wired to one shared constant would reduce drift risk in future icon updates.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react-core/src/components/ExpandableSection/ExpandableSection.tsx` at line 220, The default caret icon is duplicated: ExpandableSection sets toggleIcon inline while ExpandableSectionToggle hardcodes its own icon; create a single exported constant (e.g., DEFAULT_TOGGLE_ICON) and use it in both places so both ExpandableSection's default prop and ExpandableSectionToggle reference the same symbol (update ExpandableSection's prop default and Replace the hardcoded icon in ExpandableSectionToggle with DEFAULT_TOGGLE_ICON).
🤖 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/Icon/examples/Icon.md`:
- Around line 10-11: The imports in Icon.md for RhMicronsCaretRightIcon and
RhMicronsCaretDownIcon will fail because the mappings in pfToRhIcons.mjs for the
rh-microns-caret icons are commented out; either re-enable those mappings in
pfToRhIcons.mjs so the rh-microns-caret-right-icon and
rh-microns-caret-down-icon modules are generated from rhIconsMicrons.mjs, or
update Icon.md to import the active icons RhUiCaretRightIcon and
RhUiCaretDownIcon (using rh-ui-caret-right and rh-ui-caret-down) which map to
CaretRightIcon and CaretDownIcon. Ensure you modify either pfToRhIcons.mjs
(uncomment mappings for CaretRightIcon/CaretDownIcon) or Icon.md (change import
identifiers and module names to the rh-ui-caret variants).
In `@packages/react-core/src/components/Icon/examples/IconBasic.tsx`:
- Around line 4-5: The project imports RhMicronsCaretRightIcon and
RhMicronsCaretDownIcon in IconBasic.tsx but the corresponding mappings in
pfToRhIcons.mjs are commented out, preventing export; open pfToRhIcons.mjs and
uncomment the mappings that bind CaretLeftIcon, CaretRightIcon, and CaretUpIcon
to their rh-microns variants (matching the definitions in rhIconsMicrons.mjs) so
the rh-microns-caret-right and rh-microns-caret-down icons are exported and
available to IconBasic.tsx, or alternatively replace those imports in
IconBasic.tsx with available PF icon symbols if you prefer not to re-enable the
mappings.
In `@packages/react-core/src/deprecated/components/Wizard/WizardToggle.tsx`:
- Line 91: The deprecated WizardToggle component currently applies the
wizardToggleSeparator class directly to RhMicronsCaretRightIcon; update the JSX
in the WizardToggle (the conditional rendering using activeStepSubName and
RhMicronsCaretRightIcon) to wrap the icon in a span element with
className={css(styles.wizardToggleSeparator)} and render the icon inside that
span (remove the className from RhMicronsCaretRightIcon), matching the
non-deprecated WizardToggle pattern.
In `@packages/react-table/src/components/Table/utils/decorators/treeRow.tsx`:
- Line 7: The current import uses the microns caret icon; replace the import of
RhMicronsCaretDownIcon from
'@patternfly/react-icons/dist/esm/icons/rh-microns-caret-down-icon' with
RhUiCaretDownIcon imported from
'@patternfly/react-icons/dist/esm/icons/rh-ui-caret-down-icon' and update any
usages/JSX that reference RhMicronsCaretDownIcon (e.g., the tree row
expand/collapse renderer or component that renders the caret) to use
RhUiCaretDownIcon so the correct UI icon (rh-ui-caret-down) and its dimensions
are used.
---
Nitpick comments:
In `@packages/react-core/src/components/ExpandableSection/ExpandableSection.tsx`:
- Line 220: The default caret icon is duplicated: ExpandableSection sets
toggleIcon inline while ExpandableSectionToggle hardcodes its own icon; create a
single exported constant (e.g., DEFAULT_TOGGLE_ICON) and use it in both places
so both ExpandableSection's default prop and ExpandableSectionToggle reference
the same symbol (update ExpandableSection's prop default and Replace the
hardcoded icon in ExpandableSectionToggle with DEFAULT_TOGGLE_ICON).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 16b39385-e4cb-40ba-9438-c4291a374fc4
⛔ Files ignored due to path filters (21)
packages/react-core/src/components/BackToTop/__tests__/__snapshots__/BackToTop.test.tsx.snapis excluded by!**/*.snappackages/react-core/src/components/Breadcrumb/__tests__/__snapshots__/Breadcrumb.test.tsx.snapis excluded by!**/*.snappackages/react-core/src/components/Card/__tests__/__snapshots__/CardHeader.test.tsx.snapis excluded by!**/*.snappackages/react-core/src/components/ClipboardCopy/__tests__/__snapshots__/ClipboardCopyToggle.test.tsx.snapis excluded by!**/*.snappackages/react-core/src/components/DataList/__tests__/Generated/__snapshots__/DataListToggle.test.tsx.snapis excluded by!**/*.snap,!**/generated/**packages/react-core/src/components/DataList/__tests__/__snapshots__/DataListToggle.test.tsx.snapis excluded by!**/*.snappackages/react-core/src/components/DatePicker/__tests__/__snapshots__/DatePicker.test.tsx.snapis excluded by!**/*.snappackages/react-core/src/components/ExpandableSection/__tests__/__snapshots__/ExpandableSection.test.tsx.snapis excluded by!**/*.snappackages/react-core/src/components/Form/__tests__/__snapshots__/FormFieldGroup.test.tsx.snapis excluded by!**/*.snappackages/react-core/src/components/JumpLinks/__tests__/__snapshots__/JumpLinks.test.tsx.snapis excluded by!**/*.snappackages/react-core/src/components/MultipleFileUpload/__tests__/__snapshots__/MultipleFileUploadStatus.test.tsx.snapis excluded by!**/*.snappackages/react-core/src/components/Nav/__tests__/Generated/__snapshots__/NavExpandable.test.tsx.snapis excluded by!**/*.snap,!**/generated/**packages/react-core/src/components/Nav/__tests__/__snapshots__/Nav.test.tsx.snapis excluded by!**/*.snappackages/react-core/src/components/NotificationDrawer/__tests__/__snapshots__/NotificationDrawerGroup.test.tsx.snapis excluded by!**/*.snappackages/react-core/src/components/Page/__tests__/__snapshots__/Page.test.tsx.snapis excluded by!**/*.snappackages/react-core/src/components/Tabs/__tests__/__snapshots__/OverflowTab.test.tsx.snapis excluded by!**/*.snappackages/react-core/src/components/Tabs/__tests__/__snapshots__/Tabs.test.tsx.snapis excluded by!**/*.snappackages/react-core/src/components/TreeView/__tests__/__snapshots__/TreeViewListItem.test.tsx.snapis excluded by!**/*.snappackages/react-core/src/deprecated/components/DualListSelector/__tests__/__snapshots__/DualListSelector.test.tsx.snapis excluded by!**/*.snappackages/react-core/src/deprecated/components/Wizard/__tests__/__snapshots__/Wizard.test.tsx.snapis excluded by!**/*.snappackages/react-table/src/deprecated/components/Table/__tests__/__snapshots__/Table.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (43)
packages/code-connect/components/Breadcrumbs/ExpandableBreadcrumbs.figma.tsxpackages/react-core/src/components/Accordion/AccordionToggle.tsxpackages/react-core/src/components/Accordion/__tests__/AccordionToggle.test.tsxpackages/react-core/src/components/Alert/AlertToggleExpandButton.tsxpackages/react-core/src/components/Alert/__tests__/AlertToggleExpandButton.test.tsxpackages/react-core/src/components/BackToTop/BackToTop.tsxpackages/react-core/src/components/Breadcrumb/BreadcrumbHeading.tsxpackages/react-core/src/components/Breadcrumb/BreadcrumbItem.tsxpackages/react-core/src/components/Breadcrumb/examples/Breadcrumb.mdpackages/react-core/src/components/Breadcrumb/examples/BreadcrumbDropdown.tsxpackages/react-core/src/components/CalendarMonth/CalendarMonth.tsxpackages/react-core/src/components/Card/CardHeader.tsxpackages/react-core/src/components/ClipboardCopy/ClipboardCopyToggle.tsxpackages/react-core/src/components/DataList/DataListToggle.tsxpackages/react-core/src/components/DataList/examples/DataList.mdpackages/react-core/src/components/DualListSelector/DualListSelectorTreeItem.tsxpackages/react-core/src/components/ExpandableSection/ExpandableSection.tsxpackages/react-core/src/components/ExpandableSection/ExpandableSectionToggle.tsxpackages/react-core/src/components/Form/FormFieldGroupToggle.tsxpackages/react-core/src/components/Icon/examples/Icon.mdpackages/react-core/src/components/Icon/examples/IconBasic.tsxpackages/react-core/src/components/JumpLinks/JumpLinks.tsxpackages/react-core/src/components/Menu/MenuItem.tsxpackages/react-core/src/components/Menu/examples/Menu.mdpackages/react-core/src/components/Menu/examples/MenuWithDrilldownBreadcrumbs.tsxpackages/react-core/src/components/Nav/NavExpandable.tsxpackages/react-core/src/components/Nav/NavItem.tsxpackages/react-core/src/components/Nav/NavList.tsxpackages/react-core/src/components/NotificationDrawer/NotificationDrawerGroup.tsxpackages/react-core/src/components/Tabs/OverflowTab.tsxpackages/react-core/src/components/Tabs/Tabs.tsxpackages/react-core/src/components/TreeView/TreeViewListItem.tsxpackages/react-core/src/components/Wizard/WizardNavItem.tsxpackages/react-core/src/components/Wizard/WizardToggle.tsxpackages/react-core/src/demos/DataList/examples/DataListExpandableControlInToolbar.tsxpackages/react-core/src/demos/DataListDemo.mdpackages/react-core/src/deprecated/components/DualListSelector/DualListSelectorTreeItem.tsxpackages/react-core/src/deprecated/components/Wizard/WizardNavItem.tsxpackages/react-core/src/deprecated/components/Wizard/WizardToggle.tsxpackages/react-integration/demo-app-ts/src/components/demos/BreadcrumbDemo/BreadcrumbDemo.tsxpackages/react-table/src/components/Table/CollapseColumn.tsxpackages/react-table/src/components/Table/utils/decorators/treeRow.tsxpackages/react-table/src/demos/Table.md
packages/react-core/src/deprecated/components/Wizard/WizardToggle.tsx
Outdated
Show resolved
Hide resolved
5f76cd3 to
9e8b1bb
Compare
mcoker
left a comment
There was a problem hiding this comment.
LGTM
Ran visual regressions against main with the latest core prerelease if anyone wants to see the diff
Full report - unzip and open "html_report/index.html"
|
Your changes have been released in:
Thanks for your contribution! 🎉 |
What: Closes #12261
Note that I did not update the up/down icons used in SearchInput, nor the caret icons used in DualListSelector/Pagination (or similar implementations that have actions with both single and double carets). There's core followup work to handle that first patternfly/patternfly#8197
Additional issues:
Summary by CodeRabbit