Skip to content

Conversation

@ze-flo
Copy link
Contributor

@ze-flo ze-flo commented Oct 9, 2024

Description

This PR updates various components in Chrome to use transient props where appropriate. These changes are necessary in preparation for the upgrade to [email protected] to ensure we avoid DOM violation errors after the transition.

Checklist

  • 👌 design updates will be Garden Designer approved (add the designer as a reviewer)
  • 🌐 demo is up-to-date (npm start)
  • ⬅️ renders as expected with reversed (RTL) direction
  • ⚫ renders as expected in dark mode
  • 🤘 renders as expected with Bedrock CSS (?bedrock)
  • 💂‍♂️ includes new unit tests. Maintain existing coverage (always >= 96%)
  • ♿ tested for WCAG 2.1 AA accessibility compliance
  • 📝 tested in Chrome, Firefox, Safari, and Edge

Copy link
Member

@jzempel jzempel left a comment

Choose a reason for hiding this comment

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

If this package is complete, then its peerDependencies can bump to:

"styled-components": "^5.3.1 || ^6.0.0"

Copy link
Member

@jzempel jzempel left a comment

Choose a reason for hiding this comment

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

fwiw, the most recent code convention for spread parameters exists in dropdowns where we simply used ...props naming (vs ...other or ...rest). I'm not opposed to ...other – just pointing to the most evolved convention 😉

@ze-flo
Copy link
Contributor Author

ze-flo commented Oct 11, 2024

fwiw, the most recent code convention for spread parameters exists in dropdowns where we simply used ...props naming (vs ...other or ...rest). I'm not opposed to ...other – just pointing to the most evolved convention 😉

When using ...other (or ...rest) instead of ...props, it's immediately clear that we're only passing along a subset of the props. This makes the code easier to understand and review because you don't have to jump to the component's definition to figure out what's being passed. Since we've mostly used ...other in our codebase, I'll stick with it for consistency.

@ze-flo ze-flo merged commit 28a0df2 into main Oct 15, 2024
8 checks passed
@ze-flo ze-flo deleted the ze-flo/transient-props-chrome branch October 15, 2024 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants