Skip to content

Commit a4f4d46

Browse files
committed
Hide the menu container in changing it's visibility
This way, the dimensions of the menu container don't depend on its visibility. This patch fixes few keyboard issues I noticed when reading: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Roles/menu_role#keyboard_interactions
1 parent 6a4a3b0 commit a4f4d46

4 files changed

Lines changed: 47 additions & 18 deletions

File tree

web/menu.css

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,16 @@
1313
* limitations under the License.
1414
*/
1515

16+
button.hasPopupMenu {
17+
&[aria-expanded="true"] + menu {
18+
visibility: visible;
19+
}
20+
21+
&[aria-expanded="false"] + menu {
22+
visibility: hidden;
23+
}
24+
}
25+
1626
.popupMenu {
1727
--menuitem-checkmark-icon: url(images/checkmark.svg);
1828
--menu-mark-icon-size: 0;
@@ -75,6 +85,7 @@
7585
top: 1px;
7686
margin: 0;
7787
padding: 5px;
88+
box-sizing: border-box;
7889

7990
background: var(--menu-bg);
8091
background-blend-mode: var(--menu-background-blend-mode);

web/menu.js

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,6 @@ class Menu {
5656
return;
5757
}
5858
const menu = this.#menu;
59-
menu.classList.toggle("hidden", true);
6059
this.#triggeringButton.ariaExpanded = "false";
6160
this.#openMenuAC.abort();
6261
this.#openMenuAC = null;
@@ -82,7 +81,6 @@ class Menu {
8281
}
8382

8483
const menu = this.#menu;
85-
menu.classList.toggle("hidden", false);
8684
this.#triggeringButton.ariaExpanded = "true";
8785
this.#openMenuAC = new AbortController();
8886
const signal = AbortSignal.any([
@@ -137,6 +135,13 @@ class Menu {
137135
.focus();
138136
stopEvent(e);
139137
break;
138+
default:
139+
const char = e.key.toLocaleLowerCase();
140+
this.#goToNextItem(e.target, true, item =>
141+
item.textContent.trim().toLowerCase().startsWith(char)
142+
);
143+
stopEvent(e);
144+
break;
140145
}
141146
},
142147
{ signal, capture: true }
@@ -148,32 +153,38 @@ class Menu {
148153
});
149154
this.#triggeringButton.addEventListener(
150155
"keydown",
151-
ev => {
152-
if (!this.#openMenuAC) {
153-
return;
154-
}
155-
switch (ev.key) {
156+
e => {
157+
switch (e.key) {
158+
case " ":
159+
case "Enter":
156160
case "ArrowDown":
157161
case "Home":
162+
if (!this.#openMenuAC) {
163+
this.#triggeringButton.click();
164+
}
158165
this.#menuItems
159166
.find(
160167
item => !item.disabled && !item.classList.contains("hidden")
161168
)
162169
.focus();
163-
stopEvent(ev);
170+
stopEvent(e);
164171
break;
165172
case "ArrowUp":
166173
case "End":
174+
if (!this.#openMenuAC) {
175+
this.#triggeringButton.click();
176+
}
167177
this.#menuItems
168178
.findLast(
169179
item => !item.disabled && !item.classList.contains("hidden")
170180
)
171181
.focus();
172-
stopEvent(ev);
182+
stopEvent(e);
173183
break;
174184
case "Escape":
175185
this.#closeMenu();
176-
stopEvent(ev);
186+
stopEvent(e);
187+
break;
177188
}
178189
},
179190
{ signal }
@@ -185,7 +196,7 @@ class Menu {
185196
* @param {HTMLElement} element
186197
* @param {boolean} forward
187198
*/
188-
#goToNextItem(element, forward) {
199+
#goToNextItem(element, forward, check = () => true) {
189200
const index =
190201
this.#lastIndex === -1
191202
? this.#menuItems.indexOf(element)
@@ -198,7 +209,11 @@ class Menu {
198209
i = (i + increment) % len
199210
) {
200211
const menuItem = this.#menuItems[i];
201-
if (!menuItem.disabled && !menuItem.classList.contains("hidden")) {
212+
if (
213+
!menuItem.disabled &&
214+
!menuItem.classList.contains("hidden") &&
215+
check(menuItem)
216+
) {
202217
menuItem.focus();
203218
this.#lastIndex = i;
204219
break;

web/viewer.html

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@
130130
<div id="viewsManagerTitle">
131131
<div id="viewsManagerSelector">
132132
<button
133-
class="toolbarButton viewsManagerButton"
133+
class="toolbarButton viewsManagerButton hasPopupMenu"
134134
type="button"
135135
id="viewsManagerSelectorButton"
136136
tabindex="0"
@@ -141,7 +141,7 @@
141141
>
142142
<span data-l10n-id="pdfjs-views-manager-view-selector-button-label"></span>
143143
</button>
144-
<menu id="viewsManagerSelectorOptions" role="listbox" class="popupMenu hidden withMark">
144+
<menu id="viewsManagerSelectorOptions" role="listbox" class="popupMenu withMark">
145145
<li>
146146
<button id="thumbnailsViewMenu" role="option" type="button" tabindex="-1">
147147
<span data-l10n-id="pdfjs-views-manager-pages-option-label"></span>
@@ -196,15 +196,16 @@
196196
<div id="actionSelector">
197197
<button
198198
id="viewsManagerStatusActionButton"
199-
class="viewsManagerButton"
199+
class="viewsManagerButton hasPopupMenu"
200200
type="button"
201201
tabindex="0"
202202
aria-haspopup="menu"
203203
aria-controls="viewsManagerStatusActionOptions"
204+
aria-expanded="false"
204205
>
205206
<span data-l10n-id="pdfjs-views-manager-pages-status-action-button-label"></span>
206207
</button>
207-
<menu id="viewsManagerStatusActionOptions" class="popupMenu hidden">
208+
<menu id="viewsManagerStatusActionOptions" class="popupMenu">
208209
<li>
209210
<button id="viewsManagerStatusActionCopy" class="noIcon" role="menuitem" type="button" tabindex="0">
210211
<span data-l10n-id="pdfjs-views-manager-pages-status-copy-button-label"></span>

web/views_manager.css

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -394,6 +394,7 @@
394394

395395
#actionSelector {
396396
height: 32px;
397+
min-width: 115px;
397398
width: auto;
398399
display: block;
399400

@@ -425,8 +426,9 @@
425426
}
426427
}
427428

428-
> .contextMenu {
429-
min-width: 115px;
429+
> .popupMenu {
430+
width: auto;
431+
z-index: 1;
430432
}
431433
}
432434
}

0 commit comments

Comments
 (0)