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

[core] feat: darken dark theme colors #6711

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

adidahiya
Copy link
Contributor

@adidahiya adidahiya commented Feb 14, 2024

Supersedes #6708 and #6707

Checklist

  • Includes tests
  • Update documentation

Changes proposed in this pull request:

Update dark theme background colors to be one shade darker for many components based on the following proposal:

image

Reviewers should focus on:

Screenshot

@adidahiya
Copy link
Contributor Author

[core] feat: darken dark theme colors

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

@adidahiya adidahiya marked this pull request as ready for review February 14, 2024 17:57
@adidahiya
Copy link
Contributor Author

adjust more component colors

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

@adidahiya
Copy link
Contributor Author

some components which probably need to be updated for the new colors (or at least their docs examples):

PanelStack

image

ProgressBar

image

Spinner

image

Navbar

image

Switch

image

Table2

image

@adidahiya
Copy link
Contributor Author

notes from sync with @jessperrin:

  • many components deserve to be wrapped in a card ("elevated background color") in the docs example so they're not presented on $black ("secondary background color")
    • however, some should have "primary background color" (not elevated) since they'll likely be used in a sidebar panel (like Tree)
  • let's incorporate the default tag background color change in this PR
  • let's add a border to inline <code> elements (same as I just added for code blocks)
  • consider adding documentation around app layout and which colors (app background, secondary, elevated) apply to which UI elements - possibly fix Doc Suggestion: Move Dark Theme from Typography to Colors, Classes, or Variables #4750
  • open questions / specific components to pay attention to
    • editable table with empty space: image

@adidahiya
Copy link
Contributor Author

adidahiya commented Feb 15, 2024

I incorporated most of the suggestions from my last comment.

Not done yet:

  • I didn't update table colors - I think that will require a closer look / more involved change (since it might prompt us to use light borders on tables).
  • I did not move the "Dark theme" docs section yet.
  • I did not update default tag colors yet.

@adidahiya
Copy link
Contributor Author

improve appearance and layout of many examples

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

@adidahiya
Copy link
Contributor Author

update tag & compound tag colors

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

@adidahiya
Copy link
Contributor Author

I updated tag colors. any more feedback @jessperrin @CPerinet ?

@adidahiya
Copy link
Contributor Author

render dark background for segmented controls in docs

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

@adidahiya
Copy link
Contributor Author

fix panel stack tests

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

@adidahiya
Copy link
Contributor Author

Discussed with @ericjeney and we're going to hold off on this PR until the current set of unreleased changes is released as a minor version.

These visual changes will need to be carefully landed in our internal monorepos, since we imagine that there will be many instances of dark theme color variables which may need to be updated to deal with the darker shades in this change.

In the meantime, we should introduce new color aliases to promote a smoother migration from current colors to new ones:

$pt-app-secondary-background-color: $white !default;
$pt-dark-app-secondary-background-color: $dark-gray1 !default; // changing to $black in this PR

$pt-app-elevated-background-color: $light-gray4 !default;
$pt-dark-app-elevated-background-color: $dark-gray3 !default; // changing to $dark-gray2 in this PR

^ these color aliases should be available in some version of Blueprint before the one with this PR.

@adidahiya
Copy link
Contributor Author

@ericjeney I think we're ready to merge this now, could you give a +1 and merge it at some point this week?

@ericjeney
Copy link
Contributor

I think we should hold off on this one until we have a crisper plan for rollout and have a better sense how many hard-coded colors this will affect.

@adidahiya adidahiya removed the request for review from jessperrin February 29, 2024 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants