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

Support changing default node/background color #143

Open
2 tasks
fedarko opened this issue Mar 26, 2020 · 6 comments
Open
2 tasks

Support changing default node/background color #143

fedarko opened this issue Mar 26, 2020 · 6 comments
Assignees
Milestone

Comments

@fedarko
Copy link
Collaborator

fedarko commented Mar 26, 2020

Update: plan as I understand it for now is having the background color changing be part of the second alpha release, and having the default node color changing be part of a later release.

  • Support changing background color
  • Support changing default node color

Just a small thing that would be nice in the future. For example, drawing nodes black (rather than with the default gray color currently used) increases the contrast significantly:

image

image

This could be useful for e.g. vision-impaired folks.

Might be a nice TODO at some point down the line after #137?

@ElDeveloper
Copy link
Member

Whatever default we select, we should make both background color and tree color user-selectable.

@fedarko
Copy link
Collaborator Author

fedarko commented Mar 26, 2020

Agreed, having both selectable will be really nice. Just for reference, here's what a "dark mode" tree looks like in Empress ;) -- I imagine sticking with the black/gray on white default will be preferred by most users, but supporting this stuff (and #137) will make it really easy to style Emperor and Empress similarly.

image

@fedarko fedarko changed the title Support changing default node color Support changing default node/background color Mar 26, 2020
@ElDeveloper ElDeveloper mentioned this issue Mar 28, 2020
@ElDeveloper
Copy link
Member

Related to how we want to represent non-unique and non-represented features.

@fedarko
Copy link
Collaborator Author

fedarko commented Jun 24, 2020

One complication with this issue is that, if we change the default node color to black (which I think would be best IMO), then the root node's special black marker no longer stands out:

image

I think the way around this is changing the root's marker to be different in some other way (e.g. a square / star / unfilled circle / etc.), and maybe making the color of the root configurable as well.

@ElDeveloper
Copy link
Member

A different shape should work well 👍

fedarko added a commit to fedarko/empress that referenced this issue Jun 30, 2020
(Next up is making the default node color, and the background color,
configurable.)
@fedarko
Copy link
Collaborator Author

fedarko commented Jun 30, 2020

Did a bit more looking into this; it looks like Emperor uses Spectrum for letting users select colors, so I think for the sake of simplicity it would be good to just use that for this.

(I'd thought it might be possible for us to re-use the vendor JS code between Emperor and Empress in the tandem plots -- letting us avoid duplicating the files unnecessarily -- but it doesn't look like that is currently possible, and I suspect that would probably be more trouble than it's worth.)

fedarko added a commit to fedarko/empress that referenced this issue Jun 30, 2020
This messes with a few things (1. emperor's spectrum css, 2.
with any coloring done to the tree before changing the colors),
and I think the default node color is being used elsewhere... so
this is an early version of this. Still promising, though!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants