Skip to content

Conversation

trallard
Copy link
Collaborator

@trallard trallard commented Nov 28, 2024

While working on the design system documentation, we noticed some fixes needed:

  • Fix heading colour definition: Use same colour as body
  • Add note about zindex: Closes Missing variable: zindex-tooltip #1953
  • Replace the undefined variable with PST variable: we were using --pst-color-panel-background for the background of the card (introduced in Add panels support #641, but it seems this was removed later and it was not properly replaced). Since cards are meant to give some sense of elevation, I replaced this with --pst-color-on-background, which mostly applies to the dark theme (see screenshots below)

Current colour:
image

In this PR:
image

light theme - it's the same before and after as we rely on shadows for elevation
image

cc @smeragoel

@trallard trallard added kind: bug Something isn't working tag: CSS CSS and SCSS related issues labels Nov 28, 2024
Copy link

Coverage report

This PR does not seem to contain any modification to coverable code.

@trallard trallard merged commit 25e01e2 into pydata:main Nov 28, 2024
25 checks passed
@trallard trallard deleted the trallard/patch-colour-vars branch November 28, 2024 16:46
@bollwyvl
Copy link
Collaborator

Hm, "broken" or no, --pst-heading-color (and similar) are used in a number of downstreams, so this likely constitutes a visible and breaking change. Not sure if there's a proper way to do it with a fallback...

@trallard
Copy link
Collaborator Author

Hey @bollwyvl, yeah, you are right; I should be able to add a patch to add a fallback or something like that since the previous --pst-heading-color did not adhere to our naming convention, but this would in practice constitute a user-facing breaking change.

@drammock WDYT?

@trallard
Copy link
Collaborator Author

Since this was a rather quick addition I opened a PR already at #2082

trallard added a commit that referenced this pull request Dec 17, 2024
Nick pointed in
#2058 (comment)
that the `--pst-heading-color` variable is used by folks to customise
the headings colour (this was the colour variable before
#2058 which brought
this in line with our naming convention `--pst-color-heading`).

This change effectively renders the use of `--pst-heading-color`
useless, so this PR adds a fallback mechanism for this variable.

I also added a note about `--pst-color-heading` being our preferred
variable.
This is not a "big" breaking change, but it will likely affect folks who
do not have their PST version pinned since I just made a release. So, it
might be worth publishing this change in a follow-up.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind: bug Something isn't working tag: CSS CSS and SCSS related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing variable: zindex-tooltip

3 participants