Fix a11y for repo topic management

options/locale/locale_en-US.ini

@@ -2358,9 +2358,9 @@ tag.create_tag_from = Create new tag from '%s'
 tag.create_success = Tag '%s' has been created.
 
 topic.manage_topics = Manage Topics
-topic.done = Done
 topic.count_prompt = You cannot select more than 25 topics
 topic.format_prompt = Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
+topic.remove_topic = Remove topic "%s"
 
 find_file.go_to_file = Go to file
 find_file.no_matching = No matching file found

templates/base/footer_content.tmpl

@@ -21,7 +21,7 @@
 			{{end}}
 			<div class="ui language bottom floating slide up dropdown link item">
 				{{svg "octicon-globe"}}
-				<div class="text">{{.locale.LangName}}</div>
+				<span>{{.locale.LangName}}</span>
 				<div class="menu language-menu">
 					{{range .AllLangs}}
 						<a lang="{{.Lang}}" data-url="{{AppSubUrl}}/?lang={{.Lang}}" class="item {{if eq $.locale.Lang .Lang}}active selected{{end}}">{{.Name}}</a>

templates/repo/home.tmpl

@@ -28,34 +28,33 @@
 				</div>
 			{{end}}
 		</div>
-		<div class="gt-mt-3" id="repo-topics">
-		{{range .Topics}}<a class="ui repo-topic large label topic" href="{{AppSubUrl}}/explore/repos?q={{.Name}}&topic=1">{{.Name}}</a>{{end}}
-		{{if and .Permission.IsAdmin (not .Repository.IsArchived)}}<a id="manage_topic" class="muted">{{.locale.Tr "repo.topic.manage_topics"}}</a>{{end}}
+		<div class="gt-df gt-ac gt-fw gt-mt-3" id="repo-topics">
+			{{range .Topics}}<a class="ui repo-topic large label topic" href="{{AppSubUrl}}/explore/repos?q={{.Name}}&topic=1">{{.Name}}</a>{{end}}
+			{{if and .Permission.IsAdmin (not .Repository.IsArchived)}}<button id="manage_topic" class="ui button tiny tertiary gt-ml-2">{{.locale.Tr "repo.topic.manage_topics"}}</button>{{end}}
 		</div>
 		{{end}}
 		{{if and .Permission.IsAdmin (not .Repository.IsArchived)}}
-		<div class="ui repo-topic-edit grid form gt-hidden" id="topic_edit">
-			<div class="fourteen wide column">
-				<div class="field">
-					<div class="ui fluid multiple search selection dropdown">
-						<input type="hidden" name="topics" value="{{range $i, $v := .Topics}}{{.Name}}{{if lt (Add $i 1) (len $.Topics)}},{{end}}{{end}}">
-						{{range .Topics}}
-						<div class="ui small label topic transition visible" data-value="{{.Name}}" style="display: inline-block !important; cursor: default;">{{.Name}}{{svg "octicon-x" 16 "delete icon gt-ml-3 gt-mt-1"}}</div>
-						{{end}}
-						<div class="text"></div>
-					</div>
+		<div class="ui form gt-hidden gt-df gt-mt-4" id="topic_edit">
+			<div class="field gt-f1 gt-mr-3">
+				<div class="ui fluid multiple search selection dropdown"
+						data-text-count-prompt="{{.locale.Tr "repo.topic.count_prompt"}}"
+						data-text-format-prompt="{{.locale.Tr "repo.topic.format_prompt"}}"
+						data-text-remove-topic="{{.locale.Tr "repo.topic.remove_topic"}}"
+				>
+					<input type="hidden" name="topics" value="{{range $i, $v := .Topics}}{{.Name}}{{if lt (Add $i 1) (len $.Topics)}},{{end}}{{end}}">
+					{{range .Topics}}
+						{{/* keep the same as template (repo-topic-label), the style "display" is also added by Fomantic Dropdown automatically when generating new labels */}}
+						<div class="ui small label topic transition visible gt-cursor-default" data-value="{{.Name}}" style="display: inline-block !important;">{{.Name}}{{svg "octicon-x" 16 "delete icon gt-ml-3 gt-mt-1"}}</div>
+					{{end}}
+					<div class="text"></div>
 				</div>
 			</div>
-			<div class="two wide column">
-				<a class="ui button primary" role="button" tabindex="0" id="save_topic"
-				data-link="{{.RepoLink}}/topics">{{.locale.Tr "repo.topic.done"}}</a>
+			<div>
+				<button class="ui basic button secondary" id="cancel_topic_edit">{{.locale.Tr "cancel"}}</button>
+				<button class="ui primary button" id="save_topic" data-link="{{.RepoLink}}/topics">{{.locale.Tr "save"}}</button>
 			</div>
 		</div>
 		{{end}}
-		<div class="gt-hidden" id="validate_prompt">
-			<span id="count_prompt">{{.locale.Tr "repo.topic.count_prompt"}}</span>
-			<span id="format_prompt">{{.locale.Tr "repo.topic.format_prompt"}}</span>
-		</div>
 		{{if .Repository.IsArchived}}
 			<div class="ui warning message">
 				{{.locale.Tr "repo.archive.title"}}

templates/repo/issue/view_content/add_reaction.tmpl

@@ -1,5 +1,5 @@
 {{if .ctxData.IsSigned}}
-<div class="item action ui pointing select-reaction dropdown top right" data-action-url="{{.ActionURL}}">
+<div class="item action ui dropdown jump pointing top right select-reaction" data-action-url="{{.ActionURL}}">
 	<a class="add-reaction">
 		{{svg "octicon-smiley"}}
 	</a>

templates/repo/issue/view_content/context_menu.tmpl

@@ -1,5 +1,5 @@
 {{if .ctxData.IsSigned}}
-<div class="item action ui pointing custom dropdown top right context-dropdown">
+<div class="item action ui dropdown jump pointing top right context-dropdown">
 	<a class="context-menu">
 		{{svg "octicon-kebab-horizontal"}}
 	</a>

web_src/js/features/aria.js

@@ -6,42 +6,16 @@ function generateAriaId() {
   return `_aria_auto_id_${ariaIdCounter++}`;
 }
 
-// make the item has role=option, and add an id if there wasn't one yet.
-function prepareMenuItem($item) {
-  if (!$item.attr('id')) $item.attr('id', generateAriaId());
-  $item.attr({'role': 'menuitem', 'tabindex': '-1'});
-  $item.find('a').attr('tabindex', '-1'); // as above, the elements inside the dropdown menu item should not be focusable, the focus should always be on the dropdown primary element.
-}
-
-// when the menu items are loaded from AJAX requests, the items are created dynamically
-const defaultCreateDynamicMenu = $.fn.dropdown.settings.templates.menu;
-$.fn.dropdown.settings.templates.menu = function(response, fields, preserveHTML, className) {
-  const ret = defaultCreateDynamicMenu(response, fields, preserveHTML, className);
-  const $wrapper = $('<div>').append(ret);
-  const $items = $wrapper.find('> .item');
-  $items.each((_, item) => {
-    prepareMenuItem($(item));
-  });
-  return $wrapper.html();
-};
-
 function attachOneDropdownAria($dropdown) {
   if ($dropdown.attr('data-aria-attached')) return;
   $dropdown.attr('data-aria-attached', 1);
 
-  const $textSearch = $dropdown.find('input.search').eq(0);
-  const $focusable = $textSearch.length ? $textSearch : $dropdown; // see comment below
-  if (!$focusable.length) return;
-
-  // prepare menu list
-  const $menu = $dropdown.find('> .menu');
-  if (!$menu.attr('id')) $menu.attr('id', generateAriaId());
-
-  // dropdown has 2 different focusing behaviors
-  // * with search input: the input is focused, and it works perfectly with aria-activedescendant pointing another sibling element.
+  // Dropdown has 2 different focusing behaviors
+  // * with search input: the input is focused, and it works with aria-activedescendant pointing another sibling element.
   // * without search input (but the readonly text), the dropdown itself is focused. then the aria-activedescendant points to the element inside dropdown
+  // Some desktop screen readers may change the focus, but dropdown requires that the focus must be on its primary element, then they don't work well.
 
-  // expected user interactions for dropdown with aria support:
+  // Expected user interactions for dropdown with aria support:
   // * user can use Tab to focus in the dropdown, then the dropdown menu (list) will be shown
   // * user presses Tab on the focused dropdown to move focus to next sibling focusable element (but not the menu item)
   // * user can use arrow key Up/Down to navigate between menu items
@@ -51,25 +25,63 @@ function attachOneDropdownAria($dropdown) {
 
   // TODO: multiple selection is not supported yet.
 
-  $focusable.attr({
-    'role': 'menu',
-    'aria-haspopup': 'menu',
-    'aria-controls': $menu.attr('id'),
-    'aria-expanded': 'false',
-  });
+  const $textSearch = $dropdown.find('input.search').eq(0);
+  const $focusable = $textSearch.length ? $textSearch : $dropdown; // the primary element for focus, see comment above
+  if (!$focusable.length) return;
 
-  if ($dropdown.attr('data-content') && !$dropdown.attr('aria-label')) {
+  // detect if the dropdown has an input, if yes, it works like a combobox, otherwise it works like a menu
+  // or use a special class to indicate it's a combobox/menu in the future
+  const isComboBox = $dropdown.find('input').length > 0;
+
+  const focusableRole = isComboBox ? 'combobox' : 'button';
+  const listPopupRole = isComboBox ? 'listbox' : 'menu';
+  const listItemRole = isComboBox ? 'option' : 'menuitem';
+
+  // make the item has role=option/menuitem, and add an id if there wasn't one yet.
+  function prepareMenuItem($item) {
+    if (!$item.attr('id')) $item.attr('id', generateAriaId());
+    $item.attr({'role': listItemRole, 'tabindex': '-1'});
+    $item.find('a').attr('tabindex', '-1'); // as above, the elements inside the dropdown menu item should not be focusable, the focus should always be on the dropdown primary element.
+  }
+
+  const dropdownTemplates = {...$dropdown.dropdown('setting', 'templates')};
+  const dropdownTemplatesMenuOld = dropdownTemplates.menu;
+  dropdownTemplates.menu = function(response, fields, preserveHTML, className) {
+    // when the dropdown menu items are loaded from AJAX requests, the items are created dynamically
+    const menuItems = dropdownTemplatesMenuOld(response, fields, preserveHTML, className);
+    const $wrapper = $('<div>').append(menuItems);
+    const $items = $wrapper.find('> .item');
+    $items.each((_, item) => prepareMenuItem($(item)));
+    return $wrapper.html();
+  };
+  $dropdown.dropdown('setting', 'templates', dropdownTemplates);
+
+  // use tooltip's content as aria-label if there is no aria-label
+  if ($dropdown.hasClass('tooltip') && $dropdown.attr('data-content') && !$dropdown.attr('aria-label')) {
     $dropdown.attr('aria-label', $dropdown.attr('data-content'));
   }
 
+  // prepare dropdown menu list popup
+  const $menu = $dropdown.find('> .menu');
+  if (!$menu.attr('id')) $menu.attr('id', generateAriaId());
   $menu.find('> .item').each((_, item) => {
     prepareMenuItem($(item));
   });
+  // this role could only be changed after its content is ready, otherwise some browsers+readers (like Chrome+AppleVoice) crash
+  $menu.attr('role', listPopupRole);
+
+  $focusable.attr({
+    'role': $focusable.attr('role') ?? focusableRole,
+    'aria-haspopup': listPopupRole,
+    'aria-controls': $menu.attr('id'),
+    'aria-expanded': 'false',
+  });
+
+  const isMenuVisible = () => $menu.hasClass('visible') && !$menu.hasClass('out'); // when hiding, it has class: ".animating.out"
 
   // update aria attributes according to current active/selected item
   const refreshAria = () => {
-    const isMenuVisible = !$menu.is('.hidden') && !$menu.is('.animating.out');
-    $focusable.attr('aria-expanded', isMenuVisible ? 'true' : 'false');
+    $focusable.attr('aria-expanded', isMenuVisible() ? 'true' : 'false');
 
     let $active = $menu.find('> .item.active');
     if (!$active.length) $active = $menu.find('> .item.selected'); // it's strange that we need this fallback at the moment
@@ -85,16 +97,59 @@ function attachOneDropdownAria($dropdown) {
       if (!$item) $item = $menu.find('> .item.selected'); // when dropdown filters items by input, there is no "value", so query the "selected" item
       // if the selected item is clickable, then trigger the click event.
       // we can not click any item without check, because Fomantic code might also handle the Enter event. that would result in double click.
-      if ($item && ($item.is('a') || $item.is('.js-aria-clickable'))) $item[0].click();
+      if ($item && ($item.is('a') || $item.hasClass('js-aria-clickable'))) $item[0].click();
     }
   });
 
   // use setTimeout to run the refreshAria in next tick (to make sure the Fomantic UI code has finished its work)
-  const deferredRefreshAria = () => { setTimeout(refreshAria, 0) }; // do not return any value, jQuery has return-value related behaviors.
-  $focusable.on('focus', deferredRefreshAria);
-  $focusable.on('mouseup', deferredRefreshAria);
-  $focusable.on('blur', deferredRefreshAria);
+  // do not return any value, jQuery has return-value related behaviors.
+  const deferredRefreshAria = () => { setTimeout(refreshAria, 0) };
   $dropdown.on('keyup', (e) => { if (e.key.startsWith('Arrow')) deferredRefreshAria(); });
+
+  // if the dropdown has been opened by focus, do not trigger the next click event again.
+  // otherwise the dropdown will be closed immediately, especially on Android with TalkBack
+  // * desktop event sequence: mousedown -> focus -> mouseup -> click
+  // * mobile event sequence: focus -> mousedown -> mouseup -> click
+  // Fomantic may stop propagation of blur event, use capture to make sure we can still get the event
+  // keep the debug code for developers who want to confirm&debug this code for different browsers (without attaching a remote debugger)
+  const showDebug = false;
+  const debug = (msg) => showDebug && $('.page-content').append($('<div>').text(`${$menu.attr('id')} ${msg}, menu visible=${isMenuVisible()}`));
+  let ignoreClickPreEvents = 0, ignoreClickPreVisible = 0;
+  $dropdown[0].addEventListener('mousedown', (e) => {
+    debug(e.type);
+    ignoreClickPreVisible += isMenuVisible() ? 1 : 0;
+    ignoreClickPreEvents++;
+  }, true);
+  $dropdown[0].addEventListener('focus', (e) => {
+    debug(e.type);
+    ignoreClickPreVisible += isMenuVisible() ? 1 : 0;
+    ignoreClickPreEvents++;
+    deferredRefreshAria();
+  }, true);
+  $dropdown[0].addEventListener('blur', (e) => {
+    debug(e.type);
+    ignoreClickPreVisible = ignoreClickPreEvents = 0;
+    deferredRefreshAria();
+  }, true);
+  $dropdown[0].addEventListener('mouseup', (e) => {
+    debug(e.type);
+    setTimeout(() => {
+      debug(`${e.type} (deferred)`);
+      ignoreClickPreVisible = ignoreClickPreEvents = 0;
+      deferredRefreshAria();
+    }, 0);
+  }, true);
+  $dropdown[0].addEventListener('click', (e) => {
+    debug(`${e.type}, pre-visible=${ignoreClickPreVisible}, pre-events=${ignoreClickPreEvents}`);
+    if (isMenuVisible() &&
+      ignoreClickPreVisible !== 2 && // dropdown is switch from invisible to visible
+      ignoreClickPreEvents === 2 // the click event is related to mousedown+focus
+    ) {
+      debug(`${e.type}, stop click propagation`);
+      e.stopPropagation(); // if the dropdown menu has been opened by focus, do not trigger the next click event again
+    }
+    ignoreClickPreEvents = ignoreClickPreVisible = 0;
+  }, true);
 }
 
 export function attachDropdownAria($dropdowns) {

web_src/js/features/aria.md

@@ -30,11 +30,10 @@ There is still a problem: Fomantic UI checkbox is not friendly to screen readers
 ## ARIA Dropdown
 
 There are different solutions:
+
 * combobox + listbox + option
 * menu + menuitem
 
-At the moment, `menu + menuitem` seems to work better with Fomantic UI Dropdown, so we only use it now.
-
 ```html
 <div>
   <input role="combobox" aria-haspopup="listbox" aria-expanded="false" aria-controls="the-menu-listbox" aria-activedescendant="item-id-123456">
@@ -49,6 +48,19 @@ At the moment, `menu + menuitem` seems to work better with Fomantic UI Dropdown,
 
 ## Fomantic UI Dropdown
 
+When the dropdown has input elements, use `role=combobox`, otherwise use `role=menu` (see `aria.js`) for actions.
+
+Reference:
+
+* Combobox:
+  * https://www.w3.org/WAI/ARIA/apg/patterns/combobox/
+  * A combobox is an **input widget** with an associated popup that enables users to select a value for the combobox from
+    a collection of possible values. In some implementations, the popup presents allowed values, while in other implementations,
+    the popup presents suggested values, and users may either select one of the suggestions or type a value.
+* Menu:
+  * https://www.w3.org/WAI/ARIA/apg/patterns/menubar/
+  * A menu is a widget that offers a list of choices to the user, such as a set of **actions or functions**.
+
 ```html
 <!-- read-only dropdown -->
 <div class="ui dropdown"> <!-- focused here, then it's not perfect to use aria-activedescendant to point to the menu item -->

web_src/js/features/common-global.js

@@ -89,9 +89,12 @@ export function initGlobalCommon() {
 
   // Semantic UI modules.
   const $uiDropdowns = $('.ui.dropdown');
-  $uiDropdowns.filter(':not(.custom)').dropdown({
-    fullTextSearch: 'exact'
-  });
+
+  // do not init "custom" dropdowns, "custom" dropdowns are managed by their own code.
+  $uiDropdowns.filter(':not(.custom)').dropdown({fullTextSearch: 'exact'});
+
+  // "jump" means this dropdown is mainly used for "menu" purpose, clicking an item will jump to somewhere else or trigger an action/function.
+  // when a dropdown is used for non-refresh actions with tippy, it must have this "jump" class to hide the tippy when dropdown is closed.
   $uiDropdowns.filter('.jump').dropdown({
     action: 'hide',
     onShow() {
@@ -101,17 +104,20 @@ export function initGlobalCommon() {
     },
     onHide() {
       this._tippy?.enable();
+
+      // hide all tippy elements of items after a while. eg: keyboard Enter to trigger the "Copy Link" in the Issue Context Menu
+      setTimeout(() => {
+        $(this).find('.menu > .item').each((_, item) => {
+          item._tippy?.hide();
+        });
+      }, 2000);
     },
-    fullTextSearch: 'exact'
-  });
-  $uiDropdowns.filter('.slide.up').dropdown({
-    transition: 'slide up',
-    fullTextSearch: 'exact'
-  });
-  $uiDropdowns.filter('.upward').dropdown({
-    direction: 'upward',
-    fullTextSearch: 'exact'
   });
+
+  // special animations/popup-directions
+  $uiDropdowns.filter('.slide.up').dropdown({transition: 'slide up'});
+  $uiDropdowns.filter('.upward').dropdown({direction: 'upward'});
+
   attachDropdownAria($uiDropdowns);
 
   attachCheckboxAria($('.ui.checkbox'));