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

tag component housekeeping #16355

Merged
merged 2 commits into from
Nov 3, 2022
Merged

tag component housekeeping #16355

merged 2 commits into from
Nov 3, 2022

Conversation

NidhiKJha
Copy link
Member

@NidhiKJha NidhiKJha commented Nov 2, 2022

Explanation

Housekeeping Checklist

  • Has a className prop and the PropType descriptions are all the same
  • Prop table in MDX docs have the "Accepts all Box component props" description and link
  • We are consistent when using the same prop names like size and are suggesting the use of the generalized design-system.js constants e.g. SIZES as the primary option but noting the component consts in the documentation and using them for propType validation and storybook controls only
  • Standardize all similar prop names for images imgSrc, imgAlt(html element + attribute) (needs audit)
  • We have a story for each component prop and we use the prop name verbatim e.g. size prop would be export const Size = (args) => (
  • Are multiple props stories allowed? e.g. Color, Background Color And Border Color story in base-avatar - [ ] yes when it makes sense to
  • We have the accompanying documentation for each component prop and we use the prop name verbatim e.g. size prop would be ### Size
  • All Base components follow the suffix convention e.g. ButtonBase
  • All Base component MDX documentation have the base component notification at the top
  • Add mm- prefix to all classNames
  • className is kebab case version of the component name
  • Spread base components props and reduce duplication of props when props aren't being changed and remain the same for both variant and base components
  • Remove Component.displayName not needed
  • Add component to root index.js file in component-library
  • Add locals for any default text I18nContext as default context
  • Add any "to dos" with a // TODO: comment so we can search for them at a later date e.g. blocking components etc
  • Add snapshot testing
  • Add pixel values to propType descriptions if we use abstracted prop types that relate to pixel values e.g. SIZE.MD (32px)

More Information

Fixes: #16180

Manual Testing Steps

yarn test:unit:jest .ui/components/component-library/tag/tag.test.js

Pre-Merge Checklist

  • PR template is filled out
  • IF this PR fixes a bug, a test that would have caught the bug has been added
  • PR is linked to the appropriate GitHub issue
  • PR has been added to the appropriate release Milestone

+ If there are functional changes:

  • Manual testing complete & passed
  • "Extension QA Board" label has been applied

@NidhiKJha NidhiKJha requested a review from a team as a code owner November 2, 2022 14:07
@NidhiKJha NidhiKJha requested a review from ryanml November 2, 2022 14:07
@NidhiKJha NidhiKJha requested review from garrettbear and georgewrmarshall and removed request for ryanml November 2, 2022 14:09
@NidhiKJha NidhiKJha added team-design-system All issues relating to design system in Extension IA/NAV labels Nov 2, 2022
@metamaskbot
Copy link
Collaborator

Builds ready [44d83a5]
Page Load Metrics (2407 ± 144 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint962224226459220
domContentLoaded171927702372286137
load178229062407300144
domInteractive171927702372286137

highlights:

storybook

Copy link
Contributor

@georgewrmarshall georgewrmarshall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one! 🔥

@NidhiKJha NidhiKJha merged commit 58da1d8 into develop Nov 3, 2022
@NidhiKJha NidhiKJha deleted the fix-16180 branch November 3, 2022 20:18
@github-actions github-actions bot locked and limited conversation to collaborators Nov 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
team-design-system All issues relating to design system in Extension
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🧹 [UI House Keeping] Tag
4 participants