Improve bookmark form accessibility (#1116)

* Bump Django

* Render error messages in English

* Remove unused USE_L10N option

* Associate errors and help texts with form fields

* Make checkbox inputs clickable

* Change cancel button text

* Fix tests
This commit is contained in:
Sascha Ißbrücker
2025-07-03 08:44:41 +02:00
committed by GitHub
parent a8623d11ef
commit 4e8318d0ae
13 changed files with 123 additions and 78 deletions

View File

@@ -1,4 +1,5 @@
from django import forms from django import forms
from django.forms.utils import ErrorList
from bookmarks.models import Bookmark, build_tag_string from bookmarks.models import Bookmark, build_tag_string
from bookmarks.validators import BookmarkURLValidator from bookmarks.validators import BookmarkURLValidator
@@ -6,6 +7,10 @@ from bookmarks.type_defs import HttpRequest
from bookmarks.services.bookmarks import create_bookmark, update_bookmark from bookmarks.services.bookmarks import create_bookmark, update_bookmark
class CustomErrorList(ErrorList):
template_name = "shared/error_list.html"
class BookmarkForm(forms.ModelForm): class BookmarkForm(forms.ModelForm):
# Use URLField for URL # Use URLField for URL
url = forms.CharField(validators=[BookmarkURLValidator()]) url = forms.CharField(validators=[BookmarkURLValidator()])
@@ -48,7 +53,9 @@ class BookmarkForm(forms.ModelForm):
if instance is not None and request.method == "GET": if instance is not None and request.method == "GET":
initial = {"tag_string": build_tag_string(instance.tag_names, " ")} initial = {"tag_string": build_tag_string(instance.tag_names, " ")}
data = request.POST if request.method == "POST" else None data = request.POST if request.method == "POST" else None
super().__init__(data, instance=instance, initial=initial) super().__init__(
data, instance=instance, initial=initial, error_class=CustomErrorList
)
@property @property
def is_auto_close(self): def is_auto_close(self):

View File

@@ -19,6 +19,7 @@ class TagAutocomplete extends Behavior {
name: input.name, name: input.name,
value: input.value, value: input.value,
placeholder: input.getAttribute("placeholder") || "", placeholder: input.getAttribute("placeholder") || "",
ariaDescribedBy: input.getAttribute("aria-describedby") || "",
variant: input.getAttribute("variant"), variant: input.getAttribute("variant"),
}, },
}); });

View File

@@ -6,6 +6,7 @@
export let name; export let name;
export let value; export let value;
export let placeholder; export let placeholder;
export let ariaDescribedBy;
export let variant = 'default'; export let variant = 'default';
let isFocus = false; let isFocus = false;
@@ -110,6 +111,7 @@
<!-- autocomplete real input box --> <!-- autocomplete real input box -->
<input id="{id}" name="{name}" value="{value ||''}" placeholder="{placeholder || ' '}" <input id="{id}" name="{name}" value="{value ||''}" placeholder="{placeholder || ' '}"
class="form-input" type="text" autocomplete="off" autocapitalize="off" class="form-input" type="text" autocomplete="off" autocapitalize="off"
aria-describedby="{ariaDescribedBy}"
on:input={handleInput} on:keydown={handleKeyDown} on:input={handleInput} on:keydown={handleKeyDown}
on:focus={handleFocus} on:blur={handleBlur}> on:focus={handleFocus} on:blur={handleBlur}>
</div> </div>

View File

@@ -116,8 +116,6 @@ TIME_ZONE = os.getenv("TZ", "UTC")
USE_I18N = True USE_I18N = True
USE_L10N = True
USE_TZ = True USE_TZ = True
# Static files (CSS, JavaScript, Images) # Static files (CSS, JavaScript, Images)

View File

@@ -224,12 +224,13 @@ textarea.form-input {
position: relative; position: relative;
input { input {
clip: rect(0, 0, 0, 0); opacity: 0;
height: 1px;
margin: -1px;
overflow: hidden;
position: absolute; position: absolute;
width: 1px; top: calc((var(--control-size-sm) - var(--control-icon-size)) / 2);
left: 0;
height: var(--control-icon-size);
width: var(--control-icon-size);
cursor: pointer;
&:focus-visible + .form-icon { &:focus-visible + .form-icon {
outline: var(--focus-outline); outline: var(--focus-outline);
@@ -243,9 +244,9 @@ textarea.form-input {
} }
.form-icon { .form-icon {
pointer-events: none;
border: var(--border-width) solid var(--checkbox-border-color); border: var(--border-width) solid var(--checkbox-border-color);
box-shadow: var(--input-box-shadow); box-shadow: var(--input-box-shadow);
cursor: pointer;
display: inline-block; display: inline-block;
position: absolute; position: absolute;
transition: transition:

View File

@@ -1,5 +1,6 @@
{% load widget_tweaks %} {% load widget_tweaks %}
{% load static %} {% load static %}
{% load shared %}
<div class="bookmarks-form"> <div class="bookmarks-form">
{% csrf_token %} {% csrf_token %}
@@ -7,7 +8,7 @@
<div class="form-group {% if form.url.errors %}has-error{% endif %}"> <div class="form-group {% if form.url.errors %}has-error{% endif %}">
<label for="{{ form.url.id_for_label }}" class="form-label">URL</label> <label for="{{ form.url.id_for_label }}" class="form-label">URL</label>
<div class="has-icon-right"> <div class="has-icon-right">
{{ form.url|add_class:"form-input"|attr:"autofocus"|attr:"placeholder: " }} {{ form.url|form_field:"validation"|add_class:"form-input"|attr:"autofocus" }}
<i class="form-icon loading"></i> <i class="form-icon loading"></i>
</div> </div>
{% if form.url.errors %} {% if form.url.errors %}
@@ -22,8 +23,8 @@
</div> </div>
<div class="form-group" ld-tag-autocomplete> <div class="form-group" ld-tag-autocomplete>
<label for="{{ form.tag_string.id_for_label }}" class="form-label">Tags</label> <label for="{{ form.tag_string.id_for_label }}" class="form-label">Tags</label>
{{ form.tag_string|add_class:"form-input"|attr:"autocomplete:off"|attr:"autocapitalize:off" }} {{ form.tag_string|form_field:"help"|add_class:"form-input"|attr:"autocomplete:off"|attr:"autocapitalize:off" }}
<div class="form-input-hint"> <div id="{{ form.tag_string.auto_id }}_help" class="form-input-hint">
Enter any number of tags separated by space and <strong>without</strong> the hash (#). Enter any number of tags separated by space and <strong>without</strong> the hash (#).
If a tag does not exist it will be automatically created. If a tag does not exist it will be automatically created.
</div> </div>
@@ -35,7 +36,8 @@
<label for="{{ form.title.id_for_label }}" class="form-label">Title</label> <label for="{{ form.title.id_for_label }}" class="form-label">Title</label>
<div class="flex"> <div class="flex">
<button id="refresh-button" class="btn btn-link suffix-button" type="button">Refresh from website</button> <button id="refresh-button" class="btn btn-link suffix-button" type="button">Refresh from website</button>
<button ld-clear-button data-for="{{ form.title.id_for_label }}" class="ml-2 btn btn-link suffix-button clear-button" <button ld-clear-button data-for="{{ form.title.id_for_label }}"
class="ml-2 btn btn-link suffix-button clear-button"
type="button">Clear type="button">Clear
</button> </button>
</div> </div>
@@ -60,31 +62,31 @@
<span class="form-label d-inline-block">Notes</span> <span class="form-label d-inline-block">Notes</span>
</summary> </summary>
<label for="{{ form.notes.id_for_label }}" class="text-assistive">Notes</label> <label for="{{ form.notes.id_for_label }}" class="text-assistive">Notes</label>
{{ form.notes|add_class:"form-input"|attr:"rows:8" }} {{ form.notes|form_field:"help"|add_class:"form-input"|attr:"rows:8" }}
<div class="form-input-hint"> <div id="{{ form.notes.auto_id }}_help" class="form-input-hint">
Additional notes, supports Markdown. Additional notes, supports Markdown.
</div> </div>
</details> </details>
{{ form.notes.errors }} {{ form.notes.errors }}
</div> </div>
<div class="form-group"> <div class="form-group">
<label for="{{ form.unread.id_for_label }}" class="form-checkbox"> <div class="form-checkbox">
{{ form.unread }} {{ form.unread|form_field:"help" }}
<i class="form-icon"></i> <i class="form-icon"></i>
<span>Mark as unread</span> <label for="{{ form.unread.id_for_label }}">Mark as unread</label>
</label> </div>
<div class="form-input-hint"> <div id="{{ form.unread.auto_id }}_help" class="form-input-hint">
Unread bookmarks can be filtered for, and marked as read after you had a chance to look at them. Unread bookmarks can be filtered for, and marked as read after you had a chance to look at them.
</div> </div>
</div> </div>
{% if request.user_profile.enable_sharing %} {% if request.user_profile.enable_sharing %}
<div class="form-group"> <div class="form-group">
<label for="{{ form.shared.id_for_label }}" class="form-checkbox"> <div class="form-checkbox">
{{ form.shared }} {{ form.shared|form_field:"help" }}
<i class="form-icon"></i> <i class="form-icon"></i>
<span>Share</span> <label for="{{ form.shared.id_for_label }}">Share</label>
</label> </div>
<div class="form-input-hint"> <div id="{{ form.shared.auto_id }}_help" class="form-input-hint">
{% if request.user_profile.enable_public_sharing %} {% if request.user_profile.enable_public_sharing %}
Share this bookmark with other registered users and anonymous users. Share this bookmark with other registered users and anonymous users.
{% else %} {% else %}
@@ -100,7 +102,7 @@
{% else %} {% else %}
<input type="submit" value="Save" class="btn btn-primary btn btn-primary btn-wide"> <input type="submit" value="Save" class="btn btn-primary btn btn-primary btn-wide">
{% endif %} {% endif %}
<a href="{{ return_url }}" class="btn">Nevermind</a> <a href="{{ return_url }}" class="btn">Cancel</a>
</div> </div>
<script type="application/javascript"> <script type="application/javascript">
/** /**
@@ -227,6 +229,7 @@
} }
}); });
} }
refreshButton.addEventListener('click', refreshMetadata); refreshButton.addEventListener('click', refreshMetadata);
// Fetch website metadata when page loads and when URL changes, unless we are editing an existing bookmark // Fetch website metadata when page loads and when URL changes, unless we are editing an existing bookmark

View File

@@ -0,0 +1,6 @@
{% load i18n %}
{# Force rendering validation errors in English language to align with the rest of the app #}
{% language 'en-us' %}
{% if errors %}<ul class="{{ error_class }}"{% if errors.field_id %} id="{{ errors.field_id }}_error"{% endif %}>{% for error in errors %}<li>{{ error }}</li>{% endfor %}</ul>{% endif %}
{% endlanguage %}

View File

@@ -145,3 +145,30 @@ def render_markdown(context, markdown_text):
linkified_html = bleach.linkify(sanitized_html) linkified_html = bleach.linkify(sanitized_html)
return mark_safe(linkified_html) return mark_safe(linkified_html)
def append_attr(widget, attr, value):
attrs = widget.attrs
if attrs.get(attr):
attrs[attr] += " " + value
else:
attrs[attr] = value
@register.filter("form_field")
def form_field(field, modifier_string):
modifiers = modifier_string.split(",")
has_errors = hasattr(field, "errors") and field.errors
if "validation" in modifiers and has_errors:
append_attr(field.field.widget, "aria-describedby", field.auto_id + "_error")
if "help" in modifiers:
append_attr(field.field.widget, "aria-describedby", field.auto_id + "_help")
# Some assistive technologies announce a field as invalid when it has the
# required attribute, even if the user has not interacted with the field
# yet. Set aria-invalid false to prevent this behavior.
if field.field.required and not has_errors:
append_attr(field.field.widget, "aria-invalid", "false")
return field

View File

@@ -114,9 +114,8 @@ class BookmarkEditViewTestCase(TestCase, BookmarkFactoryMixin):
self.assertInHTML( self.assertInHTML(
f""" f"""
<input type="text" name="url" value="{bookmark.url}" placeholder=" " <input type="text" name="url" aria-invalid="false" autofocus class="form-input" required id="id_url" value="{bookmark.url}">
autofocus class="form-input" required id="id_url"> """,
""",
html, html,
) )
@@ -124,7 +123,7 @@ class BookmarkEditViewTestCase(TestCase, BookmarkFactoryMixin):
self.assertInHTML( self.assertInHTML(
f""" f"""
<input type="text" name="tag_string" value="{tag_string}" <input type="text" name="tag_string" value="{tag_string}"
autocomplete="off" autocapitalize="off" class="form-input" id="id_tag_string"> autocomplete="off" autocapitalize="off" class="form-input" id="id_tag_string" aria-describedby="id_tag_string_help">
""", """,
html, html,
) )
@@ -148,7 +147,7 @@ class BookmarkEditViewTestCase(TestCase, BookmarkFactoryMixin):
self.assertInHTML( self.assertInHTML(
f""" f"""
<textarea name="notes" cols="40" rows="8" class="form-input" id="id_notes"> <textarea name="notes" cols="40" rows="8" class="form-input" id="id_notes" aria-describedby="id_notes_help">
{bookmark.notes} {bookmark.notes}
</textarea> </textarea>
""", """,
@@ -259,12 +258,12 @@ class BookmarkEditViewTestCase(TestCase, BookmarkFactoryMixin):
self.assertInHTML( self.assertInHTML(
""" """
<label for="id_shared" class="form-checkbox"> <div class="form-checkbox">
<input type="checkbox" name="shared" id="id_shared"> <input type="checkbox" name="shared" aria-describedby="id_shared_help" id="id_shared">
<i class="form-icon"></i> <i class="form-icon"></i>
<span>Share</span> <label for="id_shared">Share</label>
</label> </div>
""", """,
html, html,
count=0, count=0,
) )
@@ -278,12 +277,12 @@ class BookmarkEditViewTestCase(TestCase, BookmarkFactoryMixin):
self.assertInHTML( self.assertInHTML(
""" """
<label for="id_shared" class="form-checkbox"> <div class="form-checkbox">
<input type="checkbox" name="shared" id="id_shared"> <input type="checkbox" name="shared" aria-describedby="id_shared_help" id="id_shared">
<i class="form-icon"></i> <i class="form-icon"></i>
<span>Share</span> <label for="id_shared">Share</label>
</label> </div>
""", """,
html, html,
count=1, count=1,
) )

View File

@@ -78,9 +78,9 @@ class BookmarkNewViewTestCase(TestCase, BookmarkFactoryMixin):
html = response.content.decode() html = response.content.decode()
self.assertInHTML( self.assertInHTML(
'<input type="text" name="url" value="http://example.com" ' """
'placeholder=" " autofocus class="form-input" required ' <input type="text" name="url" aria-invalid="false" autofocus class="form-input" required id="id_url" value="http://example.com">
'id="id_url">', """,
html, html,
) )
@@ -117,9 +117,10 @@ class BookmarkNewViewTestCase(TestCase, BookmarkFactoryMixin):
html = response.content.decode() html = response.content.decode()
self.assertInHTML( self.assertInHTML(
'<input type="text" name="tag_string" value="tag1 tag2 tag3" ' """
'class="form-input" autocomplete="off" autocapitalize="off" ' <input type="text" name="tag_string" value="tag1 tag2 tag3"
'id="id_tag_string">', aria-describedby="id_tag_string_help" autocapitalize="off" autocomplete="off" class="form-input" id="id_tag_string">
""",
html, html,
) )
@@ -137,8 +138,8 @@ class BookmarkNewViewTestCase(TestCase, BookmarkFactoryMixin):
<span class="form-label d-inline-block">Notes</span> <span class="form-label d-inline-block">Notes</span>
</summary> </summary>
<label for="id_notes" class="text-assistive">Notes</label> <label for="id_notes" class="text-assistive">Notes</label>
<textarea name="notes" cols="40" rows="8" class="form-input" id="id_notes">**Find** more info [here](http://example.com)</textarea> <textarea name="notes" cols="40" rows="8" class="form-input" id="id_notes" aria-describedby="id_notes_help">**Find** more info [here](http://example.com)</textarea>
<div class="form-input-hint"> <div id="id_notes_help" class="form-input-hint">
Additional notes, supports Markdown. Additional notes, supports Markdown.
</div> </div>
</details> </details>
@@ -196,12 +197,12 @@ class BookmarkNewViewTestCase(TestCase, BookmarkFactoryMixin):
self.assertInHTML( self.assertInHTML(
""" """
<label for="id_shared" class="form-checkbox"> <div class="form-checkbox">
<input type="checkbox" name="shared" id="id_shared"> <input type="checkbox" name="shared" aria-describedby="id_shared_help" id="id_shared">
<i class="form-icon"></i> <i class="form-icon"></i>
<span>Share</span> <label for="id_shared">Share</label>
</label> </div>
""", """,
html, html,
count=0, count=0,
) )
@@ -213,12 +214,12 @@ class BookmarkNewViewTestCase(TestCase, BookmarkFactoryMixin):
self.assertInHTML( self.assertInHTML(
""" """
<label for="id_shared" class="form-checkbox"> <div class="form-checkbox">
<input type="checkbox" name="shared" id="id_shared"> <input type="checkbox" name="shared" aria-describedby="id_shared_help" id="id_shared">
<i class="form-icon"></i> <i class="form-icon"></i>
<span>Share</span> <label for="id_shared">Share</label>
</label> </div>
""", """,
html, html,
count=1, count=1,
) )
@@ -231,10 +232,10 @@ class BookmarkNewViewTestCase(TestCase, BookmarkFactoryMixin):
html = response.content.decode() html = response.content.decode()
self.assertInHTML( self.assertInHTML(
""" """
<div class="form-input-hint"> <div id="id_shared_help" class="form-input-hint">
Share this bookmark with other registered users. Share this bookmark with other registered users.
</div> </div>
""", """,
html, html,
) )
@@ -245,10 +246,10 @@ class BookmarkNewViewTestCase(TestCase, BookmarkFactoryMixin):
html = response.content.decode() html = response.content.decode()
self.assertInHTML( self.assertInHTML(
""" """
<div class="form-input-hint"> <div id="id_shared_help" class="form-input-hint">
Share this bookmark with other registered users and anonymous users. Share this bookmark with other registered users and anonymous users.
</div> </div>
""", """,
html, html,
) )
@@ -265,7 +266,7 @@ class BookmarkNewViewTestCase(TestCase, BookmarkFactoryMixin):
html = response.content.decode() html = response.content.decode()
self.assertInHTML( self.assertInHTML(
'<input type="checkbox" name="unread" id="id_unread">', '<input type="checkbox" name="unread" id="id_unread" aria-describedby="id_unread_help">',
html, html,
) )
@@ -277,6 +278,6 @@ class BookmarkNewViewTestCase(TestCase, BookmarkFactoryMixin):
html = response.content.decode() html = response.content.decode()
self.assertInHTML( self.assertInHTML(
'<input type="checkbox" name="unread" id="id_unread" checked="">', '<input type="checkbox" name="unread" id="id_unread" checked="" aria-describedby="id_unread_help">',
html, html,
) )

View File

@@ -124,7 +124,7 @@ class BookmarkDetailsModalE2ETestCase(LinkdingE2ETestCase):
# Cancel edit, verify return to details url # Cancel edit, verify return to details url
details_url = url + f"&details={bookmark.id}" details_url = url + f"&details={bookmark.id}"
with self.page.expect_navigation(url=self.live_server_url + details_url): with self.page.expect_navigation(url=self.live_server_url + details_url):
self.page.get_by_text("Nevermind").click() self.page.get_by_text("Cancel").click()
def test_delete(self): def test_delete(self):
bookmark = self.setup_bookmark() bookmark = self.setup_bookmark()

View File

@@ -12,9 +12,9 @@ click==8.1.7
# via black # via black
coverage==7.6.1 coverage==7.6.1
# via -r requirements.dev.in # via -r requirements.dev.in
django==5.1.10 django==5.2.3
# via django-debug-toolbar # via django-debug-toolbar
django-debug-toolbar==4.4.6 django-debug-toolbar==5.2.0
# via -r requirements.dev.in # via -r requirements.dev.in
execnet==2.1.1 execnet==2.1.1
# via pytest-xdist # via pytest-xdist

View File

@@ -2,7 +2,7 @@
# This file is autogenerated by pip-compile with Python 3.12 # This file is autogenerated by pip-compile with Python 3.12
# by the following command: # by the following command:
# #
# pip-compile requirements.in # pip-compile
# #
asgiref==3.8.1 asgiref==3.8.1
# via django # via django
@@ -27,7 +27,7 @@ cryptography==43.0.1
# josepy # josepy
# mozilla-django-oidc # mozilla-django-oidc
# pyopenssl # pyopenssl
django==5.1.10 django==5.2.3
# via # via
# -r requirements.in # -r requirements.in
# django-registration # django-registration