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

funkyheatmap dark mode #176

Open
scottgigante-immunai opened this issue Mar 10, 2023 · 10 comments · May be fixed by #210
Open

funkyheatmap dark mode #176

scottgigante-immunai opened this issue Mar 10, 2023 · 10 comments · May be fixed by #210
Assignees

Comments

@scottgigante-immunai
Copy link
Collaborator

          Can we discuss how you want the funkyheatmap to look with the night theme? I can adapt different elements and use css variables for styles

Originally posted by @mxposed in #172 (comment)

Rather than hacking the css, we can design the funkyheatmap to be responsive to colour variables.

Here's my two cents:

  • in dark mode, all text should be white and rows should alternate between black and dark grey
  • in light mode, all text should be black (except header which stays white) and rows should alternate between white and light grey

This could be achieved with --bs-white for header text, --bs-body-color for other text, --bs-body-bg for row background, --bs-button-hover for alternate rows background.

cc @rcannood @KaiWaldrant

@KaiWaldrant
Copy link
Member

that seems about right to me

@mxposed mxposed self-assigned this Apr 10, 2023
@mxposed mxposed mentioned this issue Apr 10, 2023
8 tasks
@mxposed
Copy link
Contributor

mxposed commented Apr 10, 2023

For the colour schemes, I think @rcannood had a plan to have 2 funkyheatmap cells with light and dark palettes and then to switch them via #200

@scottgigante-immunai
Copy link
Collaborator Author

Seems ugly... is there no better way to define the endpoints of your colour scheme in js?

@mxposed
Copy link
Contributor

mxposed commented Apr 10, 2023

It does, but the other way would be to make funkyheatmap aware of the light/dark mode, which is ugly too (because it's a separate component, and you can't just pass a style to alter the palette), or at least I haven't thought of a good way yet.

Any ideas are welcome

@scottgigante-immunai
Copy link
Collaborator Author

How do you create the component with a different palette in js?

@rcannood
Copy link
Member

Right now the palettes are decided on in r. To fix this, we need to either define two palettes in R and pass them both to JS, or move the code that decides the palette in JS.

Happy to discuss when I'm back from holidays next week.

@scottgigante-immunai
Copy link
Collaborator Author

I don't know how funkyheatmapJS interacts with the R code, but this seems eminently solvable with css.

@mxposed
Copy link
Contributor

mxposed commented Apr 12, 2023

I learn more and more about css : )

If the filter trick does the job and looks ok, then let's just use that and call it a day?

I wouldn't like moving palettes to css entirely, but I can add the css variable that determines palette position, and then we can add css to convert that to color.

@scottgigante-immunai
Copy link
Collaborator Author

If we use css filter, we should add a css class on all of the circles / rects so I can select them more easily (.quarto-figure svg g:first-child ~ g text ~ rect, .quarto-figure svg g:first-child ~ g text ~ circle is ugly). Beyond that, the demo I show above I think does a reasonable job. Open to input from others.

@mxposed
Copy link
Contributor

mxposed commented Apr 12, 2023

I'll add this to your branch, but also waiting for input from others

@scottgigante-immunai scottgigante-immunai linked a pull request Apr 19, 2023 that will close this issue
8 tasks
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 a pull request may close this issue.

4 participants