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

fix: update existing core color tokens #1905

Merged
merged 7 commits into from
Jul 11, 2024

Conversation

pixelflips
Copy link
Member

@pixelflips pixelflips commented Jul 9, 2024

Description

Adds new rebranding color tokens and updates current tokens to new values.

Screenshots

Before After
colors-before colors-after

Testing in sage-lib

Navigate to the colors page. Verify new colors are displayed.

Navigate to Docs Site and Storybook and check for any unexpected color changes.

Testing in kajabi-products

  1. (BREAKING) Adds new rebranding color tokens and updates current tokens to new values.
    • Full regression should be performed to ensure no unexpected color changes.

Related

https://kajabi.atlassian.net/browse/DSS-668

@pixelflips pixelflips self-assigned this Jul 9, 2024
@pixelflips pixelflips changed the base branch from develop to mercury-rising July 10, 2024 15:33
@pixelflips pixelflips marked this pull request as ready for review July 11, 2024 17:34
@pixelflips pixelflips requested a review from a team July 11, 2024 17:34
Copy link
Collaborator

@ju-Skinner ju-Skinner left a comment

Choose a reason for hiding this comment

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

The comment left is suggestive and NOT blocking.

value: "SageTokens.COLOR_PALETTE.#{color.upcase}_#{value}"
} %>
</div>
</div>
Copy link
Collaborator

@ju-Skinner ju-Skinner Jul 11, 2024

Choose a reason for hiding this comment

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

Small code refactor...could we make this another partial e.g _color_pallette_body.html.erb? This would allow it to be a simple <% render "color_palette_body", color: color, value: value %> in both areas to reduce duplicity.

Copy link
Member Author

Choose a reason for hiding this comment

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

@ju-Skinner great suggestion and definitely reduced the duplication. appreciate it!

@ju-Skinner ju-Skinner requested a review from a team July 11, 2024 18:20
@@ -73,5 +73,24 @@ Reserved for times when something important is missing or for irreversible or de
Used only when no other color option is available.
", use_sage_type: true) %>
<%= render "color_values", color: "purple"%>
</div>

<div id="deprecated">
Copy link
Collaborator

Choose a reason for hiding this comment

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

🙌🏻

@monicawheeler monicawheeler requested a review from a team July 11, 2024 18:29
@monicawheeler monicawheeler requested a review from a team July 11, 2024 18:29
@pixelflips pixelflips merged commit 10e7300 into mercury-rising Jul 11, 2024
3 checks passed
ju-Skinner pushed a commit that referenced this pull request Jul 23, 2024
* fix: update existing core color tokens

* fix: update existing core color tokens

* fix: updates to color function

* docs: implemented branding color changes

* style: implemented branding color changes

* style: refactored nesting to appease stylelint

* docs: created partial for colors to reduce code
ju-Skinner pushed a commit that referenced this pull request Jul 23, 2024
* fix: update existing core color tokens

* fix: update existing core color tokens

* fix: updates to color function

* docs: implemented branding color changes

* style: implemented branding color changes

* style: refactored nesting to appease stylelint

* docs: created partial for colors to reduce code
@ju-Skinner ju-Skinner added the mercury-rising Used for rebrand work label Aug 9, 2024
@pixelflips pixelflips deleted the fix/update-color-tokens branch August 14, 2024 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mercury-rising Used for rebrand work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants