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

Update Colors based on feedback #818

Merged
merged 4 commits into from
Aug 29, 2023
Merged

Update Colors based on feedback #818

merged 4 commits into from
Aug 29, 2023

Conversation

ataker
Copy link
Contributor

@ataker ataker commented Aug 25, 2023

Chromatic

https://dst1957-color-feedback--60f9b557105290003b387cd5.chromatic.com

Description

Per feedback:

  • Update warning-message to feedback-warning-background
  • Remove gibill-accent. After investigation, this is the same color as gold-lightest, updated only usages in vets-website with this PR.
  • Remove orange. After investigation, this is only used in storybook for the USE WITH CAUTION: PROPOSED label. Moved to storybook css.

Partial CSS Library: Generate a Figma token file #1957

Testing done

warning-message unused in component library, no testing.
gibill-accent unused in component library, no testing.
orange tested in storybook

Screenshots

Screenshot 2023-08-25 at 5 24 41 PM

Acceptance criteria

  • Remove/replace these colors

Definition of done

  • Documentation has been updated, if applicable
  • A link has been provided to the originating GitHub issue (or connected to it via ZenHub)

@ataker ataker marked this pull request as ready for review August 25, 2023 21:25
@ataker ataker requested a review from a team as a code owner August 25, 2023 21:25
$color-link-default: #004795;
$color-link-visited: #4c2c92;
$color-warning-message: #fac922;
$color-gibill-accent: #fff1d2;
$color-feedback-warning-background: #fac922;
Copy link
Contributor

@jamigibbs jamigibbs Aug 28, 2023

Choose a reason for hiding this comment

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

When we eventually change-over from Formation to CSS Library, I wonder how we will deal with this token name being changed. It's being used in a few places in Formation (and thus vets-website)

Copy link
Contributor

@Andrew565 Andrew565 left a comment

Choose a reason for hiding this comment

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

Given that color-warning-message doesn't appear to be in use anywhere, I'm good with this moving forward.

Nevermind, it's being used in a couple of places, like Jami pointed out. I only looked to see if it was being used in the component-library repo.

@ataker ataker added the ignore-for-release Used if you want to ignore the PR in the generated release notes label Aug 29, 2023
@ataker
Copy link
Contributor Author

ataker commented Aug 29, 2023

Next steps in this ticket

@ataker ataker merged commit 5f0566c into main Aug 29, 2023
@ataker ataker deleted the dst1957-color-feedback branch August 29, 2023 20:26
@jamigibbs jamigibbs mentioned this pull request Aug 31, 2023
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ignore-for-release Used if you want to ignore the PR in the generated release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants