Skip to content

Commit 55cf566

Browse files
authored
Rollup merge of #104946 - notriddle:notriddle/popover-menu-focus, r=GuillaumeGomez
rustdoc: improve popover focus handling JS This commit fixes a few inconsistencies and erratic behavior from the notable traits, settings, and sidebar popups: * It makes it so that pressing Escape closes the mobile sidebar. This is a bit difficult to do on iPhone, but on other setups like desktop tiling window managers, it's easy and makes sense. * It makes sure that pressing escape while a notable trait popover is open focuses the popover's toggle button, instead of leaving nothing focused, since that makes more sense with keyboard navigation. Clicking the settings, help, or sidebar buttons, however, will not focus the notable trait popover toggle button. * It ensures that notable trait and settings popovers are exclusive with the mobile sidebar. Nothing should ever overlap a popover, and there should never be more than one popover open at once.
2 parents e3165a3 + c26074a commit 55cf566

File tree

5 files changed

+69
-5
lines changed

5 files changed

+69
-5
lines changed

src/librustdoc/html/static/js/main.js

+16-4
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,7 @@ function loadCss(cssUrl) {
202202
if (event.ctrlKey || event.altKey || event.metaKey) {
203203
return;
204204
}
205+
window.hideAllModals(false);
205206
addClass(getSettingsButton(), "rotate");
206207
event.preventDefault();
207208
// Sending request for the CSS and the JS files at the same time so it will
@@ -377,7 +378,7 @@ function loadCss(cssUrl) {
377378
}
378379
ev.preventDefault();
379380
searchState.defocus();
380-
window.hidePopoverMenus();
381+
window.hideAllModals(true); // true = reset focus for notable traits
381382
}
382383

383384
function handleShortcut(ev) {
@@ -767,6 +768,7 @@ function loadCss(cssUrl) {
767768
};
768769

769770
function showSidebar() {
771+
window.hideAllModals(false);
770772
window.rustdocMobileScrollLock();
771773
const sidebar = document.getElementsByClassName("sidebar")[0];
772774
addClass(sidebar, "shown");
@@ -843,7 +845,7 @@ function loadCss(cssUrl) {
843845
// Make this function idempotent.
844846
return;
845847
}
846-
hideNotable(false);
848+
window.hideAllModals(false);
847849
const ty = e.getAttribute("data-ty");
848850
const wrapper = document.createElement("div");
849851
wrapper.innerHTML = "<div class=\"docblock\">" + window.NOTABLE_TRAITS[ty] + "</div>";
@@ -1049,14 +1051,24 @@ function loadCss(cssUrl) {
10491051
return container;
10501052
}
10511053

1054+
/**
1055+
* Hide popover menus, notable trait tooltips, and the sidebar (if applicable).
1056+
*
1057+
* Pass "true" to reset focus for notable traits.
1058+
*/
1059+
window.hideAllModals = function(switchFocus) {
1060+
hideSidebar();
1061+
window.hidePopoverMenus();
1062+
hideNotable(switchFocus);
1063+
};
1064+
10521065
/**
10531066
* Hide all the popover menus.
10541067
*/
10551068
window.hidePopoverMenus = function() {
10561069
onEachLazy(document.querySelectorAll(".search-form .popover"), elem => {
10571070
elem.style.display = "none";
10581071
});
1059-
hideNotable(false);
10601072
};
10611073

10621074
/**
@@ -1081,7 +1093,7 @@ function loadCss(cssUrl) {
10811093
function showHelp() {
10821094
const menu = getHelpMenu(true);
10831095
if (menu.style.display === "none") {
1084-
window.hidePopoverMenus();
1096+
window.hideAllModals();
10851097
menu.style.display = "";
10861098
}
10871099
}

src/librustdoc/html/static/js/settings.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@
268268
event.preventDefault();
269269
const shouldDisplaySettings = settingsMenu.style.display === "none";
270270

271-
window.hidePopoverMenus();
271+
window.hideAllModals();
272272
if (shouldDisplaySettings) {
273273
displaySettings();
274274
}

src/test/rustdoc-gui/notable-trait.goml

+25
Original file line numberDiff line numberDiff line change
@@ -200,12 +200,14 @@ move-cursor-to: "//*[@class='notable popover']"
200200
assert-count: ("//*[@class='notable popover']", 1)
201201
press-key: "Escape"
202202
assert-count: ("//*[@class='notable popover']", 0)
203+
assert: "#method\.create_an_iterator_from_read .notable-traits:focus"
203204

204205
// Check that clicking outside works.
205206
click: "//*[@id='method.create_an_iterator_from_read']//*[@class='notable-traits']"
206207
assert-count: ("//*[@class='notable popover']", 1)
207208
click: ".search-input"
208209
assert-count: ("//*[@class='notable popover']", 0)
210+
assert-false: "#method\.create_an_iterator_from_read .notable-traits:focus"
209211

210212
// Check that pressing tab over and over works.
211213
click: "//*[@id='method.create_an_iterator_from_read']//*[@class='notable-traits']"
@@ -249,3 +251,26 @@ click: "#settings-menu a"
249251
press-key: "Escape"
250252
// We ensure we didn't come back to the previous focused item.
251253
assert-window-property-false: {"scrollY": |scroll|}
254+
255+
// Opening the mobile sidebar should close the popover.
256+
size: (650, 600)
257+
click: "//*[@id='method.create_an_iterator_from_read']//*[@class='notable-traits']"
258+
assert-count: ("//*[@class='notable popover']", 1)
259+
click: ".sidebar-menu-toggle"
260+
assert: "//*[@class='sidebar shown']"
261+
assert-count: ("//*[@class='notable popover']", 0)
262+
assert-false: "#method\.create_an_iterator_from_read .notable-traits:focus"
263+
// Clicking a notable popover should close the sidebar.
264+
click: "//*[@id='method.create_an_iterator_from_read']//*[@class='notable-traits']"
265+
assert-count: ("//*[@class='notable popover']", 1)
266+
assert-false: "//*[@class='sidebar shown']"
267+
268+
// Also check the focus handling for the help button.
269+
size: (1100, 600)
270+
reload:
271+
assert-count: ("//*[@class='notable popover']", 0)
272+
click: "//*[@id='method.create_an_iterator_from_read']//*[@class='notable-traits']"
273+
assert-count: ("//*[@class='notable popover']", 1)
274+
click: "#help-button a"
275+
assert-count: ("//*[@class='notable popover']", 0)
276+
assert-false: "#method\.create_an_iterator_from_read .notable-traits:focus"

src/test/rustdoc-gui/pocket-menu.goml

+21
Original file line numberDiff line numberDiff line change
@@ -75,3 +75,24 @@ assert-css: (
7575
)
7676
compare-elements-css: ("#help-button .popover", "#help-button .top", ["border-color"])
7777
compare-elements-css: ("#help-button .popover", "#help-button .bottom", ["border-color"])
78+
79+
// Opening the mobile sidebar should close the settings popover.
80+
size: (650, 600)
81+
click: "#settings-menu a"
82+
assert-css: ("#settings-menu .popover", {"display": "block"})
83+
click: ".sidebar-menu-toggle"
84+
assert: "//*[@class='sidebar shown']"
85+
assert-css: ("#settings-menu .popover", {"display": "none"})
86+
// Opening the settings popover should close the sidebar.
87+
click: "#settings-menu a"
88+
assert-css: ("#settings-menu .popover", {"display": "block"})
89+
assert-false: "//*[@class='sidebar shown']"
90+
91+
// Opening the settings popover at start (which async loads stuff) should also close.
92+
reload:
93+
click: ".sidebar-menu-toggle"
94+
assert: "//*[@class='sidebar shown']"
95+
assert-false: "#settings-menu .popover"
96+
click: "#settings-menu a"
97+
assert-false: "//*[@class='sidebar shown']"
98+
wait-for: "#settings-menu .popover"

src/test/rustdoc-gui/sidebar-mobile.goml

+6
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,12 @@ assert-css: ("//nav[contains(@class, 'sidebar')]//h2/a[text()='In test_docs']/pa
3232
click: "body"
3333
assert-css: (".sidebar", {"display": "block", "left": "-1000px"})
3434

35+
// Open the sidebar menu, and make sure pressing Escape closes it.
36+
click: ".sidebar-menu-toggle"
37+
assert-css: (".sidebar", {"left": "0px"})
38+
press-key: "Escape"
39+
assert-css: (".sidebar", {"display": "block", "left": "-1000px"})
40+
3541
// Check that the topbar is visible
3642
assert-property: (".mobile-topbar", {"clientHeight": "45"})
3743

0 commit comments

Comments
 (0)