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

Fixed #34627 -- Highlighted active row in admin UI when forced-colors mode is enabled. #16932

Merged
merged 3 commits into from Jun 5, 2023
Merged

Fixed #34627 -- Highlighted active row in admin UI when forced-colors mode is enabled. #16932

merged 3 commits into from Jun 5, 2023

Conversation

nmenezes0
Copy link
Contributor

Fixes https://code.djangoproject.com/ticket/34627

This PR adds highlighting on selected models in the admin sidebar and changelist when using "forced-colors".

For selected models in the admin and changelist:
image

This is what you currently see with forced-colors:
image

This PR changes this to:
image

@nessita
Copy link
Contributor

nessita commented Jun 2, 2023

Thank you for this work! For those (like me) that had no idea how to test this in Firefox, I followed https://hacks.mozilla.org/2020/07/adding-prefers-contrast-to-firefox/ and set the colors to "Override the colors specified by the page with your selections above" -> "Always".

I used the default colors from Firefox, and this is how it looks:
image

I was wondering if the row highlight could be... ligther? different? so the text on the highlighted row is easier to read. Right now the bold violet on top of dark cyan (?) seems challenging.

EDIT: After some googling (I'm not CSS expert), it seems that the SelectedItem value for background-color would use whatever my browser has configured so my suggestion does not make sense in that context.

@nessita nessita changed the title Bugfix/replace highlight - fixes 34627 Fixed #34627 - Highliligthed active row when forced-colores mode is enabled. Jun 2, 2023
@nessita nessita changed the title Fixed #34627 - Highliligthed active row when forced-colores mode is enabled. Fixed #34627 - Highlighted active row in admin UI when forced-colors mode is enabled. Jun 2, 2023
Copy link
Contributor

@nessita nessita left a comment

Choose a reason for hiding this comment

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

Approving since this is working as expected, will wait a few days before merging to ensure other reviewers get the chance to review as well.

@felixxm felixxm changed the title Fixed #34627 - Highlighted active row in admin UI when forced-colors mode is enabled. Fixed #34627 -- Highlighted active row in admin UI when forced-colors mode is enabled. Jun 2, 2023
Copy link
Sponsor Member

@thibaudcolas thibaudcolas left a comment

Choose a reason for hiding this comment

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

Thank you for giving this a go @nmenezes0. Result is looking great.

@nmenezes0
Copy link
Contributor Author

Thank you for giving this a go @nmenezes0. Result is looking great.

Thanks for all your help!

Copy link
Member

@felixxm felixxm left a comment

Choose a reason for hiding this comment

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

@nmenezes0 Thanks 👍

@nessita nessita merged commit 51fdea6 into django:main Jun 5, 2023
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants