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

Use Algolia's dark theme when needed #7871

Merged
merged 3 commits into from
Feb 13, 2025

Conversation

zbynek
Copy link
Contributor

@zbynek zbynek commented Feb 12, 2025

Before #7869 there were 3 elements with poor contrast in dark mode and the popup looked fine in light mode. That PR fixed contrast for one of the elements in dark mode, which is an improvement, but comes at the cost of lower contrastin light mode. The two symbols marked in red remain barely isible:
image

I think the preferable approach here is to use Algolia's own dark styles in dark mode and keep light mode as is.

image

CC @biru-codeastromer

@zbynek zbynek requested a review from a team as a code owner February 12, 2025 22:02
@biru-codeastromer
Copy link
Contributor

biru-codeastromer commented Feb 12, 2025

Thanks for tagging me! I really appreciate you building on the fix from my PR (#7869). Your idea of using Algolia's own dark theme for better contrast looks fantastic. I also got to learn a lot from your changes, which will definitely help me moving forward. Thanks again for your effort and for sharing these insights!

Copy link
Contributor

@biru-codeastromer biru-codeastromer left a comment

Choose a reason for hiding this comment

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

One suggestion -

Do you think we can have a greyish border around the searchbox in dark theme ?
How about it ? Since I think it will make the searchbox a bit more distinctly looked on in the dark theme ;which is not present now unless we hover over it .

Copy link
Contributor

@biru-codeastromer biru-codeastromer left a comment

Choose a reason for hiding this comment

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

Also how about keeping the colour theme of the text in dark theme consistent with the other part of it by adding a CSS for it via

[data-theme="dark"] .DocSearch-NoResults p.DocSearch-Title {
  color: rgba(255, 255, 255, 0.634);
  font-weight: bold;
}

in jenkins.css

Screenshot 2025-02-13 at 4 54 52 AM

WDYT on this ?

Copy link
Contributor

@biru-codeastromer biru-codeastromer left a comment

Choose a reason for hiding this comment

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

LGTM now . Thanks very much !

content/css/jenkins.css Outdated Show resolved Hide resolved
Co-authored-by: Kris Stern <[email protected]>
@krisstern krisstern merged commit fdf0b5e into jenkins-infra:master Feb 13, 2025
6 checks passed
@krisstern
Copy link
Member

Thanks @zbynek!

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

Successfully merging this pull request may close these issues.

3 participants