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 #34618 -- Added icon descriptions in "Recent Actions" on the admin index. #16917

Merged
merged 9 commits into from Jun 2, 2023

Conversation

nmenezes0
Copy link
Contributor

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Hello! Thank you for your contribution 💪

As it's your first contribution be sure to check out the patch review checklist.

If you're fixing a ticket from Trac make sure to set the "Has patch" flag and include a link to this PR in the ticket!

If you have any design or process questions then you can ask in the Django forum.

Welcome aboard ⛵️!

Copy link
Member

@smithdc1 smithdc1 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. 👍

Could we add tests to show that the html generated when this template is used?

django/contrib/admin/static/admin/css/dark_mode.css Outdated Show resolved Hide resolved
@DevilsAutumn
Copy link
Contributor

Hi, Just to avoid linting and formatting errors you can set up pre commit checks. See here for instructions.

@thibaudcolas
Copy link
Sponsor Member

thibaudcolas commented Jun 1, 2023

@DevilsAutumn as far as I can see there are no pre-commit checks for CSS. Be great to add some?

@DevilsAutumn
Copy link
Contributor

DevilsAutumn commented Jun 1, 2023

@DevilsAutumn as far as I can see there are no pre-commit checks for CSS. Be great to add some?

It was for admin_views/tests.py. 😄

@felixxm felixxm changed the title Bugfix/admin icons - 34618 Fixed #34618 -- Added icon descriptions in "Recent Actions" on the admin index. Jun 1, 2023
tests/admin_views/tests.py Outdated Show resolved Hide resolved
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.

👍 all working as expected in desktop Safari VoiceOver.

@felixxm
Copy link
Member

felixxm commented Jun 2, 2023

@nmenezes0 Do you have time to keep working on this? I only left reformatting suggestions.

Co-authored-by: Mariusz Felisiak <felisiak.mariusz@gmail.com>
@nmenezes0
Copy link
Contributor Author

@nmenezes0 Do you have time to keep working on this? I only left reformatting suggestions.

Yes, I have time to keep working on this today and will make the suggested changes.

nmenezes0 and others added 2 commits June 2, 2023 10:18
Co-authored-by: Mariusz Felisiak <felisiak.mariusz@gmail.com>
@felixxm
Copy link
Member

felixxm commented Jun 2, 2023

@nmenezes0 Thanks 👍 Welcome aboard ⛵ You're a Django contributor no. 2400 🥳

@nmenezes0
Copy link
Contributor Author

@nmenezes0 Thanks 👍 Welcome aboard ⛵ You're a Django contributor no. 2400 🥳

Have made those formatting changes and checked they are ok locally.

@felixxm felixxm merged commit 27fed08 into django:main Jun 2, 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
6 participants