-
-
Notifications
You must be signed in to change notification settings - Fork 281
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
Change the colors of the filter buttons and change the migration details bar (colors and tooltips) #2239
Change the colors of the filter buttons and change the migration details bar (colors and tooltips) #2239
Conversation
✅ Deploy Preview for conda-forge-previews ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Thanks! The buttons look perfect. The "progress bar" still uses the same viridis colors as before, though, which has terrible visibility in dark mode: I think Gabriela was suggesting to use tooltips to identify the different blocks of the progress bar in #2090 (comment) |
d628db9
to
0233e7c
Compare
@jaimergp I was thinking about implementing the changes on the bar and the tooltip in a different PR. |
The color coding is a usability plus in my book. People get a clear idea immediately and can guess at what the colors mean. I'd argue we should find better colors for dark mode as opposed to removing them. |
The concern raised by @GabrielaVives was:
We need to be careful with color coding and contrast issues and it's usually tricky to find a palette that works well contrast wise and doesn't run into color blindness issues. Hence why I assume Gabriela was suggesting different approaches that didn't rely exclusively on color coding. The visibility in dark mode right now needs to be fixed one way or another. I don't feel strongly about one solution over the other as long as we keep accessibility concerns in mind.
I think this time it'll be better to address both things in a single PR because we'll have to make a decision about how we approach the solution for both. Or at least, I'd like to see them together even if as a prototype, and then we can split in PRs. But the changes look reasonably small to tackle in a single PR. |
Sounds good to me. I am not knowledgeable on these accessibility issues so I defer to you all on that. |
@jaimergp Ok I will do everything in this same PR and will change the title of it. |
src/css/custom.css
Outdated
--ifm-color-grey-light:#bebebe; | ||
--ifm-color-grey-medium:#7f7f7f; | ||
--ifm-color-grey-dark:#3f3f3f; |
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.
We have all these grays in the infima variables already, I think:
--ifm-color-gray-100: #f5f6f7;
--ifm-color-gray-200: #ebedf0;
--ifm-color-gray-300: #dadde1;
--ifm-color-gray-400: #ccd0d5;
--ifm-color-gray-500: #bec3c9;
--ifm-color-gray-600: #8d949e;
--ifm-color-gray-700: #606770;
--ifm-color-gray-800: #444950;
--ifm-color-gray-900: #1c1e21;
--ifm-color-gray-1000: var(--ifm-color-black);
pre-commit.ci autofix |
4ee7b42
to
90a2a5f
Compare
@jaimergp If you have better suggestion for the tooltips text color and background color, please let me know. I thought that using the ones of the secondary button may be a solution, but I could not figure out how to get them from Infima. I also removed the title that was displayed on hovering the different migration bar element since the tooltip is showing the same information. |
pre-commit.ci autofix |
Tooltips looking awesome! Just two comments. |
I am adding two gifs here to have an overview of the rendering in light and dark mode: |
Hi @HaudinFlorence, I presented your changes in the conda-forge core meeting today to see how folks felt about it. The reception was great; we just have two tiny comments:
|
…ton. Remove unnecessary lines in the related styles.module.css
Co-authored-by: jaimergp <[email protected]>
…d the number of PRs inside this category over the total number.
0bab737
to
52fa80c
Compare
@jaimergp Thanks for the feedback. I can try to add a dot whose color is fitting with the color of the corresponding category is the migration status bar. |
After testing the filters, I believe that the filter icon on the left does not add value. The icon does not clearly indicate its purpose, and the hover or selection states of the buttons should suffice to convey their filtering actions, as is common in many applications and websites. Adding the icon suggests a lack of confidence in the UX of the filters and unnecessarily complicates the interface, distracting from the primary function of these buttons. I recommend removing the filter icon and instead placing a color dot on the left side of each tab. This approach is more intuitive, as it maintains a consistent location for the color dot, regardless of the text length. |
In my opinion, the behavior is good, but the filters could be organised differently:
|
…h the colors of the category in the migration bar.
pre-commit.ci autofix |
Both remarks should have been addressed : the additions of dots and the bot error red that has been made darker. Please let me know if it looks fine to you and how it could be improved. Thanks. |
right now I see this I think we should remove the "filter symbol" and just have the dot centered vertically and left justified in the button. This is the same comment as @GabrielaVives made above
|
I was planning to remove it, but was maybe expecting more feedback on this suggestion before doing it. So let's remove it. |
Ah great! Let's go ahead indeed! |
… positions and sizes of the button and its content elements.
@GabrielaVives Thanks for your suggestion. It has been addressed (both the removal of the filter icon and the addition of the colored dot) and it currently looks like that: The borders of the dots may be adapted in width and color. |
…uttons by unsetting the space-evenly value for the justify-content property.
@GabrielaVives Thanks for this comment. I implemented the first 2 points. For the third one, I am not sure what is the best order for a reorganisation of the buttons. A detailed suggestion is welcome. An issue has been opened here: #2243. Thanks. |
pre-commit.ci autofix |
The last commits are attempts to improve the contrast between the different colors of the migrations bars and dots. I think at this stage, all the issues mentioned previously on the filters buttons have been addressed or reported elsewhere in a new issue. If there is no other suggestion, ask for changes or objection, the PR may be ready to be merged. |
This looks great and I think we should merge. We can iterate more in future PRs. Thank you so much! |
PR Checklist:
docs/
orcommunity/
, you have added it to the sidebar in the corresponding_sidebar.json
fileThis PR aims at changing the colors of the filter buttons using
button--primary
andbutton--secondary
classes as shown:LIGHT MODE
before
now
DARK MODE
before
now
This PR aims at fixing task 3 and partially task 13 of issue #2137. It takes into account suggestions of this comment #2090 (comment)
cc @GabrielaVives @afshin