-
Notifications
You must be signed in to change notification settings - Fork 162
898 #920
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
898 #920
Conversation
I have created this draft PR, the code currently is working for line and pie chart. I am working to make it work for treemaps also. |
Hello, do you have any updates on this PR? |
Thank you for reaching out! I wanted to provide an update, I’ve been in the middle of my exams, so I haven’t been able to give this PR my full attention. My exams will be over by March 8, and I’m looking forward to resuming work on it soon after. |
No worries, just confirming that you are still working on this. This PR is not urgent. Good luck with your exams. |
Hello @Ronit0104123, sorry for pinging, but we are planning to have a new release soon. I want to check if there is any progress on this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please address my inline comments. Thanks.
default: [ | ||
'#1f77b4', '#ff7f0e', '#2ca02c', '#d62728', '#9467bd', | ||
'#8c564b', '#e377c2','#7f7f7f','#bcbd22', '#17becf' | ||
], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Delete these lines. We do not need them as the colors are reset in lines 178-180.
I was working on ensuring adjacent nodes have distinct colors, but due to different screen sizes, their positions change dynamically. I also noticed that trace.ids is not arranged in ascending order of percentages but appears randomly, which further affects color distribution. I see two possible approaches (1) Expanding the color palette for each CVD type or (2) Modifying the implementation so that once all colors are used, the next cycle uses lighter shades of the same colors. However, I’m unsure if lighter shades would still be distinguishable for people with color vision deficiencies. Would love to hear your thoughts on this. |
If using lighter shades of the same colors does not create conflicts, then we should do this. Otherwise, we have to expand the color palette. To check palette conflicts, you can use the following link: https://projects.susielu.com/viz-palette |
The lighter shades of the same colors conflict, so I am thinking of expanding the color palette. I am finding the right non-conflicting colors for each CVD using a viz-palette. |
When I created the proposed color palettes, I used the following resource: I recommend using the same one to select the rest of the color codes. After selecting them, you can then use the viz-palette tool to check for conflicts. If it is impossible to avoid conflicts entirely, then I think it will be okay to have a few minor color conflicts. Thanks. |
Okay. Yes, will need to select some colors with minor color conflicts. How many colors for each CVD must I add to the palette? |
I guess we need around 10 more colors in each palette. Be sure to keep the same cycle form for the colors, like the current palette, for example:
|
displayModeBar: true, | ||
modeBarButtonsToAdd: [{ | ||
name: 'cvd-dropdown', | ||
title: 'Toggle CVD Colors', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace title: 'Toggle CVD Colors',
with: title: 'CVD Palette',
modeBarButtonsToAdd: [{ | ||
name: 'cvd-dropdown', | ||
title: 'Toggle CVD Colors', | ||
icon: Plotly.Icons.pencil, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace icon: Plotly.Icons.pencil,
with:
icon: {
svg: '<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 512 512"><!--!Font Awesome Free 6.7.2 by @fontawesome - https://fontawesome.com License - https://fontawesome.com/license/free Copyright 2025 Fonticons, Inc.--><path d="M512 256c0 .9 0 1.8 0 2.7c-.4 36.5-33.6 61.3-70.1 61.3L344 320c-26.5 0-48 21.5-48 48c0 3.4 .4 6.7 1 9.9c2.1 10.2 6.5 20 10.8 29.9c6.1 13.8 12.1 27.5 12.1 42c0 31.8-21.6 60.7-53.4 62c-3.5 .1-7 .2-10.6 .2C114.6 512 0 397.4 0 256S114.6 0 256 0S512 114.6 512 256zM128 288a32 32 0 1 0 -64 0 32 32 0 1 0 64 0zm0-96a32 32 0 1 0 0-64 32 32 0 1 0 0 64zM288 96a32 32 0 1 0 -64 0 32 32 0 1 0 64 0zm96 96a32 32 0 1 0 0-64 32 32 0 1 0 0 64z"/></svg>'
},
I added two more inline comments regarding the icon. |
I’ve created this color palette for Protanopia, ensuring minimal color conflicts. I tested many other colors but they were conflicting for larger areas. Would this palette work effectively for Protanopia? I'd appreciate your feedback on any improvements or adjustments needed. |
I checked the colors, and this conflict level is acceptable. I refined them and reordered them, and you can find them here:
You can continue with the other two categories. For deuteranopia, you can use the viz-report again. For tritanopia, you will have to do it empirically using the following tool: https://www.tools366.com/tool/color-blindness-converter. If anything is unclear, please let me know. Thanks. |
If you have to reorder protanopia palette (the one I provided above) to avoid cases where blocks that are next to each other can have the same code color, feel free to do it. |
Hi, For Deuteranopia - viz-palette for deuteranopia For Tritanopia,
I haven't finalized the order yet but wanted to confirm first if the color conflicts are acceptable. Let me know your thoughts! |
Hello, I checked the colors, below I have refined them, but they are unordered. Please adjust the order.
Thank you. |
Hi,
Let me know if any further refinements are needed. Thanks! |
Ok, you can proceed with the code implementation. Thanks. |
I've submitted the PR. Please have a look when you get a chance. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
Description
Related issue
#898
Screenshots (if appropriate):
Types of changes
Checklist: