Skip to content

Conversation

@geotrev
Copy link
Contributor

@geotrev geotrev commented Apr 18, 2024

Description

As a follow up to #1791, updates various components with new StyledBaseIcon.

Detail

Key changes to note:

  • Replaces React.cloneElement(...) instances by extending StyledBaseIcon
  • Updates custom props to transient props (non-consumer facing)
    • Updates a few prop signatures to map props to transient props

Package/component checklist

  • react-buttons
    • StyledIcon
  • react-forms
    • StyledFileIcon
    • StyledTextMediaFigure
  • react-dropdowns
    • StyledInputIcon
    • StyledOptionIcon
    • StyledOptionTypeIcon
  • react-typography
    • StyledIcon
  • react-notifications
    • StyledIcon
    • StyledGlobalAlertIcon
  • react-pagination
    • StyledIcon
  • react-tags
    • StyledAvatar

Testing considerations:

  • Visual testing IconButton instances (used my multiple packages)
  • Visual testing Button.StartIcon and Button.EndIcon instances (used by multiple packages)
  • Visual testing updated styled icon instances (package-specific)

Checklist

  • 👌 design updates will be Garden Designer approved (add the designer as a reviewer)
  • 🌐 demo is up-to-date (npm start)
  • ⬅️ renders as expected with reversed (RTL) direction
  • 🤘 renders as expected with Bedrock CSS (?bedrock)
  • 💂‍♂️ includes new unit tests. Maintain existing coverage (always >= 96%)
  • ♿ tested for WCAG 2.1 AA accessibility compliance
  • 📝 tested in Chrome, Firefox, Safari, and Edge

@geotrev geotrev self-assigned this Apr 18, 2024
@geotrev geotrev requested a review from a team as a code owner April 18, 2024 19:56
@geotrev geotrev force-pushed the george/styled-icon-refactor branch from 6a68c25 to fc5ef83 Compare April 18, 2024 20:44
@coveralls
Copy link

coveralls commented Apr 18, 2024

Coverage Status

coverage: 96.222% (-0.006%) from 96.228%
when pulling f7c3ee2 on george/styled-icon-refactor
into ff81ed3 on next.

@geotrev geotrev force-pushed the george/styled-icon-refactor branch from ec53455 to 765f345 Compare April 18, 2024 21:24
error: <ErrorIcon />,
warning: <WarningIcon />,
info: <InfoIcon />
}[type];
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not critical to this PR but I can't say no to DRY opportunities.

@geotrev geotrev changed the title 🚧 feat: adds StyledBaseIcon to various icon components feat: adds StyledBaseIcon to various icon components Apr 19, 2024
Copy link
Member

@jzempel jzempel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this cleanup looks fantastic ✨

@geotrev geotrev merged commit 2190e56 into next Apr 19, 2024
@geotrev geotrev deleted the george/styled-icon-refactor branch April 19, 2024 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

5 participants