Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Icon Cleanup #179

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Icon Cleanup #179

wants to merge 1 commit into from

Conversation

Jeremy-Walton
Copy link
Member

@Jeremy-Walton Jeremy-Walton commented Jun 15, 2023

Task

TR #126

Why?

Icon classes were implemented before some of the scale patterns found in other components were established. Additionally, there were some cases of needing to force an icon style from another component. Rather than duplicate properties, Adding more placeholder selectors allows extending without duplicating.

What Changed

  • Refactor icon implementation to match api naming patterns
  • Refactor icon to use placeholder selector pattern
  • Make placeholder selectors for size and weight properties of icon to allow overriding from a parent component.

Sanity Check

  • Have you updated any usage of changed tokens?
  • Have you updated the docs with any component changes?
  • Have you updated the dependency graph with any component changes?
  • Have you run linters?
  • Have you run prettier?
  • Have you tried building the css?
  • Have you tried building storybook?
  • Do you need to update the package version?

@Jeremy-Walton Jeremy-Walton added documentation Improvements or additions to documentation Breaking Change This will cause problems so be sure to indicate that in the release notes Components Changes to a component labels Jun 15, 2023
@Jeremy-Walton Jeremy-Walton requested a review from scriswell June 15, 2023 16:14
@Jeremy-Walton Jeremy-Walton self-assigned this Jun 15, 2023
@Jeremy-Walton
Copy link
Member Author

Icon Revisions

@Jeremy-Walton Jeremy-Walton added this to the v0.4.3 Release milestone Jun 15, 2023
@Jeremy-Walton
Copy link
Member Author

Note: This PR is on hold since it technically introduces breaking changes.

This does clean up icon usage significantly and doesn't change the usage (names of classes), but it does affect anyone who is overriding or customizing their icon classes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blocked Breaking Change This will cause problems so be sure to indicate that in the release notes Components Changes to a component documentation Improvements or additions to documentation
Development

Successfully merging this pull request may close these issues.

2 participants