From 9d862008b3bc6b97ba42ae0c646a7b2865cd72f4 Mon Sep 17 00:00:00 2001
From: wxiaoguang <wxiaoguang@gmail.com>
Date: Wed, 24 Jul 2024 14:19:48 +0800
Subject: [PATCH 1/5] fix

---
 web_src/js/components/DiffCommitSelector.vue | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/web_src/js/components/DiffCommitSelector.vue b/web_src/js/components/DiffCommitSelector.vue
index a5239527738cf..8ff7649c4d87f 100644
--- a/web_src/js/components/DiffCommitSelector.vue
+++ b/web_src/js/components/DiffCommitSelector.vue
@@ -202,7 +202,8 @@ export default {
     >
       <svg-icon name="octicon-git-commit"/>
     </button>
-    <div class="left menu" id="diff-commit-selector-menu" :class="{visible: menuVisible}" v-show="menuVisible" v-cloak :aria-expanded="menuVisible ? 'true': 'false'">
+    <!-- this dropdown is not managed by Fomantic UI, so it needs some classes like "transition" explicitly -->
+    <div class="left menu transition" id="diff-commit-selector-menu" :class="{visible: menuVisible}" v-show="menuVisible" v-cloak :aria-expanded="menuVisible ? 'true': 'false'">
       <div class="loading-indicator is-loading" v-if="isLoading"/>
       <div v-if="!isLoading" class="vertical item" id="diff-commit-list-show-all" role="menuitem" @keydown.enter="showAllChanges()" @click="showAllChanges()">
         <div class="gt-ellipsis">

From 5cd082ea93b7fed32cf4011a356f5310e77b68af Mon Sep 17 00:00:00 2001
From: wxiaoguang <wxiaoguang@gmail.com>
Date: Wed, 24 Jul 2024 17:59:44 +0800
Subject: [PATCH 2/5] fix

---
 web_src/js/components/DiffCommitSelector.vue | 62 +++++++++-----------
 1 file changed, 27 insertions(+), 35 deletions(-)

diff --git a/web_src/js/components/DiffCommitSelector.vue b/web_src/js/components/DiffCommitSelector.vue
index 8ff7649c4d87f..a94979b56e54d 100644
--- a/web_src/js/components/DiffCommitSelector.vue
+++ b/web_src/js/components/DiffCommitSelector.vue
@@ -1,6 +1,7 @@
 <script lang="ts">
 import {SvgIcon} from '../svg.ts';
 import {GET} from '../modules/fetch.ts';
+import {generateAriaId} from '../modules/fomantic/base.ts';
 
 export default {
   components: {SvgIcon},
@@ -15,6 +16,8 @@ export default {
       commits: [],
       hoverActivated: false,
       lastReviewCommitSha: null,
+      uniqueIdMenu: generateAriaId(),
+      uniqueIdShowAll: generateAriaId(),
     };
   },
   computed: {
@@ -113,12 +116,10 @@ export default {
       }
       // set correct tabindex to allow easier navigation
       this.$nextTick(() => {
-        const expandBtn = this.$el.querySelector('#diff-commit-list-expand');
-        const showAllChanges = this.$el.querySelector('#diff-commit-list-show-all');
         if (this.menuVisible) {
-          this.focusElem(showAllChanges, expandBtn);
+          this.focusElem(this.$refs.showAllChanges, this.$refs.expandBtn);
         } else {
-          this.focusElem(expandBtn, showAllChanges);
+          this.focusElem(this.$refs.expandBtn, this.$refs.showAllChanges);
         }
       });
     },
@@ -189,23 +190,23 @@ export default {
 };
 </script>
 <template>
-  <div class="ui scrolling dropdown custom">
+  <div class="ui scrolling dropdown custom diff-commit-selector">
     <button
+      ref="expandBtn"
       class="ui basic button"
-      id="diff-commit-list-expand"
       @click.stop="toggleMenu()"
       :data-tooltip-content="locale.filter_changes_by_commit"
       aria-haspopup="true"
-      aria-controls="diff-commit-selector-menu"
       :aria-label="locale.filter_changes_by_commit"
-      aria-activedescendant="diff-commit-list-show-all"
+      :aria-controls="uniqueIdMenu"
+      :aria-activedescendant="uniqueIdShowAll"
     >
       <svg-icon name="octicon-git-commit"/>
     </button>
     <!-- this dropdown is not managed by Fomantic UI, so it needs some classes like "transition" explicitly -->
-    <div class="left menu transition" id="diff-commit-selector-menu" :class="{visible: menuVisible}" v-show="menuVisible" v-cloak :aria-expanded="menuVisible ? 'true': 'false'">
+    <div class="left menu transition" :id="uniqueIdMenu" :class="{visible: menuVisible}" v-show="menuVisible" v-cloak :aria-expanded="menuVisible ? 'true': 'false'">
       <div class="loading-indicator is-loading" v-if="isLoading"/>
-      <div v-if="!isLoading" class="vertical item" id="diff-commit-list-show-all" role="menuitem" @keydown.enter="showAllChanges()" @click="showAllChanges()">
+      <div v-if="!isLoading" class="item" :id="uniqueIdShowAll" ref="showAllChanges" role="menuitem" @keydown.enter="showAllChanges()" @click="showAllChanges()">
         <div class="gt-ellipsis">
           {{ locale.show_all_commits }}
         </div>
@@ -216,7 +217,7 @@ export default {
       <!-- only show the show changes since last review if there is a review AND we are commits ahead of the last review -->
       <div
         v-if="lastReviewCommitSha != null" role="menuitem"
-        class="vertical item"
+        class="item"
         :class="{disabled: !commitsSinceLastReview}"
         @keydown.enter="changesSinceLastReviewClick()"
         @click="changesSinceLastReviewClick()"
@@ -231,8 +232,8 @@ export default {
       <span v-if="!isLoading" class="info text light-2">{{ locale.select_commit_hold_shift_for_range }}</span>
       <template v-for="commit in commits" :key="commit.id">
         <div
-          class="vertical item" role="menuitem"
-          :class="{selection: commit.selected, hovered: commit.hovered}"
+          class="item" role="menuitem"
+          :class="{active: commit.selected}"
           @keydown.enter.exact="commitClicked(commit.id)"
           @keydown.enter.shift.exact="commitClickedShift(commit)"
           @mouseover.shift="highlight(commit)"
@@ -262,46 +263,37 @@ export default {
   </div>
 </template>
 <style scoped>
-  .hovered:not(.selection) {
-    background-color: var(--color-small-accent) !important;
-  }
-  .selection {
-    background-color: var(--color-accent) !important;
-  }
-
-  .info {
-    display: inline-block;
-    padding: 7px 14px !important;
-    line-height: 1.4;
-    width: 100%;
-  }
-
-  #diff-commit-selector-menu {
+  .ui.dropdown.diff-commit-selector .menu {
+    margin-top: 0.25em;
     overflow-x: hidden;
     max-height: 450px;
   }
 
-  #diff-commit-selector-menu .loading-indicator {
+  .ui.dropdown.diff-commit-selector .menu .loading-indicator {
     height: 200px;
     width: 350px;
   }
 
-  #diff-commit-selector-menu .item,
-  #diff-commit-selector-menu .info {
-    display: flex !important;
+  .ui.dropdown.diff-commit-selector .menu > .item,
+  .ui.dropdown.diff-commit-selector .menu > .info {
+    display: flex;
     flex-direction: row;
     line-height: 1.4;
+    gap: 0.25em;
     padding: 7px 14px !important;
+  }
+
+  .ui.dropdown.diff-commit-selector .menu > .item:not(:first-child),
+  .ui.dropdown.diff-commit-selector .menu > .info:not(:first-child) {
     border-top: 1px solid var(--color-secondary) !important;
-    gap: 0.25em;
   }
 
-  #diff-commit-selector-menu .item:focus {
+  .ui.dropdown.diff-commit-selector .menu > .item:focus {
     color: var(--color-text);
     background: var(--color-hover);
   }
 
-  #diff-commit-selector-menu .commit-list-summary {
+  .ui.dropdown.diff-commit-selector .menu .commit-list-summary {
     max-width: min(380px, 96vw);
   }
 </style>

From e78287836a6d5ed8d2128d97357b4e57a014cfc9 Mon Sep 17 00:00:00 2001
From: wxiaoguang <wxiaoguang@gmail.com>
Date: Wed, 24 Jul 2024 18:35:42 +0800
Subject: [PATCH 3/5] fix el access, fix hover

---
 web_src/js/components/DiffCommitSelector.vue | 22 ++++++++++++--------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/web_src/js/components/DiffCommitSelector.vue b/web_src/js/components/DiffCommitSelector.vue
index a94979b56e54d..9ce5fc3349fa1 100644
--- a/web_src/js/components/DiffCommitSelector.vue
+++ b/web_src/js/components/DiffCommitSelector.vue
@@ -10,6 +10,8 @@ export default {
     return {
       menuVisible: false,
       isLoading: false,
+      queryParams: el.getAttribute('data-queryparams'),
+      issueLink: el.getAttribute('data-issuelink'),
       locale: {
         filter_changes_by_commit: el.getAttribute('data-filter_changes_by_commit'),
       },
@@ -27,12 +29,6 @@ export default {
       }
       return 0;
     },
-    queryParams() {
-      return this.$el.parentNode.getAttribute('data-queryparams');
-    },
-    issueLink() {
-      return this.$el.parentNode.getAttribute('data-issuelink');
-    },
   },
   mounted() {
     document.body.addEventListener('click', this.onBodyClick);
@@ -216,8 +212,8 @@ export default {
       </div>
       <!-- only show the show changes since last review if there is a review AND we are commits ahead of the last review -->
       <div
-        v-if="lastReviewCommitSha != null" role="menuitem"
-        class="item"
+        v-if="lastReviewCommitSha != null"
+        class="item" role="menuitem"
         :class="{disabled: !commitsSinceLastReview}"
         @keydown.enter="changesSinceLastReviewClick()"
         @click="changesSinceLastReviewClick()"
@@ -233,7 +229,7 @@ export default {
       <template v-for="commit in commits" :key="commit.id">
         <div
           class="item" role="menuitem"
-          :class="{active: commit.selected}"
+          :class="{selected: commit.selected, hovered: commit.hovered}"
           @keydown.enter.exact="commitClicked(commit.id)"
           @keydown.enter.shift.exact="commitClickedShift(commit)"
           @mouseover.shift="highlight(commit)"
@@ -288,6 +284,14 @@ export default {
     border-top: 1px solid var(--color-secondary) !important;
   }
 
+  .ui.dropdown.diff-commit-selector .menu > .item.hovered {
+    background-color: var(--color-small-accent);
+  }
+
+  .ui.dropdown.diff-commit-selector .menu > .item.selected {
+    background-color: var(--color-accent);
+  }
+
   .ui.dropdown.diff-commit-selector .menu > .item:focus {
     color: var(--color-text);
     background: var(--color-hover);

From 58bb761ea6e64fd02f0285eead2c3af559b2c9b6 Mon Sep 17 00:00:00 2001
From: wxiaoguang <wxiaoguang@gmail.com>
Date: Wed, 24 Jul 2024 19:21:48 +0800
Subject: [PATCH 4/5] fix keyboard selection

---
 web_src/js/components/DiffCommitSelector.vue | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/web_src/js/components/DiffCommitSelector.vue b/web_src/js/components/DiffCommitSelector.vue
index 9ce5fc3349fa1..84a292fe515c2 100644
--- a/web_src/js/components/DiffCommitSelector.vue
+++ b/web_src/js/components/DiffCommitSelector.vue
@@ -67,6 +67,11 @@ export default {
           this.toggleMenu();
           break;
       }
+      if (event.key === 'ArrowDown' || event.key === 'ArrowUp') {
+        const item = document.activeElement; // try to highlight the selected commits
+        const commitIdx = item?.matches('.item') ? item.getAttribute('data-commit-idx') : null;
+        if (commitIdx) this.highlight(this.commits[commitIdx]);
+      }
     },
     onKeyUp(event) {
       if (!this.menuVisible) return;
@@ -226,10 +231,11 @@ export default {
         </div>
       </div>
       <span v-if="!isLoading" class="info text light-2">{{ locale.select_commit_hold_shift_for_range }}</span>
-      <template v-for="commit in commits" :key="commit.id">
+      <template v-for="(commit, idx) in commits" :key="commit.id">
         <div
           class="item" role="menuitem"
           :class="{selected: commit.selected, hovered: commit.hovered}"
+          :data-commit-idx="idx"
           @keydown.enter.exact="commitClicked(commit.id)"
           @keydown.enter.shift.exact="commitClickedShift(commit)"
           @mouseover.shift="highlight(commit)"
@@ -292,11 +298,6 @@ export default {
     background-color: var(--color-accent);
   }
 
-  .ui.dropdown.diff-commit-selector .menu > .item:focus {
-    color: var(--color-text);
-    background: var(--color-hover);
-  }
-
   .ui.dropdown.diff-commit-selector .menu .commit-list-summary {
     max-width: min(380px, 96vw);
   }

From 60fd769b2e65d9e88beef3ab5fb80a4224dc0b4b Mon Sep 17 00:00:00 2001
From: wxiaoguang <wxiaoguang@gmail.com>
Date: Wed, 24 Jul 2024 19:35:34 +0800
Subject: [PATCH 5/5] fix keyboard focus background

---
 web_src/js/components/DiffCommitSelector.vue | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/web_src/js/components/DiffCommitSelector.vue b/web_src/js/components/DiffCommitSelector.vue
index 84a292fe515c2..b95e18796c68e 100644
--- a/web_src/js/components/DiffCommitSelector.vue
+++ b/web_src/js/components/DiffCommitSelector.vue
@@ -290,6 +290,10 @@ export default {
     border-top: 1px solid var(--color-secondary) !important;
   }
 
+  .ui.dropdown.diff-commit-selector .menu > .item:focus {
+    background: var(--color-active);
+  }
+
   .ui.dropdown.diff-commit-selector .menu > .item.hovered {
     background-color: var(--color-small-accent);
   }