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

Refactoring the brand color object #585

Merged
merged 2 commits into from
Jan 24, 2024
Merged

Conversation

georgewrmarshall
Copy link
Collaborator

@georgewrmarshall georgewrmarshall commented Jan 18, 2024

Description

This PR introduces an enhancement to the brandColors object that was recently added to our codebase. Although the initial implementation was not released, we identified a few areas for improvement to make the object more intuitive and compatible with developer tools.

The original syntax, brandColors.white['000'], had a couple of limitations. It didn't work optimally with intellisense, and the usage wasn't as straightforward as we desired.

After further discussions and considerations, we decided to make two key changes:

  1. Rename brandColors to brandColor: This change aligns with our convention of using singular names for types. It also makes the object name more intuitive when accessing individual colors.

  2. Adjust the object structure: Instead of accessing colors with brandColor.white['000'], we've flattened the structure to allow for more straightforward access, like brandColor.white000.

These changes offer several benefits:

  • Improved compatibility with autocomplete and intellisense tools, making coding more efficient.
  • Better adherence to our naming conventions, leading to more consistent code.
  • More intuitive color access, enhancing readability and ease of use.

Related: #581

Before

Video shows 2 preivous options that we were exploring. The first displays it not working well with intellisense

autocomplete.mov

After

Below screencast shows new JS token story

after.tokens.mov

Below screencast shows iframe of JS token story in comparison to the Figma json story

after720.mov

@georgewrmarshall georgewrmarshall self-assigned this Jan 18, 2024
@georgewrmarshall georgewrmarshall added the team-design-system All issues relating to design system label Jan 18, 2024
@metamaskbot
Copy link
Collaborator

Builds ready [5cc9329]

Storybook: Storybook

lightTheme,
darkTheme,
Theme,
brandColor,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Including brandColor export from root index. Changed formatting with extra export

@georgewrmarshall georgewrmarshall marked this pull request as ready for review January 23, 2024 22:42
@georgewrmarshall georgewrmarshall requested a review from a team as a code owner January 23, 2024 22:42
@@ -0,0 +1,130 @@
export interface BrandColor {
/** white/white000: #ffffff */
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These comments align with the Figma naming that engineers see in Figma and on hover of the object will also see the hex value

figma.mov

@brianacnguyen brianacnguyen merged commit 9bb60a2 into main Jan 24, 2024
7 checks passed
@brianacnguyen brianacnguyen deleted the fix/brand-color-update branch January 24, 2024 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-design-system All issues relating to design system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants