From c4aac2ac1e469f0dc27b4dce230c7b29add617ba Mon Sep 17 00:00:00 2001 From: Rasmus Magnell <rasmus@personalkollen.se> Date: Sat, 24 Sep 2022 12:21:11 +0200 Subject: [PATCH] Fixed #34042 -- Improved accessibility of admin's navigation sidebar. --- .../admin/static/admin/css/nav_sidebar.css | 5 ++ .../admin/static/admin/js/nav_sidebar.js | 29 ++-------- .../admin/templates/admin/nav_sidebar.html | 2 +- tests/admin_views/test_nav_sidebar.py | 53 +++++++++++-------- 4 files changed, 39 insertions(+), 50 deletions(-) diff --git a/django/contrib/admin/static/admin/css/nav_sidebar.css b/django/contrib/admin/static/admin/css/nav_sidebar.css index 5fd2ff0bde..f76e6ce485 100644 --- a/django/contrib/admin/static/admin/css/nav_sidebar.css +++ b/django/contrib/admin/static/admin/css/nav_sidebar.css @@ -59,8 +59,13 @@ content: '\00AB'; } +.main > #nav-sidebar { + visibility: hidden; +} + .main.shifted > #nav-sidebar { margin-left: 0; + visibility: visible; } [dir="rtl"] .main.shifted > #nav-sidebar { diff --git a/django/contrib/admin/static/admin/js/nav_sidebar.js b/django/contrib/admin/static/admin/js/nav_sidebar.js index 261a9d4992..7e735db15c 100644 --- a/django/contrib/admin/static/admin/js/nav_sidebar.js +++ b/django/contrib/admin/static/admin/js/nav_sidebar.js @@ -2,47 +2,24 @@ { const toggleNavSidebar = document.getElementById('toggle-nav-sidebar'); if (toggleNavSidebar !== null) { - const navLinks = document.querySelectorAll('#nav-sidebar a'); - function disableNavLinkTabbing() { - for (const navLink of navLinks) { - navLink.tabIndex = -1; - } - } - function enableNavLinkTabbing() { - for (const navLink of navLinks) { - navLink.tabIndex = 0; - } - } - function disableNavFilterTabbing() { - document.getElementById('nav-filter').tabIndex = -1; - } - function enableNavFilterTabbing() { - document.getElementById('nav-filter').tabIndex = 0; - } - + const navSidebar = document.getElementById('nav-sidebar'); const main = document.getElementById('main'); let navSidebarIsOpen = localStorage.getItem('django.admin.navSidebarIsOpen'); if (navSidebarIsOpen === null) { navSidebarIsOpen = 'true'; } - if (navSidebarIsOpen === 'false') { - disableNavLinkTabbing(); - disableNavFilterTabbing(); - } main.classList.toggle('shifted', navSidebarIsOpen === 'true'); + navSidebar.setAttribute('aria-expanded', navSidebarIsOpen); toggleNavSidebar.addEventListener('click', function() { if (navSidebarIsOpen === 'true') { navSidebarIsOpen = 'false'; - disableNavLinkTabbing(); - disableNavFilterTabbing(); } else { navSidebarIsOpen = 'true'; - enableNavLinkTabbing(); - enableNavFilterTabbing(); } localStorage.setItem('django.admin.navSidebarIsOpen', navSidebarIsOpen); main.classList.toggle('shifted'); + navSidebar.setAttribute('aria-expanded', navSidebarIsOpen); }); } diff --git a/django/contrib/admin/templates/admin/nav_sidebar.html b/django/contrib/admin/templates/admin/nav_sidebar.html index 6ddff01d63..a413e23754 100644 --- a/django/contrib/admin/templates/admin/nav_sidebar.html +++ b/django/contrib/admin/templates/admin/nav_sidebar.html @@ -1,6 +1,6 @@ {% load i18n %} <button class="sticky toggle-nav-sidebar" id="toggle-nav-sidebar" aria-label="{% translate 'Toggle navigation' %}"></button> -<nav class="sticky" id="nav-sidebar"> +<nav class="sticky" id="nav-sidebar" aria-label="{% translate 'Sidebar' %}"> <input type="search" id="nav-filter" placeholder="{% translate 'Start typing to filter…' %}" aria-label="{% translate 'Filter navigation items' %}"> diff --git a/tests/admin_views/test_nav_sidebar.py b/tests/admin_views/test_nav_sidebar.py index 3cb20e4068..e9b367b63b 100644 --- a/tests/admin_views/test_nav_sidebar.py +++ b/tests/admin_views/test_nav_sidebar.py @@ -43,21 +43,29 @@ class AdminSidebarTests(TestCase): def test_sidebar_not_on_index(self): response = self.client.get(reverse("test_with_sidebar:index")) self.assertContains(response, '<div class="main" id="main">') - self.assertNotContains(response, '<nav class="sticky" id="nav-sidebar">') + self.assertNotContains( + response, '<nav class="sticky" id="nav-sidebar" aria-label="Sidebar">' + ) def test_sidebar_disabled(self): response = self.client.get(reverse("test_without_sidebar:index")) - self.assertNotContains(response, '<nav class="sticky" id="nav-sidebar">') + self.assertNotContains( + response, '<nav class="sticky" id="nav-sidebar" aria-label="Sidebar">' + ) def test_sidebar_unauthenticated(self): self.client.logout() response = self.client.get(reverse("test_with_sidebar:login")) - self.assertNotContains(response, '<nav class="sticky" id="nav-sidebar">') + self.assertNotContains( + response, '<nav class="sticky" id="nav-sidebar" aria-label="Sidebar">' + ) def test_sidebar_aria_current_page(self): url = reverse("test_with_sidebar:auth_user_changelist") response = self.client.get(url) - self.assertContains(response, '<nav class="sticky" id="nav-sidebar">') + self.assertContains( + response, '<nav class="sticky" id="nav-sidebar" aria-label="Sidebar">' + ) self.assertContains( response, '<a href="%s" aria-current="page">Users</a>' % url ) @@ -80,7 +88,9 @@ class AdminSidebarTests(TestCase): def test_sidebar_aria_current_page_missing_without_request_context_processor(self): url = reverse("test_with_sidebar:auth_user_changelist") response = self.client.get(url) - self.assertContains(response, '<nav class="sticky" id="nav-sidebar">') + self.assertContains( + response, '<nav class="sticky" id="nav-sidebar" aria-label="Sidebar">' + ) # Does not include aria-current attribute. self.assertContains(response, '<a href="%s">Users</a>' % url) self.assertNotContains(response, "aria-current") @@ -146,16 +156,15 @@ class SeleniumTests(AdminSeleniumTestCase): ) self.assertEqual(toggle_button.tag_name, "button") self.assertEqual(toggle_button.get_attribute("aria-label"), "Toggle navigation") - for link in self.selenium.find_elements(By.CSS_SELECTOR, "#nav-sidebar a"): - self.assertEqual(link.get_attribute("tabIndex"), "0") - filter_input = self.selenium.find_element(By.CSS_SELECTOR, "#nav-filter") - self.assertEqual(filter_input.get_attribute("tabIndex"), "0") + nav_sidebar = self.selenium.find_element(By.ID, "nav-sidebar") + self.assertEqual(nav_sidebar.get_attribute("aria-expanded"), "true") + self.assertTrue(nav_sidebar.is_displayed()) toggle_button.click() - # Hidden sidebar is not reachable via keyboard navigation. - for link in self.selenium.find_elements(By.CSS_SELECTOR, "#nav-sidebar a"): - self.assertEqual(link.get_attribute("tabIndex"), "-1") - filter_input = self.selenium.find_element(By.CSS_SELECTOR, "#nav-filter") - self.assertEqual(filter_input.get_attribute("tabIndex"), "-1") + + # Hidden sidebar is not visible. + nav_sidebar = self.selenium.find_element(By.ID, "nav-sidebar") + self.assertEqual(nav_sidebar.get_attribute("aria-expanded"), "false") + self.assertFalse(nav_sidebar.is_displayed()) main_element = self.selenium.find_element(By.CSS_SELECTOR, "#main") self.assertNotIn("shifted", main_element.get_attribute("class").split()) @@ -189,16 +198,14 @@ class SeleniumTests(AdminSeleniumTestCase): toggle_button = self.selenium.find_element( By.CSS_SELECTOR, "#toggle-nav-sidebar" ) - # Hidden sidebar is not reachable via keyboard navigation. - for link in self.selenium.find_elements(By.CSS_SELECTOR, "#nav-sidebar a"): - self.assertEqual(link.get_attribute("tabIndex"), "-1") - filter_input = self.selenium.find_element(By.CSS_SELECTOR, "#nav-filter") - self.assertEqual(filter_input.get_attribute("tabIndex"), "-1") + # Hidden sidebar is not visible. + nav_sidebar = self.selenium.find_element(By.ID, "nav-sidebar") + self.assertEqual(nav_sidebar.get_attribute("aria-expanded"), "false") + self.assertFalse(nav_sidebar.is_displayed()) toggle_button.click() - for link in self.selenium.find_elements(By.CSS_SELECTOR, "#nav-sidebar a"): - self.assertEqual(link.get_attribute("tabIndex"), "0") - filter_input = self.selenium.find_element(By.CSS_SELECTOR, "#nav-filter") - self.assertEqual(filter_input.get_attribute("tabIndex"), "0") + nav_sidebar = self.selenium.find_element(By.ID, "nav-sidebar") + self.assertEqual(nav_sidebar.get_attribute("aria-expanded"), "true") + self.assertTrue(nav_sidebar.is_displayed()) self.assertEqual( self.selenium.execute_script( "return localStorage.getItem('django.admin.navSidebarIsOpen')"