design-system icon indicating copy to clipboard operation
design-system copied to clipboard

Standardize type declarations for existing components

Open didoo opened this issue 1 year ago • 2 comments

:pushpin: Summary

Per discussion and agreement today, I've reviewed components already converted to TypeScript to follow as much as possible the patterns we have agreed upon.

:hammer_and_wrench: Detailed description

In this PR I have:

Hds::Interactive - Aligned type declaration to agreed patterns

  • Updated class name (removed Index from the name)

Hds::Button - Aligned type declaration to agreed patterns

  • Updated class name (removed Index from the name)
  • Updated “Component API” documentation
  • Sorted signature arguments to match documentation

Hds::Alert - Aligned type declaration to agreed patterns

  • Updated class name (removed Index from the name)

Hds::Badge - Aligned type declaration to agreed patterns

  • Updated class name (removed Index from the name)
  • Updated “Component API” documentation to be consistent with Button and BadgeCount
  • Sorted signature arguments to match documentation

Hds::BadgeCount - Aligned type declaration to agreed patterns

  • Updated class and signature names (removed Index from the names)
  • Updated “Component API” documentation to match signature and be consistent with Button and Badge

Hds::Card::Container - Aligned type declaration to agreed patterns

  • Sorted signature arguments to match documentation

Hds:DisclosurePrimitive - Updated documentation

  • Added missing isOpen argument to the "Component API" section

Hds::DismissButton - Aligned type declaration to agreed patterns

  • Updated class name (removed Index from the name)

Hds::IconTile - Aligned type declaration to agreed patterns

  • Updated class name (removed Index from the name)
  • Updated “Component API” documentation
  • Sorted signature arguments to match documentation

Hds::Link::Inline - Aligned type declaration to agreed patterns

  • Remove extra args (they are already provided by the Interactive signature)

Hds::Link::Standalone - Aligned type declaration to agreed patterns

  • Updated “Component API” documentation to be consistent with Link::Inline and Badge/BadgeCount
  • Sorted signature arguments to match documentation
  • Remove extra args (they are already provided by the Interactive signature)

Hds::Tag - Aligned type declaration to agreed patterns

  • Updated class and signature names (removed Index from the names)
  • Updated “Component API” documentation to match signature and be consistent with Button, Badge, etc.

Hds::Text - Aligned type declaration to agreed patterns for text components

  • Updated class and signature names (removed Index from the names)
  • Sorted signature arguments to match documentation

👀 Component checklist

  • [ ] Percy was checked for any visual regression
  • [x] A changelog entry was added via Changesets if needed (see templates here)

:speech_balloon: Please consider using conventional comments when reviewing this PR.

didoo avatar May 16 '24 18:05 didoo

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
hds-showcase ✅ Ready (Inspect) Visit Preview May 17, 2024 10:27am
hds-website ✅ Ready (Inspect) Visit Preview May 17, 2024 10:27am

vercel[bot] avatar May 16 '24 18:05 vercel[bot]

@aklkv @natmegs @WenInCode I've added as reviewers to this PR because I likely touched code you implemented, so... feel free to review or not, as you prefer (don't feel obliged to).

didoo avatar May 16 '24 18:05 didoo

I see that in this PR we change both the argument order in signatures (from alphabetical to match the order we currently have in the website) and the order of the arguments in the website, which makes the order consistent but arbitrary. If we are to use this pattern going forward would you be able to document/articulate the rules for sorting arguments so we can have it as a reference?

@alex-ju as you say, it's somehow arbitrary. What I've tried to do is to use other similar components as reference, and follow the same (or at least a similar) order. For example, I've noticed that Button, Badge/BadgeCount, IconTile, LinkInline/Standalone, Tag they all share some common props, like type, size, color, text, textSomething, icon, iconSomething so I tried to keep this order; most of the components have the ...attributes at the very bottom; for the links I have tried to keep the props coming from the Interactive all in the same block and order. I can write this down somewhere, where do you suggest to do it?

didoo avatar May 17 '24 10:05 didoo

I see that in this PR we change both the argument order in signatures (from alphabetical to match the order we currently have in the website) and the order of the arguments in the website, which makes the order consistent but arbitrary. If we are to use this pattern going forward would you be able to document/articulate the rules for sorting arguments so we can have it as a reference?

@alex-ju as you say, it's somehow arbitrary. What I've tried to do is to use other similar components as reference, and follow the same (or at least a similar) order. For example, I've noticed that Button, Badge/BadgeCount, IconTile, LinkInline/Standalone, Tag they all share some common props, like type, size, color, text, textSomething, icon, iconSomething so I tried to keep this order; most of the components have the ...attributes at the very bottom; for the links I have tried to keep the props coming from the Interactive all in the same block and order. I can write this down somewhere, where do you suggest to do it?

not something for this PR, more of a thing for our backlog to revisit

alex-ju avatar May 17 '24 10:05 alex-ju

not something for this PR, more of a thing for our backlog to revisit

would be OK to add something to the https://github.com/hashicorp/design-system/pull/2087 (I can add a suggestion) for the time being, then when the migration is complete we can review all the components-related documentation (there will be new engineering onboardings to do, it will be a work worth to do in advance)

didoo avatar May 17 '24 10:05 didoo

not something for this PR, more of a thing for our backlog to revisit

would be OK to add something to the #2087 (I can add a suggestion) for the time being, then when the migration is complete we can review all the components-related documentation (there will be new engineering onboardings to do, it will be a work worth to do in advance)

yes, that sounds good!

alex-ju avatar May 17 '24 10:05 alex-ju

yes, that sounds good!

https://github.com/hashicorp/design-system/pull/2087/files#r1604785808

didoo avatar May 17 '24 11:05 didoo