Icon house keeping updates #16621
Icon house keeping updates #16621
Conversation
georgewrmarshall
added
area-UI
|
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. |
| * Search for an icon: https://metamask.github.io/metamask-storybook/?path=/story/ui-components-component-library-icon-icon-stories-js--default-story | ||
| * | ||
| * Add an icon: https://metamask.github.io/metamask-storybook/?path=/docs/ui-components-component-library-icon-icon-stories-js--default-story#adding-a-new-icon | ||
| * | ||
| * ICON_NAMES is generated using svgs in app/images/icons and | ||
| * the generateIconNames script in development/generate-icon-names.js | ||
| * then stored as an environment variable |
There was a problem hiding this comment.
Updating description here in hopes of directing engineers to the search icon page in storybook
| LG: SIZES.LG, | ||
| XL: SIZES.XL, | ||
| AUTO: SIZES.AUTO, | ||
| }; |
There was a problem hiding this comment.
Adding ICON_SIZES to curate sizes passed to storybook controls and propTypes
| * Possible values could be 'SIZES.XXS', 'SIZES.XS', 'SIZES.SM', 'SIZES.MD', 'SIZES.LG', 'SIZES.XL', | ||
| * Default value is 'SIZES.MD'. | ||
| * Possible values could be SIZES.XXS (10px), SIZES.XS (12px), SIZES.SM (16px), SIZES.MD (20px), SIZES.LG (24px), SIZES.XL (32px), | ||
| * Default value is SIZES.MD (20px). |
There was a problem hiding this comment.
Adding pixel values for sizes in propType comments
| @@ -107,7 +108,7 @@ export const TextFieldBase = ({ | |||
| backgroundColor={COLORS.BACKGROUND_DEFAULT} | |||
| alignItems={ALIGN_ITEMS.CENTER} | |||
| borderWidth={1} | |||
| borderRadius={SIZES.SM} | |||
| borderRadius={BORDER_RADIUS.SM} | |||
| paddingLeft={leftAccessory ? 4 : 0} | |||
| paddingRight={rightAccessory ? 4 : 0} | |||
| onClick={handleClick} | |||
There was a problem hiding this comment.
I could move these into the text-field-base updates into the house keeping pr for text-field-base if it's cleaner I'll work on that next.
| @@ -40,6 +40,7 @@ | |||
| &__input { | |||
| border: none; | |||
| height: 100%; | |||
| width: 100%; | |||
There was a problem hiding this comment.
This fixes an issue I ran into using the read only input in the icon search page. I've checked other component libraries and it seems they do the same https://chakra-ui.com/docs/components/input/usage
georgewrmarshall
requested review from
PeterYinusa,
garrettbear and
NidhiKJha
and removed request for
PeterYinusa
Explanation
What is the current state of things and why does it need to change?
Some small updates to the
Iconcomponent were required to match the quality and standard of our other componentsWhat is the solution your changes offer and how does it work?
This PR includes all of the house keeping updates that are listed in the issue as well as some other DX improvements
Fixes 🧹 [UI House Keeping] Icon #16186
Screenshots/Screencaps
Before
icon.before.mov
After
icon.after.mov
Manual Testing Steps
Iconin the search bar (make sure it's the one in 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 Boardlabel.In this case, a QA Engineer approval will be be required.
House keeping checklist
Also checked off in issue #16186
classNameprop and the PropType descriptions are all the samesizeand are suggesting the use of the generalizeddesign-system.jsconstants e.g.SIZESas the primary option but noting the component consts in the documentation and using them for propType validation and storybook controls onlyimgSrc,imgAlt(html element + attribute) (needs audit)sizeprop would beexport const Size = (args) => (sizeprop would be### SizeColor, Background Color And Border Colorstory inbase-avatar- [ ] yes when it makes sense toButtonBasemm-prefix to all classNamesindex.jsfile in component-library// TODO:comment so we can search for them at a later date e.g. blocking components etcSIZE.MD (32px)