[#2137] Show disabled pseudo-checkbox for actions #1056
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #1056 +/- ##
========================================
Coverage 94.92% 94.92%
========================================
Files 881 881
Lines 30863 30863
========================================
Hits 29298 29298
Misses 1565 1565 ☔ View full report in Codecov by Sentry. |
jiromaykin
force-pushed
the
feature/2137-pseudo-checkbox-profile-notifications
branch
from
17c86c4
to
ac958a7
Compare
| {% autorender_field form field %} | ||
| {% endfor %} | ||
|
|
||
| <div class="form__control"> |
There was a problem hiding this comment.
Maybe we could move this disabled checkbox to a separate template that can be included, to avoid duplication.
Also, is there a reason why this is a <span role="checkbox"> and not an <input type="checkbox" disabled>?
There was a problem hiding this comment.
Let's not make a reusable template for bad patterns such as actual disabled inputs that cannot be enabled by the user, as I explained in the description of this PR above - also: new designs are coming soon and this PR is an intermediate change.
Also: yes, this should not be an actual input tag, because accessibility devices completely ignore disabled inputs and do not give any textual/label information.
There was a problem hiding this comment.
check, didn't know that 👍
| {# pseudo checkbox for notifications that cannot be disabled #} | ||
| <div class="checkbox"> | ||
| <span role="checkbox" aria-disabled="true" aria-checked="true" class="checkbox__input checkbox__pseudo checkbox__input--disabled" aria-labelledby="id_cases_notifications_action_required" tabindex="0"></span> | ||
| <span class="checkbox__label checkbox__pseudo-label" id="id_cases_notifications_action_required">{% trans "Zaaknotificaties - action required" %}</span> |
There was a problem hiding this comment.
{% trans "Zaaknotificaties - action required" %} is a mix of Dutch and English (should have been cleaned up as part of my PR).
There was a problem hiding this comment.
Guess I should make it all in Dutch (including the paragraph)?
(We have new translations for this one now I can use)
There was a problem hiding this comment.
Yes, client-facing text should be Dutch, preferably. It will be translated anyways, but this will make it easier and more consistent.
jiromaykin
force-pushed
the
feature/2137-pseudo-checkbox-profile-notifications
branch
from
ac958a7
to
15fb014
Compare
alextreme
deleted the
feature/2137-pseudo-checkbox-profile-notifications
branch
Issue: https://taiga.maykinmedia.nl/project/open-inwoner/issue/2137
To explicitly show required-actions notifications cannot be turned off (on http://localhost:8000/mijn-profiel/notificaties/).
➕ also making the pseudo checkbox accessible (⚠️ ordinary disabled inputs are not detected by screenreaders and are not read + cannot be focussed, so making a visual fake checkbox here that actually gets detected).
A disabled input that cannot be enabled by the user in any way is a violation of accessibility.