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

Navigation sidebar border-top disappears on dark theme #8067

Closed
paulinashakirova opened this issue Oct 9, 2024 · 7 comments · Fixed by #8070
Closed

Navigation sidebar border-top disappears on dark theme #8067

paulinashakirova opened this issue Oct 9, 2024 · 7 comments · Fixed by #8070

Comments

@paulinashakirova
Copy link

Describe the bug
Border top of navigation side-bar is inconsistent between light and dark theme.

Impact and severity
Depends on whether this is a bug or is implemented this way by design.

To Reproduce
Steps to reproduce the behavior:

  1. Switch between dark and light themes

Expected behavior
The border, on dark theme, to be identical (lighter) to the right border of the side-bar, as it is on light theme.

Minimum reproducible sandbox
This can be reproduced in the documentation.

Screenshots
Image

Additional context
It seems that the code related to this issue comes from the /eui/packages/eui/src/components/header/header.styles.ts where there is a separate logic for both themes. In the DARK theme the backgroundColor is used to define the bottom border color, whereas for the default theme the defaultBorderColor is used.

There is also a comment about this difference.

/**
 * The `dark` header is (currently) a bit of a special case. We don't
 * actually want to use <EuiThemeProvider colorMode="dark"> inside it
 * because that will affect popovers and `SelectableSitewideTemplate`
 * as well, which we do not necessarily want to do (?)
 *
 * It's also possible that the dark header will go away or become unused
 * by Kibana in the near future, at which point we can remove this
 */

This issue was previously opened on Kibana's side, but I’d like to transfer the issue to EUI repo so that the team could check if this is intentional or if this is a bug.

@mgadewoll
Copy link
Contributor

mgadewoll commented Oct 9, 2024

ℹ The component in question here used for the navigation on Serverless is EuiCollapsibleNavBeta

The dark top border comes from the header component which has a dark bottom border.

Image

So either the header should always have a lighter bottom border or the flyout (I guess that's unlikely) or the EuiCollapsibleNavBeta would need to have a separate top border in the expected lighter shade.

Image

On non-serverless Kibana this is a bit less clear as the sidebar itself doesn't have a right border.

Image

@MichaelMarcialis do you have any additional thoughts on this?

cc @ek-so

@ek-so
Copy link
Contributor

ek-so commented Oct 9, 2024

The second part of the problem seems to be that the top bar itself in the dark mode has a wrong color, and looks cropped here:
Image

As soon as we change the color, it starts looking fine (imo):
Image

@cee-chen
Copy link
Contributor

cee-chen commented Oct 9, 2024

The "problematic" code is coming from here:

// Curated border color to fade into the shadow without looking too much like a border
// It adds separation between the header and flyout
const borderColor =
colorMode === 'DARK'
? shade(euiTheme.colors.emptyShade, 0.35)
: shade(euiTheme.border.color, 0.03);

It appears to have been an intentional design decision by Caroline in #3538. It's likely due to the how shadows work in dark vs light mode, and the fact that the border is not noticeable in light mode and but becomes very noticeable in dark mode:

Dark Dark After
Image Image

vs light mode, which has a nice mix of both:
Image

If we're fine having the border contrast/stand out from the shadow, this is a pretty easy fix/removal.

FWIW, when Elizabet was here, I believe she mentioned that our dark mode shades could use general improvement in that they don't play very well with shadows (it's harder to go darker than absolute black). I'm not sure if that's something the designers would be interested in generally taking a look at and improving.

@ek-so
Copy link
Contributor

ek-so commented Oct 10, 2024

Hey @gvnmagni @MichaelMarcialis @ryankeairns please share your thoughts! Like I said, I'm personally fine with "Dark after" that @cee-chen suggested above, at least for now.

As for the shadows and grey shades in general — that's right, we can't go darker than black in dark mode. We have almost all possible shades (namely 28, from a very light to a very dark grey) in the upcoming palette refresh, so I believe we can support any solution we want to introduce. We might even want to remove this shadow under a top bar entirely, coz we are generally moving into more "flat" direction. But for now I'd leave it as is and say, let border contrast with the shadow in dark mode, imo.

@gvnmagni
Copy link
Contributor

Absolutely fine to go with Dark After solution proposed to me 👍
Thank you for investigating this, I didn't know it would be so articulate

@cee-chen cee-chen added design decision and removed ⚠️ needs validation For bugs that need confirmation as to whether they're reproducible labels Oct 10, 2024
@cee-chen cee-chen self-assigned this Oct 10, 2024
@cee-chen
Copy link
Contributor

#8070

@MichaelMarcialis
Copy link
Contributor

I'm good with the "Dark After" approach as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants