Fix menu dropdown focus traps (#944)

bookmarks/frontend/behaviors/dropdown.js

@@ -6,23 +6,38 @@ class DropdownBehavior extends Behavior {
     this.opened = false;
     this.onClick = this.onClick.bind(this);
     this.onOutsideClick = this.onOutsideClick.bind(this);
+    this.onEscape = this.onEscape.bind(this);
+    this.onFocusOut = this.onFocusOut.bind(this);
+
+    // Prevent opening the dropdown automatically on focus, so that it only
+    // opens on click then JS is enabled
+    this.element.style.setProperty("--dropdown-focus-display", "none");
+    this.element.addEventListener("keydown", this.onEscape);
+    this.element.addEventListener("focusout", this.onFocusOut);
 
     this.toggle = element.querySelector(".dropdown-toggle");
+    this.toggle.setAttribute("aria-expanded", "false");
     this.toggle.addEventListener("click", this.onClick);
   }
 
   destroy() {
     this.close();
     this.toggle.removeEventListener("click", this.onClick);
+    this.element.removeEventListener("keydown", this.onEscape);
+    this.element.removeEventListener("focusout", this.onFocusOut);
   }
 
   open() {
+    this.opened = true;
     this.element.classList.add("active");
+    this.toggle.setAttribute("aria-expanded", "true");
     document.addEventListener("click", this.onOutsideClick);
   }
 
   close() {
+    this.opened = false;
     this.element.classList.remove("active");
+    this.toggle.setAttribute("aria-expanded", "false");
     document.removeEventListener("click", this.onOutsideClick);
   }
 
@@ -39,6 +54,20 @@ class DropdownBehavior extends Behavior {
       this.close();
     }
   }
+
+  onEscape(event) {
+    if (event.key === "Escape" && this.opened) {
+      event.preventDefault();
+      this.close();
+      this.toggle.focus();
+    }
+  }
+
+  onFocusOut(event) {
+    if (!this.element.contains(event.relatedTarget)) {
+      this.close();
+    }
+  }
 }
 
 registerBehavior("ld-dropdown", DropdownBehavior);

bookmarks/styles/theme/dropdowns.css

@@ -1,5 +1,7 @@
 /* Dropdown */
 .dropdown {
+  --dropdown-focus-display: block;
+
   display: inline-block;
   position: relative;
 
@@ -20,9 +22,13 @@
     }
   }
 
-  &.active .menu,
-  .dropdown-toggle:focus + .menu,
-  .menu:hover {
+  &:focus-within .menu {
+    /* Use custom CSS property to allow disabling opening on focus when using JS */
+    display: var(--dropdown-focus-display);
+  }
+
+  &.active .menu {
+    /* Always show menu when class is added through JS */
     display: block;
   }
 

bookmarks/templates/bookmarks/nav_menu.html

@@ -3,11 +3,11 @@
   {# Basic menu list #}
   <div class="hide-md">
     <a href="{% url 'bookmarks:new' %}" class="btn btn-primary mr-2">Add bookmark</a>
-    <div class="dropdown">
+    <div ld-dropdown class="dropdown">
       <button class="btn btn-link dropdown-toggle" tabindex="0">
         Bookmarks
       </button>
-      <ul class="menu">
+      <ul class="menu" role="list">
         <li class="menu-item">
           <a href="{% url 'bookmarks:index' %}" class="menu-link">Active</a>
         </li>
@@ -49,7 +49,7 @@
         </svg>
       </button>
       <!-- menu component -->
-      <ul class="menu">
+      <ul class="menu" role="list">
         <li class="menu-item">
           <a href="{% url 'bookmarks:index' %}" class="menu-link">Bookmarks</a>
         </li>

bookmarks/templates/bookmarks/search.html

@@ -15,7 +15,8 @@
     {% endfor %}
   </form>
   <div ld-dropdown class="search-options dropdown dropdown-right">
-    <button type="button" class="btn dropdown-toggle{% if search.has_modified_preferences %} badge{% endif %}">
+    <button type="button" aria-label="Search preferences"
+            class="btn dropdown-toggle{% if search.has_modified_preferences %} badge{% endif %}">
       <svg xmlns="http://www.w3.org/2000/svg" width="20" height="20" viewBox="0 0 24 24" stroke-width="2"
            stroke="currentColor" fill="none" stroke-linecap="round" stroke-linejoin="round">
         <path stroke="none" d="M0 0h24v24H0z" fill="none"></path>

bookmarks/tests/test_bookmark_search_tag.py

@@ -190,27 +190,26 @@ class BookmarkSearchTagTest(TestCase, BookmarkFactoryMixin, HtmlTestMixin):
         # Without modifications
         url = "/test"
         rendered_template = self.render_template(url)
+        soup = self.make_soup(rendered_template)
+        button = soup.select_one("button[aria-label='Search preferences']")
 
-        self.assertIn(
-            '<button type="button" class="btn dropdown-toggle">', rendered_template
-        )
+        self.assertNotIn("badge", button["class"])
 
         # With modifications
         url = "/test?sort=title_asc"
         rendered_template = self.render_template(url)
+        soup = self.make_soup(rendered_template)
+        button = soup.select_one("button[aria-label='Search preferences']")
 
-        self.assertIn(
-            '<button type="button" class="btn dropdown-toggle badge">',
-            rendered_template,
-        )
+        self.assertIn("badge", button["class"])
 
         # Ignores non-preferences modifications
         url = "/test?q=foo&user=john"
         rendered_template = self.render_template(url)
+        soup = self.make_soup(rendered_template)
+        button = soup.select_one("button[aria-label='Search preferences']")
 
-        self.assertIn(
-            '<button type="button" class="btn dropdown-toggle">', rendered_template
-        )
+        self.assertNotIn("badge", button["class"])
 
     def test_modified_labels(self):
         # Without modifications