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

Improve shadows across components #1557

Closed
2 tasks done
smeragoel opened this issue Nov 9, 2023 · 9 comments
Closed
2 tasks done

Improve shadows across components #1557

smeragoel opened this issue Nov 9, 2023 · 9 comments
Assignees
Labels
tag: CSS CSS and SCSS related issues

Comments

@smeragoel
Copy link

smeragoel commented Nov 9, 2023

While working on the Tabbed Interfaces (#1555) redesign, based on @gabalafou and @12rambau's discussions, it was noticed that the shadows in dark mode are barely visible. This issue also affects other components, such as the admonitions.

Proposed Tasks

  • Reworking dark mode shadows so they stand out more
  • Defining standard shadow colours for light mode and dark mode
@12rambau
Copy link
Collaborator

12rambau commented Nov 9, 2023

I think there is a misunderstanding of my last message. To me there should be NO shadow in a dark theme but only color changes to convey elevation and I had set them to transparent last time I checked the colors.

@trallard
Copy link
Collaborator

Echoing what @12rambau said - the shadows in dark themes seem a tad redundant (and many sources recommend using colour mostly/only to denote depth). So, it would be best to keep this pattern to indicate depth (canvas, on-bg, surface). https://pydata-sphinx-theme.readthedocs.io/en/stable/user_guide/styling.html is #1555 (so the active tab might be best with the surface colour).

However, what I did in the rework was to darken the shadows and make them a bit sharper.
The previous ones made everything look very muddy https://pydata-sphinx-theme.readthedocs.io/en/v0.12.0/index.html (defined in the Figma file)

@trallard trallard added the tag: CSS CSS and SCSS related issues label Nov 13, 2023
@drammock
Copy link
Collaborator

FWIW I like shadows in dark mode. Our background is not fully black, and (on my screen at least) it is possible to have a dark drop shadow successfully create the visual impression of layering/stacking on our dark BG. So IMO if (for a11y reasons) we need to rely on something besides shadows in dark mode, that doesn't to me imply that we cannot / should not also include a drop shadow.

(NB: this is a preference / nitpick, not something I will fight hard for popular opinion falls the other way)

@trallard
Copy link
Collaborator

Note I am not suggesting dropping the shadows - there are already shadows in the theme (and design system) we should ensure all components are using those unified shadows and continue to use the colours to indicate elevation too (in the mentioned PR above for example).

I am unsure if the shadows should indeed be more prominent (darker perhaps?) in dark mode - see https://pydata-sphinx-theme.readthedocs.io/en/stable/examples/kitchen-sink/admonitions.html
I think these look good as they are but it is also not a hill I am willing to die on as there is no direct accessibility implication on having a lighter or darker shadow here.

@gabalafou
Copy link
Collaborator

A couple observations:

  • In dark mode, I cannot perceive the current shadow color (0, 0, 0, 0.2) unless it's against a low-contrasting adjacent color. So for example, I cannot perceive the shadow on the topic admonition, but I can on the regular admonition:
    screenshot: topic versus regular admonition

  • One problem with coloring the active tab with the surface color is that it loses continuity with the body of the tab, which unless I'm mistaken, is an integral part of @smeragoel's designs:

    using background color using surface color
    image image

@12rambau
Copy link
Collaborator

but is it a problem to use the normal background without shadow?
I find the underlying more than sufficient to convey the focus state isn't it ?

@drammock
Copy link
Collaborator

I am having trouble keeping this all straight. Here is an attempt to hopefully clarify (maybe only useful to me):

Shadows

  • there is a11y guidance that for elevation/layering cues we should (mostly) rely on color.
  • We have "surface" and "on-background" colors defined for this purpose.
  • Some elements (like admonition boxes) have shadows that provide a redundant/extra layering cue, some elements (like code blocks) do not.
    • This doesn't seem to cause problems and I don't recall any complaints about it (please correct me if I'm being forgetful here)
  • In light mode, the elements that have shadows have always had them.
  • At some point @12rambau eliminated the shadows in dark mode. I think the big color overhaul brought it back? Personally I think the shadow should be there in both light and dark mode; it doesn't hurt anything and we are already not relying on it for depth cues (we have color variables for that, mentioned above).

Tabs

The shadows discussion came up in context of the tab redesign. I think the main issue is that the tabs aren't using "on-bg" nor "surface" colors, so they're in some sense violating the depth design that we follow elsewhere. The separation is still being achieved via a high-contrast border around the tab.

So for the purposes of this issue can we all agree that

  1. shadows in dark mode are OK as long as we also keep using the "surface" and "on-bg" colors?
  2. discussion of the tab design can stay over in Restyle Sphinx Design tabs #1555?

@smeragoel
Copy link
Author

@trallard, @gabalafou and I got on a call to discuss this issue and this is what we came up with:

We keep the shadows in dark mode as part of a two-layer (shadow and colour) approach to conveying depth in dark mode. We will darken them sligthly to rgba(0, 0, 0, 0.5) so that they are slightly more perciptible, but not too harsh and in-your-face.

For the tabs, I'll conform them to the depth design followed by other components.

@trallard
Copy link
Collaborator

trallard commented Jul 1, 2024

This has already been addressed within a number of PRs so will go ahead and close

@trallard trallard closed this as completed Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tag: CSS CSS and SCSS related issues
Projects
None yet
Development

No branches or pull requests

5 participants