-
Notifications
You must be signed in to change notification settings - Fork 324
Breaking Change: Remove built-in icons #2082
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
Conversation
🦋 Changeset detectedLatest commit: 537cd03 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Can you please make sure the following icons are not removed since they are being used in the staging repo?
|
Thanks for the review @ayush987goyal. IconClose is still there but I will add the other three back: |
Added back the icons needed by staging repo |
This pull request introduces 1 alert when merging d476820 into 28bf92d - view on LGTM.com new alerts:
|
Thanks @reesscot . Let's discuss offline to see how can we remove the dependency from staging repo on these icons. We can then remove these ones too later. |
--- | ||
"@aws-amplify/ui-react": major | ||
--- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
Description of changes
This PR is an updated version of #1310. It removes all the icons not used in our code base and updates all the docs examples to use
pathData
instead of the built-in icons.@aws-amplify/ui-react
BEFORE removing icons:@aws-amplify/ui-react
AFTER removing icons:Issue #, if available
Description of how you validated changes
Checklist
yarn test
passessideEffects
field updatedBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.