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

Docs: Non-readable diagrams when the dark theme is on #16683

Closed
1 task done
myshov opened this issue Dec 20, 2022 · 15 comments · Fixed by #16822
Closed
1 task done

Docs: Non-readable diagrams when the dark theme is on #16683

myshov opened this issue Dec 20, 2022 · 15 comments · Fixed by #16822
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion documentation Relates to ESLint's documentation
Projects

Comments

@myshov
Copy link

myshov commented Dec 20, 2022

URL(s)

https://eslint.org/docs/latest/developer-guide/code-path-analysis

What did you do?

Turn on the dark theme

What did you expect to happen?

Diagrams are readable

What actually happened?

The black arrows of diagrams almost invisible on the dark background of the site

Participation

  • I am willing to submit a pull request for this issue.

Additional comments

As a workaround we can use filter: invert(100%); for the images when the dark theme is on

@myshov myshov added the bug ESLint is working incorrectly label Dec 20, 2022
@myshov myshov changed the title Bug: (fill in) Non-readable diagramw when the dark theme is on Bug: Non-readable diagrams when the dark theme is on Dec 20, 2022
@mdjermanovic mdjermanovic transferred this issue from eslint/eslint.org Dec 20, 2022
@mdjermanovic mdjermanovic added documentation Relates to ESLint's documentation and removed bug ESLint is working incorrectly labels Dec 20, 2022
@mdjermanovic mdjermanovic changed the title Bug: Non-readable diagrams when the dark theme is on Docs: Non-readable diagrams when the dark theme is on Dec 20, 2022
@mdjermanovic
Copy link
Member

Transferred to eslint/eslint, as the code path analysis document is in this repo.

@myshov
Copy link
Author

myshov commented Dec 20, 2022

@mdjermanovic hi! I believe that we have to change styles of the site itself too

@mdjermanovic
Copy link
Member

Styles for the docs pages are also in this repo. While at this, I'm not sure if we should change styles to invert colors or make sure that those images have a light background so that they look the same in both light and dark modes. @eslint/website-team thoughts?

@harish-sethuraman
Copy link
Member

My suggestions are

  • we can change the colors of the image's nodes that are not visible by editing the image
  • apply styles to those SVG elements and directly import it instead of using img tag? (so we can fill colors to specific node depending on the theme)
  • add common background color that works well with dark and light theme?

PS: we cant change the styles of the site itself?

@amareshsm
Copy link
Member

invert() won't work I guess.

light theme dark theme with invert property
image image

As @harish-sethuraman suggested we use SVG or add common background color that works well with both themes.

@kecrily
Copy link
Member

kecrily commented Dec 20, 2022

I like the idea of SVG themes, we can make a special id for them and then override them uniformly.

@myshov
Copy link
Author

myshov commented Dec 21, 2022

invert() won't work I guess.

at least it is more readable and the colors are distinguishable. But anyway I also like the idea with SVG. Btw SVG supports the media query prefers-color-scheme https://blog.tomayac.com/2019/09/21/prefers-color-scheme-in-svg-favicons-for-dark-mode-icons/

@harish-sethuraman
Copy link
Member

I'd like to hear @eslint/eslint-tsc's thoughts on this too. We might want to make it consistent going forward where these kind of UI differences occurs.

@nzakas
Copy link
Member

nzakas commented Dec 26, 2022

It's important to note that these SVG images are automatically generated through GraphViz, so any solution we choose should not involve manually editing the SVG.

I think the easiest solution would be to just set the background-color to white on all SVGs that are in the content of a documentation page.

@kecrily
Copy link
Member

kecrily commented Dec 27, 2022

If it's auto-generated, then maybe we can also consider the mermaid + light/dark theme option. Just a thought

@nzakas nzakas moved this from Needs Triage to Feedback Needed in Triage Dec 27, 2022
@nzakas
Copy link
Member

nzakas commented Dec 28, 2022

They are generated by hand right now. :)

@Tanujkanti4441
Copy link
Contributor

What about applying this background color ? It's quite similar with the whole background and arrows are also visible enough.

Annotation 2023-01-18 204544
Annotation 2023-01-18 205711

@nzakas
Copy link
Member

nzakas commented Jan 20, 2023

There's not enough contrast between the background and the black lines/dots.

I think we can just start by applying a white background to these diagrams?

@Tanujkanti4441
Copy link
Contributor

But instead of applying pure white in background can we apply colors that are more soothing to eyes in dark mode and same time have enough contrast with lines/dots
like this

Screenshot 2023-01-22 at 20-13-02 Code Path Analysis Details - ESLint - Pluggable JavaScript Linter
Screenshot 2023-01-22 at 20-15-38 Code Path Analysis Details - ESLint - Pluggable JavaScript Linter

Screenshot 2023-01-22 at 20-16-22 Code Path Analysis Details - ESLint - Pluggable JavaScript Linter

@nzakas
Copy link
Member

nzakas commented Jan 24, 2023

If we choose white then there’s no border in light mode at all.

If the website team wants to choose an off white color, they can. I’d just like to get this issue closed. :)

@mdjermanovic mdjermanovic linked a pull request Jan 26, 2023 that will close this issue
1 task
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Jul 26, 2023
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Jul 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion documentation Relates to ESLint's documentation
Projects
Archived in project
Triage
Feedback Needed
Development

Successfully merging a pull request may close this issue.

7 participants