Quansight-Labs / jupyterlab-accessible-themes

♿️🎨 An access-centred implementation of the JupyterLab default themes

BSD 3-Clause "New" or "Revised" License

2 stars 1 forks source link

Fix Github Light and Pitaya Smoothie contrast color bugs #62

Closed steff456 closed 1 year ago

steff456 commented 1 year ago

Fixes #59

This PR,

[x] Fixes the issues found in the Pitaya Smoothie and Github Light themes.

Here are some screenshots on how the current themes look,

Github Light

[x] The active tab colour is noticeable
[x] Hover background (menu) has enough contrast against the background colour
[x] Colours in the kernel/tab manager sidebar
[x] Active indicator on the search bar
[x] Toggle background is noticeable in the status bar

Pitaya Smoothie

[x] Icons have enough contrast against the background
[x] Active indicator on the search bar

github-actions[bot] commented 1 year ago

:point_left: Launch a binder notebook on branch Quansight-Labs/jupyterlab-accessible-themes/fix-themes _{Comment updated on 2023-07-05T19:03:54.853Z}

steff456 commented 1 year ago

This PR addresses all the issues in #59 for Github Light and Pitaya Smoothie. I made the changes of Github Dark in the PR #53.

Hope this clarifies!

gabalafou commented 1 year ago

Sure, thanks! Who is going to be responsible for checking off the tasks in issue #59?

Maybe also the title of the PR should be renamed to "Fixes for Pitaya Smoothie and Github Light." I understand that it says this already in the description, but right now the PR title is really generic.

steff456 commented 1 year ago

Who is going to be responsible for checking off the tasks in issue https://github.com/Quansight-Labs/jupyterlab-accessible-themes/issues/59?

If we agree on the changes that I made for fixing them, I could check them once this PR is merged.

trallard commented 1 year ago

Disagree - changes should be checked BEFORE merging not the other way around

trallard commented 1 year ago

For example there is one that was not checked off for the dark theme as when I did the review I noticed the fix did not fix the colour contrast issue

steff456 commented 1 year ago

So the reviewer should be the one checking off in the original issue

trallard commented 1 year ago

Or you provided you tested / verified the changes and they meet the contrast requirements. Which IMO is part of doing the fix.

steff456 commented 1 year ago

I added the screenshots to the description of this PR, I will update it with the contrast that every pair has. Hope that helps with the review!

steff456 commented 1 year ago

With the latest changes, here are the screenshots of every change with its contrast check. (I already updated the issue as well)

Github Light

[x] The active tab colour is noticeable.

Blue line and text have enough contrast against the background color

[x] Hover background (menu) has enough contrast against the background colour

Gray background with white text

White background with gray hover color

[x] Colours in the kernel/tab manager sidebar

[x] Active indicator on the search bar

[x] Toggle background is noticeable in the status bar

Light gray background has enough contrast with toggle background

Toggle has enough contrast

Pitaya Smoothie

[x] Icons have enough contrast against the background

Non-selected,

Selected,

[x] Active indicator on the search bar

github-actions[bot] commented 1 year ago

:point_left: Launch a Binder on branch Quansight-Labs/jupyterlab-accessible-themes/fix-themes

github-actions[bot] commented 1 year ago

:point_left: Launch a Binder on branch Quansight-Labs/jupyterlab-accessible-themes/fix-themes

trallard commented 1 year ago

I am going to merge this as it seems ok after a quick review and I suppose users might raise issues later