Skip to content

Conversation

@geotrev
Copy link
Contributor

@geotrev geotrev commented Mar 21, 2024

Description

Updates multiple packages so their icon prop types are consistent with other packages. Also updates several component type signatures as well.

See migration.md for full changelog.

Detail

There are several icon components still using div styled component that nest an svg icon inside them. I didn't touch these as they're still spreading props and setting their ref to the styled div, not the svg directly.

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

@coveralls
Copy link

coveralls commented Mar 21, 2024

Coverage Status

coverage: 96.022% (-0.001%) from 96.023%
when pulling 84dd45f on george/icon-type-update
into d5fa1dc on next.

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.

@geotrev this is a good start and good for what this PR is. But I think we're looking for more out of the sequence task before it's considered complete for v9. Let's sync up on the internal doc comments.

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.

Now I think we're missing Span icons. Maybe others. Be sure to scour for styled component usage with cloneElement. Fwiw, I think we should expose a utility StyledIcon (or something similar) base class. But that's TBD.

@geotrev geotrev force-pushed the george/icon-type-update branch from 7f37d36 to 52bb585 Compare March 25, 2024 20:15
@geotrev geotrev force-pushed the george/icon-type-update branch from 52bb585 to 84dd45f Compare March 25, 2024 20:17
@geotrev geotrev changed the title feat(accordions): update icon types to ReactElement feat: updates icon component and prop types for cross-package consistency Mar 25, 2024
@geotrev geotrev merged commit 759548a into next Mar 26, 2024
@geotrev geotrev deleted the george/icon-type-update branch March 26, 2024 15:49
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