Reintegrate the dropdown patch using fomantic hooks and template changes

[zeripath]

• Gitea version (or commit ref): 1.16+dev

Description

The dropdown patch from #8638 is somewhat of a blackbox with little shared understanding as to how it works and why. However, looking again at it I think there are ways that some of its work could be done using hooks already present in fomantic and with template changes.

I would like to use this issue to discuss parts of the patch and the reasoning for these

I hope @Jookia would be able to help explain this patch.

[zeripath]

First of all I'm going to provide the original patch in its entirety:

Original Patch

--- semanti.dropdown.js.original	2018-03-19 04:04:27.000000000 +0000
+++ semantic.dropdown.js	2019-11-03 20:24:41.929563833 +0000
@@ ---- semanti.dropdown.js.original	2018-03-19 04:04:27.000000000 +0000
+++ semantic.dropdown.js	2019-11-03 20:24:41.929563833 +0000
@@ -8,6 +8,13 @@
  *
  */
 
+/*
+ * Copyright 2019 The Gitea Authors
+ * Released under the MIT license
+ * http://opensource.org/licenses/MIT
+ * This version has been modified by Gitea to improve accessibility.
+ */
+
 ;(function ($, window, document, undefined) {
 
 'use strict';
@@ -33,6 +40,7 @@
     query          = arguments[0],
     methodInvoked  = (typeof query == 'string'),
     queryArguments = [].slice.call(arguments, 1),
+    lastAriaID = 1,
     returnedValue
   ;
 
@@ -114,6 +122,8 @@
 
             module.observeChanges();
             module.instantiate();
+
+            module.aria.setup();
           }
 
         },
@@ -296,6 +306,86 @@
           }
         },
 
+        aria: {
+          setup: function() {
+            var role = module.aria.guessRole();
+            if( role !== 'menu' ) {
+              return;
+            }
+            $module.attr('aria-busy', 'true');
+            $module.attr('role', 'menu');
+            $module.attr('aria-haspopup', 'menu');
+            $module.attr('aria-expanded', 'false');
+            $menu.find('.divider').attr('role', 'separator');
+            $item.attr('role', 'menuitem');
+            $item.each(function (index, item) {
+              if( !item.id ) {
+                item.id = module.aria.nextID('menuitem');
+              }
+            });
+            $text = $module
+              .find('> .text')
+              .eq(0)
+            ;
+            if( $module.data('content') ) {
+              $text.attr('aria-hidden');
+              $module.attr('aria-label', $module.data('content'));
+            }
+            else {
+              $text.attr('id', module.aria.nextID('menutext'));
+              $module.attr('aria-labelledby', $text.attr('id'));
+            }
+            $module.attr('aria-busy', 'false');
+          },
+          nextID: function(prefix) {
+            var nextID;
+            do {
+              nextID = prefix + '_' + lastAriaID++;
+            } while( document.getElementById(nextID) );
+            return nextID;
+          },
+          setExpanded: function(expanded) {
+            if( $module.attr('aria-haspopup') ) {
+              $module.attr('aria-expanded', expanded);
+            }
+          },
+          refreshDescendant: function() {
+            if( $module.attr('aria-haspopup') !== 'menu' ) {
+              return;
+            }
+            var
+              $currentlySelected = $item.not(selector.unselectable).filter('.' + className.selected).eq(0),
+              $activeItem        = $menu.children('.' + className.active).eq(0),
+              $selectedItem      = ($currentlySelected.length > 0)
+                ? $currentlySelected
+                : $activeItem
+            ;
+            if( $selectedItem ) {
+              $module.attr('aria-activedescendant', $selectedItem.attr('id'));
+            }
+            else {
+              module.aria.removeDescendant();
+            }
+          },
+          removeDescendant: function() {
+            if( $module.attr('aria-haspopup') == 'menu' ) {
+              $module.removeAttr('aria-activedescendant');
+            }
+          },
+          guessRole: function() {
+            var
+              isIcon = $module.hasClass('icon'),
+              hasSearch = module.has.search(),
+              hasInput = ($input.length > 0),
+              isMultiple = module.is.multiple()
+            ;
+            if ( !isIcon && !hasSearch && !hasInput && !isMultiple ) {
+              return 'menu';
+            }
+            return 'unknown';
+          }
+        },
+
         setup: {
           api: function() {
             var
@@ -335,6 +425,7 @@
             if(settings.allowTab) {
               module.set.tabbable();
             }
+            $item.attr('tabindex', '-1');
           },
           select: function() {
             var
@@ -477,6 +568,8 @@
               return true;
             }
             if(settings.onShow.call(element) !== false) {
+              module.aria.setExpanded(true);
+              module.aria.refreshDescendant();
               module.animate.show(function() {
                 if( module.can.click() ) {
                   module.bind.intent();
@@ -499,6 +592,8 @@
           if( module.is.active() && !module.is.animatingOutward() ) {
             module.debug('Hiding dropdown');
             if(settings.onHide.call(element) !== false) {
+              module.aria.setExpanded(false);
+              module.aria.removeDescendant();
               module.animate.hide(function() {
                 module.remove.visible();
                 callback.call(element);
@@ -902,7 +997,7 @@
           ;
           if(hasSelected && !module.is.multiple()) {
             module.debug('Forcing partial selection to selected item', $selectedItem);
-            module.event.item.click.call($selectedItem, {}, true);
+            $selectedItem[0].click();
             return;
           }
           else {
@@ -1363,7 +1458,7 @@
               // allow selection with menu closed
               if(isAdditionWithoutMenu) {
                 module.verbose('Selecting item from keyboard shortcut', $selectedItem);
-                module.event.item.click.call($selectedItem, event);
+                $selectedItem[0].click();
                 if(module.is.searchSelection()) {
                   module.remove.searchTerm();
                 }
@@ -1380,7 +1475,7 @@
                   }
                   else if(selectedIsSelectable) {
                     module.verbose('Selecting item from keyboard shortcut', $selectedItem);
-                    module.event.item.click.call($selectedItem, event);
+                    $selectedItem[0].click();
                     if(module.is.searchSelection()) {
                       module.remove.searchTerm();
                     }
@@ -1405,6 +1500,7 @@
                         .closest(selector.item)
                           .addClass(className.selected)
                       ;
+                      module.aria.refreshDescendant();
                       event.preventDefault();
                     }
                   }
@@ -1421,6 +1517,7 @@
                         .find(selector.item).eq(0)
                           .addClass(className.selected)
                       ;
+                      module.aria.refreshDescendant();
                       event.preventDefault();
                     }
                   }
@@ -1445,6 +1542,7 @@
                     $nextItem
                       .addClass(className.selected)
                     ;
+                    module.aria.refreshDescendant();
                     module.set.scrollPosition($nextItem);
                     if(settings.selectOnKeydown && module.is.single()) {
                       module.set.selectedItem($nextItem);
@@ -1472,6 +1570,7 @@
                     $nextItem
                       .addClass(className.selected)
                     ;
+                    module.aria.refreshDescendant();
                     module.set.scrollPosition($nextItem);
                     if(settings.selectOnKeydown && module.is.single()) {
                       module.set.selectedItem($nextItem);
@@ -2399,6 +2498,7 @@
               module.set.scrollPosition($nextValue);
               $selectedItem.removeClass(className.selected);
               $nextValue.addClass(className.selected);
+              module.aria.refreshDescendant();
               if(settings.selectOnKeydown && module.is.single()) {
                 module.set.selectedItem($nextValue);
               }
8,6 +8,13 @@
  *
  */
 
+/*
+ * Copyright 2019 The Gitea Authors
+ * Released under the MIT license
+ * http://opensource.org/licenses/MIT
+ * This version has been modified by Gitea to improve accessibility.
+ */
+
 ;(function ($, window, document, undefined) {
 
 'use strict';
@@ -33,6 +40,7 @@
     query          = arguments[0],
     methodInvoked  = (typeof query == 'string'),
     queryArguments = [].slice.call(arguments, 1),
+    lastAriaID = 1,
     returnedValue
   ;
 
@@ -114,6 +122,8 @@
 
             module.observeChanges();
             module.instantiate();
+
+            module.aria.setup();
           }
 
         },
@@ -296,6 +306,86 @@
           }
         },
 
+        aria: {
+          setup: function() {
+            var role = module.aria.guessRole();
+            if( role !== 'menu' ) {
+              return;
+            }
+            $module.attr('aria-busy', 'true');
+            $module.attr('role', 'menu');
+            $module.attr('aria-haspopup', 'menu');
+            $module.attr('aria-expanded', 'false');
+            $menu.find('.divider').attr('role', 'separator');
+            $item.attr('role', 'menuitem');
+            $item.each(function (index, item) {
+              if( !item.id ) {
+                item.id = module.aria.nextID('menuitem');
+              }
+            });
+            $text = $module
+              .find('> .text')
+              .eq(0)
+            ;
+            if( $module.data('content') ) {
+              $text.attr('aria-hidden');
+              $module.attr('aria-label', $module.data('content'));
+            }
+            else {
+              $text.attr('id', module.aria.nextID('menutext'));
+              $module.attr('aria-labelledby', $text.attr('id'));
+            }
+            $module.attr('aria-busy', 'false');
+          },
+          nextID: function(prefix) {
+            var nextID;
+            do {
+              nextID = prefix + '_' + lastAriaID++;
+            } while( document.getElementById(nextID) );
+            return nextID;
+          },
+          setExpanded: function(expanded) {
+            if( $module.attr('aria-haspopup') ) {
+              $module.attr('aria-expanded', expanded);
+            }
+          },
+          refreshDescendant: function() {
+            if( $module.attr('aria-haspopup') !== 'menu' ) {
+              return;
+            }
+            var
+              $currentlySelected = $item.not(selector.unselectable).filter('.' + className.selected).eq(0),
+              $activeItem        = $menu.children('.' + className.active).eq(0),
+              $selectedItem      = ($currentlySelected.length > 0)
+                ? $currentlySelected
+                : $activeItem
+            ;
+            if( $selectedItem ) {
+              $module.attr('aria-activedescendant', $selectedItem.attr('id'));
+            }
+            else {
+              module.aria.removeDescendant();
+            }
+          },
+          removeDescendant: function() {
+            if( $module.attr('aria-haspopup') == 'menu' ) {
+              $module.removeAttr('aria-activedescendant');
+            }
+          },
+          guessRole: function() {
+            var
+              isIcon = $module.hasClass('icon'),
+              hasSearch = module.has.search(),
+              hasInput = ($input.length > 0),
+              isMultiple = module.is.multiple()
+            ;
+            if ( !isIcon && !hasSearch && !hasInput && !isMultiple ) {
+              return 'menu';
+            }
+            return 'unknown';
+          }
+        },
+
         setup: {
           api: function() {
             var
@@ -335,6 +425,7 @@
             if(settings.allowTab) {
               module.set.tabbable();
             }
+            $item.attr('tabindex', '-1');
           },
           select: function() {
             var
@@ -477,6 +568,8 @@
               return true;
             }
             if(settings.onShow.call(element) !== false) {
+              module.aria.setExpanded(true);
+              module.aria.refreshDescendant();
               module.animate.show(function() {
                 if( module.can.click() ) {
                   module.bind.intent();
@@ -499,6 +592,8 @@
           if( module.is.active() && !module.is.animatingOutward() ) {
             module.debug('Hiding dropdown');
             if(settings.onHide.call(element) !== false) {
+              module.aria.setExpanded(false);
+              module.aria.removeDescendant();
               module.animate.hide(function() {
                 module.remove.visible();
                 callback.call(element);
@@ -902,7 +997,7 @@
           ;
           if(hasSelected && !module.is.multiple()) {
             module.debug('Forcing partial selection to selected item', $selectedItem);
-            module.event.item.click.call($selectedItem, {}, true);
+            $selectedItem[0].click();
             return;
           }
           else {
@@ -1363,7 +1458,7 @@
               // allow selection with menu closed
               if(isAdditionWithoutMenu) {
                 module.verbose('Selecting item from keyboard shortcut', $selectedItem);
-                module.event.item.click.call($selectedItem, event);
+                $selectedItem[0].click();
                 if(module.is.searchSelection()) {
                   module.remove.searchTerm();
                 }
@@ -1380,7 +1475,7 @@
                   }
                   else if(selectedIsSelectable) {
                     module.verbose('Selecting item from keyboard shortcut', $selectedItem);
-                    module.event.item.click.call($selectedItem, event);
+                    $selectedItem[0].click();
                     if(module.is.searchSelection()) {
                       module.remove.searchTerm();
                     }
@@ -1405,6 +1500,7 @@
                         .closest(selector.item)
                           .addClass(className.selected)
                       ;
+                      module.aria.refreshDescendant();
                       event.preventDefault();
                     }
                   }
@@ -1421,6 +1517,7 @@
                         .find(selector.item).eq(0)
                           .addClass(className.selected)
                       ;
+                      module.aria.refreshDescendant();
                       event.preventDefault();
                     }
                   }
@@ -1445,6 +1542,7 @@
                     $nextItem
                       .addClass(className.selected)
                     ;
+                    module.aria.refreshDescendant();
                     module.set.scrollPosition($nextItem);
                     if(settings.selectOnKeydown && module.is.single()) {
                       module.set.selectedItem($nextItem);
@@ -1472,6 +1570,7 @@
                     $nextItem
                       .addClass(className.selected)
                     ;
+                    module.aria.refreshDescendant();
                     module.set.scrollPosition($nextItem);
                     if(settings.selectOnKeydown && module.is.single()) {
                       module.set.selectedItem($nextItem);
@@ -2399,6 +2498,7 @@
               module.set.scrollPosition($nextValue);
               $selectedItem.removeClass(className.selected);
               $nextValue.addClass(className.selected);
+              module.aria.refreshDescendant();
               if(settings.selectOnKeydown && module.is.single()) {
                 module.set.selectedItem($nextValue);
               }

[zeripath]

The patch in #16576 differs in that these sections of the patch have been deleted:

@@ -902,7 +997,7 @@
           ;
           if(hasSelected && !module.is.multiple()) {
             module.debug('Forcing partial selection to selected item', $selectedItem);
-            module.event.item.click.call($selectedItem, {}, true);
+            $selectedItem[0].click();
             return;
           }
           else {
@@ -1363,7 +1458,7 @@
               // allow selection with menu closed
               if(isAdditionWithoutMenu) {
                 module.verbose('Selecting item from keyboard shortcut', $selectedItem);
-                module.event.item.click.call($selectedItem, event);
+                $selectedItem[0].click();
                 if(module.is.searchSelection()) {
                   module.remove.searchTerm();
                 }
@@ -1363,7 +1458,7 @@
               // allow selection with menu closed
               if(isAdditionWithoutMenu) {
                 module.verbose('Selecting item from keyboard shortcut', $selectedItem);
-                module.event.item.click.call($selectedItem, event);
+                $selectedItem[0].click();
                 if(module.is.searchSelection()) {
                   module.remove.searchTerm();
                 }

These have been removed because with the updated JQuery these prevent the dropdown from ever losing focus.

---

Now, the second and third of these don't appear to be different from module.event.item.click.call(...) but I'm not certain. The first however, does have a difference in that the the 3rd argument to module.event.item.click.call prevents the dropdown from recapturing focus and it is this third argument that will be missed when you replace it with a jQuery click.

So I guess the question is what was the aim of this?

[zeripath]

module.aria.setup

@@ -114,6 +122,8 @@
 
             module.observeChanges();
             module.instantiate();
+
+            module.aria.setup();
           }
 
         },

This part of the patch calls module.aria.setup() at the end of setup of dropdown.

Looking module.aria.setup this first of all calls guessRole which does some heurisitic checking:

+          guessRole: function() {
+            var
+              isIcon = $module.hasClass('icon'),
+              hasSearch = module.has.search(),
+              hasInput = ($input.length > 0),
+              isMultiple = module.is.multiple()
+            ;
+            if ( !isIcon && !hasSearch && !hasInput && !isMultiple ) {
+              return 'menu';
+            }
+            return 'unknown';
+          }
+        },

and if so it will set aria-busy=true, then add some aria attributes to the dropdown, before unsetting the aria-busy:

@@ -296,6 +306,86 @@
           }
         },
 
+        aria: {
+          setup: function() {
+            var role = module.aria.guessRole();
+            if( role !== 'menu' ) {
+              return;
+            }
+            $module.attr('aria-busy', 'true');
+            $module.attr('role', 'menu');
+            $module.attr('aria-haspopup', 'menu');
+            $module.attr('aria-expanded', 'false');
+            $menu.find('.divider').attr('role', 'separator');
+            $item.attr('role', 'menuitem');
+            $item.each(function (index, item) {
+              if( !item.id ) {
+                item.id = module.aria.nextID('menuitem');
+              }
+            });
+            $text = $module
+              .find('> .text')
+              .eq(0)
+            ;
+            if( $module.data('content') ) {
+              $text.attr('aria-hidden');
+              $module.attr('aria-label', $module.data('content'));
+            }
+            else {
+              $text.attr('id', module.aria.nextID('menutext'));
+              $module.attr('aria-labelledby', $text.attr('id'));
+            }
+            $module.attr('aria-busy', 'false');
+          },
...

---

1. From my reading of ARIA documentation - as inconsistent as it appears to be - I thought role="menu" was discouraged except were it definitely represents a menu.
2. I think that most of these changes should really be done at the template level. The ids could be autogenerated using js if they're really necessary.

[zeripath]

Set items tabindex -1

@@ -335,6 +425,7 @@
             if(settings.allowTab) {
               module.set.tabbable();
             }
+            $item.attr('tabindex', '-1');
           },
           select: function() {
             var

This sets the tabindex to -1 on all of the items - to match roving tabindex and to make the dropdown feel a lot more a select I guess - but I'm not sure if this is really all that necessary as tabbing works fine without it at present.

---

Is this really necessary? It could actually be set in onShow though.

[zeripath]

setExpanded and refreshDescendant

...
+          setExpanded: function(expanded) {
+            if( $module.attr('aria-haspopup') ) {
+              $module.attr('aria-expanded', expanded);
+            }
+          },
+          refreshDescendant: function() {
+            if( $module.attr('aria-haspopup') !== 'menu' ) {
+              return;
+            }
+            var
+              $currentlySelected = $item.not(selector.unselectable).filter('.' + className.selected).eq(0),
+              $activeItem        = $menu.children('.' + className.active).eq(0),
+              $selectedItem      = ($currentlySelected.length > 0)
+                ? $currentlySelected
+                : $activeItem
+            ;
+            if( $selectedItem ) {
+              $module.attr('aria-activedescendant', $selectedItem.attr('id'));
+            }
+            else {
+              module.aria.removeDescendant();
+            }
+          },
+          removeDescendant: function() {
+            if( $module.attr('aria-haspopup') == 'menu' ) {
+              $module.removeAttr('aria-activedescendant');
+            }
+          },
...
@@ -477,6 +568,8 @@
               return true;
             }
             if(settings.onShow.call(element) !== false) {
+              module.aria.setExpanded(true);
+              module.aria.refreshDescendant();
               module.animate.show(function() {
                 if( module.can.click() ) {
                   module.bind.intent();
@@ -499,6 +592,8 @@
           if( module.is.active() && !module.is.animatingOutward() ) {
             module.debug('Hiding dropdown');
             if(settings.onHide.call(element) !== false) {
+              module.aria.setExpanded(false);
+              module.aria.removeDescendant();
               module.animate.hide(function() {
                 module.remove.visible();

First of all both of these functions appear to be being called straight after onShow and onHide which are programmable hooks for dropdown and otherwise unused in Gitea so they should/could just be integrated there.

Essentially these functions appear to keep track of the aria-activedescendant and the aria-haspopup attributes on the dropdown.

---

1. Should these just be being hooked in as onShow and onHide functions?

[Jookia]

So the best way to look at this patch is to look through my mindset of what I could do to immediately make things a little better without putting load on maintainers to actually fix things, since it didn't seem like maintainers wanted to spend time refactoring HTML or learning ARIA in order to fix things properly. I was also learning ARIA at the time specifically to make this patch, and getting pretty burned out in the process. In the end I was looking at the specific solution of getting some menus working so my blind friend could use Gitea better.

For what it's worth, I had a lot of explanations in git commit messages but these got squashed and removed I think. I was grumpy about this for this exact reason.

Gitea uses dropdowns everywhere. Not just for menus, but also for search boxes, tag selection, branch selection, everywhere. These widgets range from 'listbox' to 'combobox' to other fancy widgets that do multiple things such as manage entries in a list and letting you add new entries by searching in the entry list and adding new tags. Sometimes there's even listboxes that have buttons to open the listbox with an arrow, and those are marked as icons. All of these use Fomantic's 'dropdown' UI.

So the first thing to really do is try and find some actual listboxes (which I incorrectly refer to as menu):

+          guessRole: function() {
+            var
+              isIcon = $module.hasClass('icon'),
+              hasSearch = module.has.search(),
+              hasInput = ($input.length > 0),
+              isMultiple = module.is.multiple()
+            ;
+            if ( !isIcon && !hasSearch && !hasInput && !isMultiple ) {
+              return 'menu';
+            }
+            return 'unknown';
+          }
+        },
+

The criteria here is that:

• It's not also an icon CSS-wise
• It's not a search box
• It doesn't have any way to input text
• There are multiple elements in the dropdown

That seems like it's MAYBE a dropdown. The correct solution here would be to go through every dropdown in Gitea's source code and categorize what it is and how to actually handle it, because for each dropdown Fomantic also adds some kind of Javascript, even on the little dropdown icons people are supposed to click.

The next step is to decide what ARIA widget this is and start implementing its role, as a contract between us and any assistive technology. I chose 'menubar' because I wasn't sure what to pick, but in retrospect it should've been 'listbox'. In any case, I did this wrong since I was trying to fit a square in to a round hole.

• The role 'menu' should be on a button element that opens the menu. I figured the dropdown div was close enough.
• The attribute 'aria-haspopup' is set to 'menu' on the button. Again, not a button, so invalid.
• The attribute 'aria-expanded' is set to 'true' when the menu is open. It should be removed instead of set to 'false' when the menu is closed, but I didn't want to deal with that logic and figured it was in spec to just set it to 'false'.
• Each element in the menu is given the role 'menuitem' and a unique ID, so we can point to them later for focus management
• aria-label/aria-labelledby is set to whatever elements or text titles menu button
• Any text that's labeled is hidden using aria-hidden to hack around it being read twice
• While all ARIA stuff is being set, we use aria-busy to avoid any assistive technology reading incomplete state

+        aria: {
+          setup: function() {
+            var role = module.aria.guessRole();
+            if( role !== 'menu' ) {
+              return;
+            }
+            $module.attr('aria-busy', 'true');
+            $module.attr('role', 'menu');
+            $module.attr('aria-haspopup', 'menu');
+            $module.attr('aria-expanded', 'false');
+            $menu.find('.divider').attr('role', 'separator');
+            $item.attr('role', 'menuitem');
+            $item.each(function (index, item) {
+              if( !item.id ) {
+                item.id = module.aria.nextID('menuitem');
+              }
+            });
+            $text = $module
+              .find('> .text')
+              .eq(0)
+            ;
+            if( $module.data('content') ) {
+              $text.attr('aria-hidden');
+              $module.attr('aria-label', $module.data('content'));
+            }
+            else {
+              $text.attr('id', module.aria.nextID('menutext'));
+              $module.attr('aria-labelledby', $text.attr('id'));
+            }
+            $module.attr('aria-busy', 'false');
+          },
+          nextID: function(prefix) {
+            var nextID;
+            do {
+              nextID = prefix + '_' + lastAriaID++;
+            } while( document.getElementById(nextID) );
+            return nextID;
+          },

The next thing to think about is focus management. This is what most dropdowns on the Internet get wrong. When you use a keyboard to access ARIA widgets, you don't want to use tab, you want to just arrows and specific keys mandated by ARIA. This means for example that you can tab past listboxes without tabbing through each element in the listbox. If a person wants to use the dropdown, they'll open it and use the arrows.

It's also important to note that assistive technology have a thing called 'browse mode'- this is where the browser allows assistive technology to capture all key presses on a website. This is so a user can navigate a page using the keyboard arrows to step through each line of text, or jump to headings. This is much faster than tabbing through every element on a page, but it's also something that only really comes in to play for screen readers as far as I can tell. So people unable to use a mouse still have to tab through everything, but people with screen readers can skip it.

Quoting https://www.w3.org/TR/wai-aria-1.2/#application :

Some user agents and assistive technologies have a browse mode where standard input events, such as up and down arrow key events, are intercepted and used to control a reading cursor. This browse mode behavior prevents elements that do not have a widget role from receiving and using such keyboard and gesture events to provide interactive functionality.

The lack of focus of individual elements means the assistive technology can no longer track and read out what items are selected, which is why we need to hint it:

Because only the focusable elements contained in an application element are accessible to users of some assistive technologies, authors MUST use one of the following techniques to ensure all non-decorative static text or image content inside an application is accessible:

• Associate the content with a focusable element using aria-labelledby or aria-describedby.
• Place the content in a focusable element that has role document or article.
• Manage focus of owned elements as described in Managing Focus, updating the value of aria-activedescendant to reference the element containing the focused content.

I think widgets should manage their own focus since it provides a less confusing experiences- you can't accidentally focus subelements using tab when you're not supposed to and get in to an invalid and confusing state.

With that in mind, we set the tabindex to -1 so NOTHING inside the dropdown is focusable:

> +            $item.attr('tabindex', '-1');

We also should be setting the icon next to the menu to tabindex -1 so we don't have to tab past it:

commit a404e8dc86305ef7b3087b50d0f4448f5e5d8cab
Author: Jookia <166291@gmail.com>
Date:   2019-10-23 10:30:07 +1100

    js: Set tabindex=-1 on child dropdown icons
    
    This fixes having to effectively tab past menus twice: Once for the menu
    itself, once for the icon that just opens the menu.

diff --git a/public/js/semantic.dropdown.js b/public/js/semantic.dropdown.js
index 889429578..9b408ac9e 100644
--- a/public/js/semantic.dropdown.js
+++ b/public/js/semantic.dropdown.js
@@ -414,6 +414,7 @@ $.fn.dropdown = function(parameters) {
               module.set.tabbable();
             }
             $item.attr("tabindex", "-1");
+            $icon.attr("tabindex", "-1");
           },
           select: function() {
             var

But that might not have been upstreamed or something.

We set aria-expanded when that changes:

+          setExpanded: function(expanded) {
+            if( $module.attr('aria-haspopup') ) {
+              $module.attr('aria-expanded', expanded);
+            }
+          },

Then whenever anything changes, we update the aria-activedescent attribute to point to the selected item:

+          refreshDescendant: function() {
+            if( $module.attr('aria-haspopup') !== 'menu' ) {
+              return;
+            }
+            var
+              $currentlySelected = $item.not(selector.unselectable).filter('.' + className.selected).eq(0),
+              $activeItem        = $menu.children('.' + className.active).eq(0),
+              $selectedItem      = ($currentlySelected.length > 0)
+                ? $currentlySelected
+                : $activeItem
+            ;
+            if( $selectedItem ) {
+              $module.attr('aria-activedescendant', $selectedItem.attr('id'));
+            }
+            else {
+              module.aria.removeDescendant();
+            }
+          },
+          removeDescendant: function() {
+            if( $module.attr('aria-haspopup') == 'menu' ) {
+              $module.removeAttr('aria-activedescendant');
+            }
+          },

This does not work for listboxes that let you select multiple things, only one or no elements.

Calls to these hooks are peppers basically whenever input happens or the dropdown is shown. This is because I didn't wanted to dig in the dropdown spaghetti and wanted to instead make sure it worked each time.

Now, when it's time to select an element, the dropdown code calls module.event.item.click.call. This doesn't actually call onclick() handlers all the time. Instead you have to directly call click(). I noted this in my now squashed commit message:

commit de6e43497122723922f51a460aa60c2f0cee176b
Author: Jookia <166291@gmail.com>
Date:   2019-10-21 11:07:45 +1100

    js: Don't use jQuery to click menu items
    
    Menu items are often <a> elements, which jQuery refuses to trigger click
    events on. Instead it just bubbles up to the menu.
    
    Using HTMLElement's click method fixes this and makes menu items
    clickable from the keyboard using dropdown menus.

I had to revert this in one case later:

commit 9fa89002cd6941682b06a7e9f40c83880cf5e193
Author: Jookia <166291@gmail.com>
Date:   2019-11-11 18:47:43 +1100

    js: Revert change to click behaviour in forceSelection
    
    Calling .click() in forceSelection() in blur() causes a focus loop,
    this needs a proper fix later but for now just revert the change.

diff --git a/public/js/semantic.dropdown.custom.js b/public/js/semantic.dropdown.custom.js
index 1745869fb..82e6efb2e 100644
--- a/public/js/semantic.dropdown.custom.js
+++ b/public/js/semantic.dropdown.custom.js
@@ -997,7 +997,7 @@ $.fn.dropdown = function(parameters) {
           ;
           if(hasSelected && !module.is.multiple()) {
             module.debug('Forcing partial selection to selected item', $selectedItem);
-            $selectedItem[0].click();
+            module.event.item.click.call($selectedItem, {}, true);
             return;
           }
           else {

[zeripath]

Remaining refreshDescendant calls

@@ -1405,6 +1500,7 @@
                         .closest(selector.item)
                           .addClass(className.selected)
                       ;
+                      module.aria.refreshDescendant();
                       event.preventDefault();
                     }
                   }

This adds a refreshDescendant call when you close a submenu using the left arrow key..

@@ -1421,6 +1517,7 @@
                         .find(selector.item).eq(0)
                           .addClass(className.selected)
                       ;
+                      module.aria.refreshDescendant();
                       event.preventDefault();
                     }
                   }

And adds one when you open a submenu using the right arrow key.

@@ -1445,6 +1542,7 @@
                     $nextItem
                       .addClass(className.selected)
                     ;
+                    module.aria.refreshDescendant();
                     module.set.scrollPosition($nextItem);
                     if(settings.selectOnKeydown && module.is.single()) {
                       module.set.selectedItem($nextItem);

When you move using the up arrow key.

@@ -1472,6 +1570,7 @@
                     $nextItem
                       .addClass(className.selected)
                     ;
+                    module.aria.refreshDescendant();
                     module.set.scrollPosition($nextItem);
                     if(settings.selectOnKeydown && module.is.single()) {
                       module.set.selectedItem($nextItem);

And down with the down arrow key.

@@ -2399,6 +2498,7 @@
               module.set.scrollPosition($nextValue);
               $selectedItem.removeClass(className.selected);
               $nextValue.addClass(className.selected);
+              module.aria.refreshDescendant();
               if(settings.selectOnKeydown && module.is.single()) {
                 module.set.selectedItem($nextValue);
               }

Finally this one is when we scroll by letter.

---

I think we could get away with using onChange and/or onLabelSelect to fire these.

[zeripath]

Thanks @Jookia

I can imagine how burning out it felt - there's a reason why I never got round to fixing this myself. The documentation is highly confusing and its not clear how to map things on to Gitea's UI (or most UI TBH.)

---

So the best way to look at this patch is to look through my mindset of what I could do to immediately make things a little better without putting load on maintainers to actually fix things, since it didn't seem like maintainers wanted to spend time refactoring HTML or learning ARIA in order to fix things properly. I was also learning ARIA at the time specifically to make this patch, and getting pretty burned out in the process. In the end I was looking at the specific solution of getting some menus working so my blind friend could use Gitea better.

I wouldn't assume that anyone doesn't want to learn ARIA - we just have other things to do and if someone appears to know things better most of us are happy to defer.

Your experience of getting burnt out by learning ARIA is precisely my feeling of when I tried looking into it - especially as I could not get orca to work at all.

Honestly - as I said I'd be very happy to copy patterns that work but finding any consistency in the documentation I last found when I looked in to this stuff was impossible.

For what it's worth, I had a lot of explanations in git commit messages but these got squashed and removed I think. I was grumpy about this for this exact reason.

Ah this is the issue with squash merging. Honestly if you ever feel like giving us another PR - and I hope you can - code comments and comments in issues are a better way of describing things.

Gitea uses dropdowns everywhere. Not just for menus, but also for search boxes, tag selection, branch selection, everywhere. These widgets range from 'listbox' to 'combobox' to other fancy widgets that do multiple things such as manage entries in a list and letting you add new entries by searching in the entry list and adding new tags. Sometimes there's even listboxes that have buttons to open the listbox with an arrow, and those are marked as icons. All of these use Fomantic's 'dropdown' UI.

Yup - it's a very versatile element. Especially with the searching.

So the first thing to really do is try and find some actual listboxes (which I incorrectly refer to as menu):
...
The criteria here is that:

• It's not also an icon CSS-wise
• It's not a search box
• It doesn't have any way to input text
• There are multiple elements in the dropdown

That seems like it's MAYBE a dropdown. The correct solution here would be to go through every dropdown in Gitea's source code and categorize what it is and how to actually handle it, because for each dropdown Fomantic also adds some kind of Javascript, even on the little dropdown icons people are supposed to click.

Agreed - this is the correct solution.

Whilst not a solution that can be done for 1.15 - this can and should be done for 1.16 and would definitely receive positive reviews.

The next step is to decide what ARIA widget this is and start implementing its role, as a contract between us and any assistive technology. I chose 'menubar' because I wasn't sure what to pick, but in retrospect it should've been 'listbox'. In any case, I did this wrong since I was trying to fit a square in to a round hole.

This explains my slight confusion with the aria-role="menu" choice. I also think that in a lot of cases that we have a listbox we may be able to just switch to use <select> which as I understand it even with JS is handled better for screenreaders(?).

The next thing to think about is focus management. This is what most dropdowns on the Internet get wrong. When you use a keyboard to access ARIA widgets, you don't want to use tab, you want to just arrows and specific keys mandated by ARIA. This means for example that you can tab past listboxes without tabbing through each element in the listbox. If a person wants to use the dropdown, they'll open it and use the arrows.

My impression of fomantic-ui was that this was the one thing that it does right - but it doesn't set the active-descendent - which could be solved using a selection hook.

I think widgets should manage their own focus since it provides a less confusing experiences- you can't accidentally focus subelements using tab when you're not supposed to and get in to an invalid and confusing state.

With that in mind, we set the tabindex to -1 so NOTHING inside the dropdown is focusable:

> +            $item.attr('tabindex', '-1');

OK, I guess this really only applies if we have <a> in the dropdown menu items - as <div> are not tabable by default. This should really be at the template level

We also should be setting the icon next to the menu to tabindex -1 so we don't have to tab past it:

I guess also a template level thing - but should probably not apply to searching dropdowns.

But that might not have been upstreamed or something.

Yeah I'm not certain what happened there - may have been conflicted away.

We set aria-expanded when that changes:

...

Then whenever anything changes, we update the aria-activedescent attribute to point to the selected item:

...

This does not work for listboxes that let you select multiple things, only one or no elements.

This would require aria-multipleselected, aria-selected and focusing for the elements in the drop down IIRC?

Calls to these hooks are peppers basically whenever input happens or the dropdown is shown. This is because I didn't wanted to dig in the dropdown spaghetti and wanted to instead make sure it worked each time.

Now, when it's time to select an element, the dropdown code calls module.event.item.click.call. This doesn't actually call onclick() handlers all the time. Instead you have to directly call click(). I noted this in my now squashed commit message:

but I don't understand why you want the onclick call exactly?

I had to revert this in one case later:

I think this is the one that I've noticed here .and had to add a fix in against.

[Jookia]

I didn't know about multipleselected/aria-selected, etc.

The onClick call is because some Gitea code uses onClick elements and don't account for keyboard focus.

[zeripath]

I didn't know about multipleselected/aria-selected, etc.

The onClick call is because some Gitea code uses onClick elements and don't account for keyboard focus.

OK I think we should change those to use the select callback.

[Jookia]

On Sat, Jul 31, 2021 at 02:15:13AM -0700, zeripath wrote: OK I think we should change those to use the select callback.

Well, yeah. But who's going to spelunk the code to find them all?