Fixed #34977: Make change password form reachable through button #17489

fsbraun · 2023-11-19T13:52:12Z

https://code.djangoproject.com/ticket/34977

To improve usability and accessibility, this PR replaces the link to change a user's password within the help text of the password field of the change user form by a button.

Besides an improved user experience, I would expect this PR to also improve accessibility. But since I am anything but an expert on accessibility, I would like to ask the accessibility team to review and give hints on hot to improve accessibility.

nessita · 2024-01-05T23:04:09Z

I would go a different route to solve this issue, specially because I believe that the best practice for help texts is not to use markup at all (more so for easing translations).

So my proposal is to:

Re-phrase the help texts so they are complete sentences that make sense but without any actionable links nor buttons.
Add the action button next to the help text.

This would also have the (nice) side effect to separate "form concerns" (ie help texts) from UI elements that want to be displayed together in a template.

Initial proposal (to be discussed with the accessibility team):

diff --git a/django/contrib/auth/forms.py b/django/contrib/auth/forms.py
index 5398af4dad..30d28cbe00 100644
--- a/django/contrib/auth/forms.py
+++ b/django/contrib/auth/forms.py
@@ -179,9 +179,8 @@ class UserChangeForm(forms.ModelForm):
     password = ReadOnlyPasswordHashField(
         label=_("Password"),
         help_text=_(
-            "<p>Raw passwords are not stored, so there is no way to see this "
-            "user’s password, but you can change the password here:</p>"
-            '<p><a class="button" role="button" href="{}">Change password</a></p>'
+            "Raw passwords are not stored, so there is no way to see this "
+            "user’s password."
         ),
     )
 
diff --git a/django/contrib/auth/templates/auth/widgets/read_only_password_hash.html b/django/contrib/auth/templates/auth/widgets/read_only_password_hash.html
index c73042b18f..f88d2e1ba4 100644
--- a/django/contrib/auth/templates/auth/widgets/read_only_password_hash.html
+++ b/django/contrib/auth/templates/auth/widgets/read_only_password_hash.html
@@ -1,5 +1,8 @@
+{% load i18n %}
+
 <div{% include 'django/forms/widgets/attrs.html' %}>
 {% for entry in summary %}
 <strong>{{ entry.label }}</strong>{% if entry.value %}: <bdi>{{ entry.value }}</bdi>{% endif %}
 {% endfor %}
+<a class="button" role="button" href="../password/">{% trans "Change password" %}</a>
 </div>

Draft UI:

fsbraun · 2024-01-08T20:06:17Z

Good idea! Once we're touching ReadOnlyPasswordHashField I'd also like to change the formatting that the button always will be on the right. Depending on viewport width, the button will otherwise break into the next line.

nessita · 2024-03-07T17:57:53Z

@fsbraun Would you have time to update this PR by rebasing onto main and ensuring is ready for re-review? Thanks!

@django/accessibility Would you have any opinion on my proposal above?

/remind me to re-check the status of this PR in 3 weeks

github-actions · 2024-03-07T17:58:07Z

@nessita set a reminder for 3/28/2024

fsbraun · 2024-03-11T15:42:47Z

@nessita I've rebased and rewritten to reflect your first comment:

Help texts do not contain markup or buttons
Widget now contains the button for the link
In private exchange with @sabderemane we concluded that a role attribute for the button is not advisable from a accessibility viewpoint

nessita · 2024-03-11T16:37:48Z

@nessita I've rebased and rewritten to reflect your first comment:

* Help texts do not contain markup or buttons

* Widget now contains the button for the link

* In private exchange with @sabderemane we concluded that a `role` attribute for the button is not advisable from a accessibility viewpoint

Thank you @fsbraun ! Please remember to update the ticket flags accordingly.

nessita

Overall looks good!

Added some comments, and also please note that this work needs documentation updates, specifically we should review if any of the existing docs mention the link, and also we need release notes in the 5.1.txt file.

Thank you @fsbraun !

django/contrib/admin/static/admin/css/widgets.css

nessita · 2024-03-26T20:20:30Z

django/contrib/auth/forms.py

@@ -54,11 +54,17 @@ def get_context(self, name, value, attrs):
                for key, value_ in hasher.safe_summary(value).items():
                    summary.append({"label": gettext(key), "value": value_})
        context["summary"] = summary
+        context["usable_password"] = len(summary) > 1


I believe that is cleaner to define using value.startswith(UNUSABLE_PASSWORD_PREFIX). So, something like:

usable_password = not value.startswith(UNUSABLE_PASSWORD_PREFIX) if not value or not usable_password: summary.append({"label": gettext("No password set.")}) else: # ... context["usable_password"] = usable_password

On a second thought, since this is the Widget class, shall we instead add to the context the label and action of the button? My initial thinking is that approach would be cleaner and simpler in the template. What do you think?

@nessita What's the right way to get the admin namespace from within the widget? I would need it to reverse the set password URL. The template uses a ../bla which indeed is a bit hacky. The reason might be that the admin namespace is not readily available.

Admin forms get the admin site passed to their __init__ function. But that pattern would require to change the signature of the UserChangeForm which I would like to avoid.

I have moved the button label from the template to the get_context method, however.

nessita · 2024-03-26T20:30:46Z

django/contrib/auth/forms.py

+        css = {
+            "all": ("admin/css/widgets.css",),
+        }
+


Is this really needed? I have removed it and I still see the same formatting in the admin 🤔

Other widgets on the user change form load the widgets.css, so removing it here will indeed not change anything.

I would still like to keep it, since the widget needs it for margins. If we removed the css from the widget it will fail to display properly if other widgets on the change from do not load the widgets.css for whatever reason.

Alternative: Have your own auth/css/widgets.css? (Once might argue this is cleaner since it does not cross app boundaries.)

Thanks for the extra details. My advice then is as follows (I'll make a new commit with these):

Change the HTML to make use of semantic elements such as <p>

Remove the new CSS classes and custom CSS rule

Let users of this widget define their own CSS by including the widget in whichever CSS context is relevant for them.

fsbraun · 2024-03-27T07:41:05Z

@nessita Thank you for your comments!

I added a bullet point to the release notes. I searched the docs and did not find a reference to the link. Have I missed something?

I also implemented some of your comments where I did not see any roadblocks. I added comments to the others. Getting the URL of the change password action from within the widget seems tricky to me.

fsbraun · 2024-03-27T07:45:25Z

Building docs fails apparently since it cannot locate the class django.contrib.auth.forms.ReadOnlyPasswordHashWidget . I am not sure why. What am I missing?

nessita

Looks good, thanks! I will make a few tweaks and merge. 🏆

django/contrib/auth/forms.py

nessita · 2024-03-27T18:01:35Z

django/contrib/auth/forms.py

+        css = {
+            "all": ("admin/css/widgets.css",),
+        }
+


Thanks for the extra details. My advice then is as follows (I'll make a new commit with these):

Change the HTML to make use of semantic elements such as <p>

Remove the new CSS classes and custom CSS rule

Let users of this widget define their own CSS by including the widget in whichever CSS context is relevant for them.

…cing the reset password link with a button. Co-authored-by: Natalia <124304+nessita@users.noreply.github.com>

fsbraun mentioned this pull request Nov 19, 2023

Fixed #34429 -- Allowed setting unusable passwords for users in the auth forms. #16942

Merged

felixxm requested a review from a team November 20, 2023 04:57

nessita self-requested a review November 27, 2023 18:16

nessita removed their request for review January 5, 2024 23:04

thibaudcolas self-requested a review January 12, 2024 08:06

github-actions bot added the reminder label Mar 7, 2024

fsbraun force-pushed the ticket_34977 branch 3 times, most recently from 52c7f22 to 64844a9 Compare March 11, 2024 15:24

nessita reviewed Mar 26, 2024

View reviewed changes

fsbraun force-pushed the ticket_34977 branch from ecf49fc to ea0e3db Compare March 27, 2024 08:50

nessita reviewed Mar 27, 2024

View reviewed changes

nessita force-pushed the ticket_34977 branch from 3ab8e58 to 0c85d85 Compare March 27, 2024 19:19

Fixed #34977 -- Improved accessibility in the UserChangeForm by repla…

944745a

…cing the reset password link with a button. Co-authored-by: Natalia <124304+nessita@users.noreply.github.com>

nessita force-pushed the ticket_34977 branch from 0c85d85 to 944745a Compare March 27, 2024 19:42

nessita merged commit 944745a into django:main Mar 28, 2024
35 checks passed

felixxm temporarily deployed to schedules March 28, 2024 02:44 — with GitHub Actions Inactive

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed #34977: Make change password form reachable through button #17489

Fixed #34977: Make change password form reachable through button #17489

fsbraun commented Nov 19, 2023 •

edited by github-actions bot

nessita commented Jan 5, 2024

fsbraun commented Jan 8, 2024 •

edited

nessita commented Mar 7, 2024

github-actions bot commented Mar 7, 2024

fsbraun commented Mar 11, 2024

nessita commented Mar 11, 2024

nessita left a comment

nessita Mar 26, 2024

nessita Mar 26, 2024

fsbraun Mar 27, 2024

nessita Mar 26, 2024

fsbraun Mar 27, 2024

fsbraun Mar 27, 2024 •

edited

nessita Mar 27, 2024

fsbraun commented Mar 27, 2024

fsbraun commented Mar 27, 2024

nessita left a comment

nessita Mar 27, 2024

Fixed #34977: Make change password form reachable through button #17489

Fixed #34977: Make change password form reachable through button #17489

Conversation

fsbraun commented Nov 19, 2023 • edited by github-actions bot

nessita commented Jan 5, 2024

fsbraun commented Jan 8, 2024 • edited

nessita commented Mar 7, 2024

github-actions bot commented Mar 7, 2024

fsbraun commented Mar 11, 2024

nessita commented Mar 11, 2024

nessita left a comment

Choose a reason for hiding this comment

nessita Mar 26, 2024

Choose a reason for hiding this comment

nessita Mar 26, 2024

Choose a reason for hiding this comment

fsbraun Mar 27, 2024

Choose a reason for hiding this comment

nessita Mar 26, 2024

Choose a reason for hiding this comment

fsbraun Mar 27, 2024

Choose a reason for hiding this comment

fsbraun Mar 27, 2024 • edited

Choose a reason for hiding this comment

nessita Mar 27, 2024

Choose a reason for hiding this comment

fsbraun commented Mar 27, 2024

fsbraun commented Mar 27, 2024

nessita left a comment

Choose a reason for hiding this comment

nessita Mar 27, 2024

Choose a reason for hiding this comment

fsbraun commented Nov 19, 2023 •

edited by github-actions bot

fsbraun commented Jan 8, 2024 •

edited

fsbraun Mar 27, 2024 •

edited