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.

Sign up for GitHub

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

Jump to bottom

chore: updating doc components and helpers #706

Merged
merged 1 commit into from
May 29, 2024
Merged

chore: updating doc components and helpers #706

merged 1 commit into from
May 29, 2024

Conversation

georgewrmarshall
Copy link
Collaborator

@georgewrmarshall georgewrmarshall commented May 27, 2024 •
edited
Loading

Description

This PR updates Storybook documentation components and helpers. No tokens are affected in this update; only the documentation components and helpers have been modified. The purpose of these changes is to separate non-breaking changes from PR #698 for easier review and to facilitate the comparison of token changes.

  • Updates ColorSwatchs to use black/white text dependent on tone
  • Fixes bug with displaying network colors in JS tokens
  • Adds white and black color JSON
  • Breaks out logic from UI components into helper functions

Related issues

Fixes: N/A

Manual testing steps

  1. Open Storybook.
  2. Navigate through brand color and theme stories
  3. Verify that the changes reflect correctly and that no tokens have been affected.

Screenshots/Recordings

Before

before720.mov

After

after720.mov

Pre-merge author checklist

  • I’ve followed MetaMask Coding Standards.
  • I've clearly explained what problem this PR is solving and how it is solved.
  • I've linked related issues
  • I've included manual testing steps
  • I've included screenshots/recordings if applicable
  • I’ve included tests if applicable
  • I’ve documented my code using JSDoc format if applicable
  • I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.
  • I’ve properly set the pull request status:
    • In case it's not yet "ready for review", I've set it to "draft".
    • In case it's "ready for review", I've changed it from "draft" to "non-draft".

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

@georgewrmarshall georgewrmarshall added the team-design-system All issues relating to design system label May 27, 2024
@georgewrmarshall georgewrmarshall self-assigned this May 27, 2024
georgewrmarshall
georgewrmarshall commented May 27, 2024
View reviewed changes
docs/BrandColors.stories.tsx Outdated
@@ -22,6 +26,7 @@ type Story = StoryObj<typeof ColorSwatchGroup>;
export const Figma: Story = {
render: () => {
const { brandColor } = useJsonColor();
console.log(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.

Let's retain console logs in the stories while we are working on these updates I find it's quite helpful. Maybe we can change the console logs to

Suggested change
console.log(brandColor);
console.log('brandColor JSON', brandColor);

@georgewrmarshall georgewrmarshall force-pushed the update/doc-components branch 2 times, most recently from a6865ed to f7496ef Compare May 27, 2024 22:12
georgewrmarshall
georgewrmarshall commented May 27, 2024
View reviewed changes
docs/BrandColors.stories.tsx
@@ -22,6 +26,7 @@ type Story = StoryObj<typeof ColorSwatchGroup>;
export const Figma: Story = {
render: () => {
const { brandColor } = useJsonColor();
console.log('brandColor JSON', 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.

Going to retain console logs until we finish the brand evolution work it's quite hepful

@metamaskbot
Copy link
Collaborator

Builds ready [f7496ef]

Storybook: Storybook

georgewrmarshall
georgewrmarshall commented May 27, 2024
View reviewed changes
docs/utils/useJsonColor.ts
Comment on lines +95 to +97
if (theme[category]?.[key]) {
resolvedValue = parseColorValue(
theme[category][key].value,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There are a couple of TypeScript errors here that will need to be resolved in future
Type 'undefined' cannot be used as an index type

Screenshot 2024-05-27 at 3 13 46 PM

georgewrmarshall
georgewrmarshall commented May 27, 2024
View reviewed changes
src/figma/brandColors.json Outdated
Comment on lines 611 to 568
},
"white": {
"value": "#ffffff",
"type": "color",
"parent": "Brand Colors/v1 - current",
"description": ""
},
"black": {
"value": "#000000",
"type": "color",
"parent": "Brand Colors/v1 - current",
"description": ""
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Adding white and black tokens here to test the updates to useJsonColor hook

georgewrmarshall
georgewrmarshall commented May 27, 2024
View reviewed changes
docs/components/ColorSwatchGroup/ColorSwatchGroup.tsx
@@ -1,5 +1,6 @@
import React, { FunctionComponent } from 'react';
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Includes all the changes from #698. I think it would be good to get these into main so we can separate documentation UI updates from actual token color changes.

@georgewrmarshall georgewrmarshall marked this pull request as ready for review May 27, 2024 22:17
@georgewrmarshall georgewrmarshall requested a review from a team as a code owner May 27, 2024 22:17
@georgewrmarshall georgewrmarshall mentioned this pull request May 29, 2024
Merged
13 tasks
brianacnguyen
brianacnguyen previously approved these changes May 29, 2024
View reviewed changes
@georgewrmarshall georgewrmarshall merged commit 8ae1190 into main May 29, 2024
17 checks passed
@georgewrmarshall georgewrmarshall deleted the update/doc-components branch May 29, 2024 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brianacnguyen brianacnguyen brianacnguyen approved these changes

Assignees

@georgewrmarshall georgewrmarshall

Labels
team-design-system All issues relating to design system
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@georgewrmarshall @metamaskbot @brianacnguyen