-
Notifications
You must be signed in to change notification settings - Fork 5k
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 audit, remove xxs icon size #17089
Conversation
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
Builds ready [9a3a2f8]
Page Load Metrics (1425 ± 83 ms)
Bundle size diffs
highlights:storybook
|
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.
Oh you are right the generator does remove the |
Builds ready [6e39bad]
Page Load Metrics (1280 ± 92 ms)
Bundle size diffs
highlights:storybook
|
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.
I think we can also remove all the ids from the svgs files.
Builds ready [1471f39]
Page Load Metrics (1531 ± 135 ms)
Bundle size diffs
highlights:storybook
|
6be9cce
to
77ba798
Compare
Builds ready [eb82787]
Page Load Metrics (1193 ± 37 ms)
Bundle size diffs
highlights:storybook
|
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.
Everything looks great! Just one suggestion about the font awesome story
Builds ready [321b212]
Page Load Metrics (1295 ± 113 ms)
Bundle size diffs
highlights:storybook
|
Codecov Report
@@ Coverage Diff @@
## develop #17089 +/- ##
========================================
Coverage 59.80% 59.80%
========================================
Files 937 937
Lines 36141 36141
Branches 9284 9284
========================================
Hits 21614 21614
Misses 14527 14527
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
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!
Builds ready [a2d9910]
Page Load Metrics (1301 ± 102 ms)
Bundle size diffs
|
Explanation
Update Icon names, remove unused icons, add missing icons, and fix icons with stroke/line issues.
Screenshots/Screencaps
Before
After
Manual Testing Steps
yarn test:unit:jest ui/components/component-library
Pre-merge author checklist
Pre-merge reviewer checklist
If further QA is required (e.g. new feature, complex testing steps, large refactor), add the
Extension QA Board
label.In this case, a QA Engineer approval will be be required.