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

Illegible text color in dark scheme for mermaid diagram #4517

Closed
5 tasks done
2bndy5 opened this issue Oct 20, 2022 · 5 comments · Fixed by #4518
Closed
5 tasks done

Illegible text color in dark scheme for mermaid diagram #4517

2bndy5 opened this issue Oct 20, 2022 · 5 comments · Fixed by #4518
Labels
bug Issue reports a bug resolved Issue is resolved, yet unreleased if open

Comments

@2bndy5
Copy link
Contributor

2bndy5 commented Oct 20, 2022

Contribution guidelines

I've found a bug and checked that ...

  • ... the problem doesn't occur with the mkdocs or readthedocs themes
  • ... the problem persists when all overrides are removed, i.e. custom_dir, extra_javascript and extra_css
  • ... the documentation does not mention anything about my problem
  • ... there are no open or closed issues that are related to my problem

Description

If a picture is worth a thousand words, then please observe these snapshots from the mkdocs-material docs
image
But notice that the "post-it note" node is legible in the light scheme:
image

Expected behaviour

If the node's bg color is static, then I would expect the text color also be static and legible.

After playing with it in my browser (Firefox), I was able to achieve expected behavior by removing the .noteText > tspan specifier from

/* Sequence message, loop and note text */
.messageText,
.loopText > tspan,
.noteText > tspan {
font-family: var(--md-mermaid-font-family) !important;
fill: var(--md-mermaid-edge-color);
stroke: none;
}

Actual behaviour

See description

Steps to reproduce

Whatever the docs CI is doing.

Package versions

I'm merging updates from here (v8.5.6) into sphinx-immaterial and I noticed this in our rendered docs (local only for now). After checking with the docs here, I found the same problem.

Configuration

See this repo's mkdocs config.

System information

  • Operating system: Whatever the docs CI is using.
  • Browser: Firefox
@squidfunk
Copy link
Owner

Thank for reporting. If you're dissecting my code to incorporate into another project, a PR is very much appreciated. Note that .nodeText > tspan was set for a reason, so it's probably a better idea to override text color specifically for notes.

Also, please make sure that all other diagrams still work in light and dark modes.

@squidfunk squidfunk added the bug Issue reports a bug label Oct 20, 2022
@2bndy5
Copy link
Contributor Author

2bndy5 commented Oct 20, 2022

Sorry the rendered CSS is a bit different from the src CSS, and I just searched for the .noteText in the repo and pointed to the only hit in src/... I'll keep digging and try to submit a PR soon.

I prefer to only merge updates from stable releases, but if we can get the fix for this from this repo's main branch, then I might break that tradition for just this time.

@squidfunk
Copy link
Owner

Thanks for your help! You need to set up the theme development process to preview your changes, but given that you use this project downstream, it should be a good idea anyways.

I prefer to only merge updates from stable releases, but if we can get the fix for this from this repo's main branch, then I might break that tradition for just this time.

I'm not sure what you mean by stable releases, because our master off which all releases are created can be considered stable. Yes, there will always be bugs, sometimes regressions, but I try to fix them as fast as possible 😅

@2bndy5
Copy link
Contributor Author

2bndy5 commented Oct 20, 2022

Yeah, the workflow downstream is pretty much the same.

I'm not sure what you mean by stable releases, because our master off which all releases are created can be considered stable. Yes, there will always be bugs, sometimes regressions, but I try to fix them as fast as possible 😅

Oh, good to know. I'll still try to focus only on tagged commits (my interpretation of "stable releases"), so it would be easier to help diagnose bug reports (if/when we get them downstream). That way your changelog/release description(s) is more useful 😄

@squidfunk
Copy link
Owner

Released as part of 8.5.8.

@squidfunk squidfunk added the resolved Issue is resolved, yet unreleased if open label Nov 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue reports a bug resolved Issue is resolved, yet unreleased if open
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants