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

Reorganize the color palette #43304

Merged
merged 3 commits into from
Jun 21, 2024
Merged

Reorganize the color palette #43304

merged 3 commits into from
Jun 21, 2024

Conversation

bl-nero
Copy link
Contributor

@bl-nero bl-nero commented Jun 20, 2024

This change puts together color combinations that are defined by the new design system. Most of these colors were already there, but scattered and disorganized. This change deprecates the old fields and introduces new ones that have text and background colors grouped together and are intended to serve as the source of truth.

This is a preparation step for the upcoming change that implements the new button system.

There are no visible changes expected.

Note: This change is coupled with https://github.com/gravitational/teleport.e/pull/4457, but (hopefully) not enough to break the build or tests.

@bl-nero bl-nero added the no-changelog Indicates that a PR does not require a changelog entry label Jun 20, 2024
@github-actions github-actions bot requested review from gzdunek and ravicious June 20, 2024 18:22
Copy link
Member

@ravicious ravicious left a comment

Choose a reason for hiding this comment

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

🚀

{word}
{i < arr.length - 1 ? '.' : ''}
{/*potential line break*/}
<wbr />
Copy link
Member

Choose a reason for hiding this comment

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

TIL <wbr /> exists.

web/packages/design/src/theme/themeColors.story.tsx Outdated Show resolved Hide resolved
@@ -87,13 +104,15 @@ export type ThemeColors = {
textDisabled: string;
bgDisabled: string;

/** @deprecated Use `interactive.solid.primary` instead. */
Copy link
Member

Choose a reason for hiding this comment

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

Oh, that is huge that this now works properly thanks to types in UI components.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, only in style functions, but the usage of colors in attributes is somehow limited to just a handful of cases.

web/packages/design/src/theme/utils/colorManipulator.js Outdated Show resolved Hide resolved
web/packages/design/src/theme/themes/types.ts Outdated Show resolved Hide resolved
@bl-nero bl-nero enabled auto-merge June 21, 2024 14:54
@bl-nero bl-nero added this pull request to the merge queue Jun 21, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 21, 2024
@bl-nero bl-nero added this pull request to the merge queue Jun 21, 2024
Merged via the queue into master with commit 37ee8bd Jun 21, 2024
36 checks passed
@bl-nero bl-nero deleted the bl-nero/colors branch June 21, 2024 16:00
bl-nero added a commit that referenced this pull request Oct 11, 2024
* Reorganize the color palette

* Update web/packages/design/src/theme/themeColors.story.tsx

Co-authored-by: Rafał Cieślak <[email protected]>

* review

---------

Co-authored-by: Rafał Cieślak <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Oct 18, 2024
* Reorganize the color palette (#43304)

* Reorganize the color palette

* Update web/packages/design/src/theme/themeColors.story.tsx

Co-authored-by: Rafał Cieślak <[email protected]>

* review

---------

Co-authored-by: Rafał Cieślak <[email protected]>

* Get rid of unnecessary text colors (#47458)

* wip

* wip

* Remove the background field

* Remove the type

* Post-merge fix

---------

Co-authored-by: Rafał Cieślak <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-changelog Indicates that a PR does not require a changelog entry size/md ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants