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

demo css dynamic colour palette #210

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

scottgigante-immunai
Copy link
Collaborator

@scottgigante-immunai scottgigante-immunai commented Apr 11, 2023

Describe your changes

Issue ticket number and link

Closes #176

Checklist before requesting a review

  • I have performed a self-review of my code

  • Check the correct box. Does this PR contain:

    • Breaking changes
    • New functionality
    • Major changes
    • Minor changes
    • Bug fixes
  • Proposed changes are described in the CHANGELOG.md

  • CI Preview succeeds and looks good!

@github-actions
Copy link

github-actions bot commented Apr 11, 2023

Deploy: success

@scottgigante-immunai scottgigante-immunai changed the base branch from main to revert-199-docs/v2/update April 11, 2023 19:01
@mxposed
Copy link
Contributor

mxposed commented Apr 19, 2023

Hi Scott, I added simpler selectors, and then I also changed the stroke color rule: I moved var(--bs-dark) to the funkyheatmap call as a parameter, instead of overriding it via css. But maybe I missed something with it that you were targeting specifically

@scottgigante-immunai scottgigante-immunai changed the base branch from revert-199-docs/v2/update to main April 19, 2023 19:18
@scottgigante-immunai
Copy link
Collaborator Author

Thanks @mxposed ! This is much cleaner.

Copy link
Contributor

@mxposed mxposed left a comment

Choose a reason for hiding this comment

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

LGTM

@scottgigante-immunai
Copy link
Collaborator Author

@rcannood happy to merge?

@rcannood
Copy link
Member

Is this the intended look? How does it work -- does it invert the available palettes?

Screenshot from 2023-04-21 15-53-11

I was really excited to open up this PR and be able to merge it, but looking at I just can't bring myself to saying it's an improvement :P

I still think we simply need two separate palettes, one optimized for a light theme and one optimized for a dark one. I wouldn't invert the palette direction, but rather tone down the brightness (or shift the palette) to work well with a dark theme.

Mind if I make a hard coded example?

@KaiWaldrant
Copy link
Member

I agree with @rcannood although it looks a lot better for dark mode it is just not there yet.

@scottgigante-immunai
Copy link
Collaborator Author

This is why I asked for input :) We can tone down the brightness if you like -- you just have to tell me what you want

@scottgigante-immunai
Copy link
Collaborator Author

Another possible alternative -- I just reduced the brightness and left everything else the same. @rcannood @KaiWaldrant @mxposed thoughts? suggestions?

image

@KaiWaldrant
Copy link
Member

Looks perfect to me

@rcannood
Copy link
Member

rcannood commented May 5, 2023

I explored a dark color palette here. It uses the original ColorBrewer palettes to shift the palette rather than darkening the existing palettes. I'd propose switching between two different palette sets based on the current theme.

Previous:

Proposal:

@scottgigante-immunai @mxposed Does this look visually appealing to you?

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.

funkyheatmap dark mode
4 participants