Skip to content

Commit

Permalink
Rollup merge of rust-lang#104946 - notriddle:notriddle/popover-menu-f…
Browse files Browse the repository at this point in the history
…ocus, 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.
  • Loading branch information
matthiaskrgr authored Nov 27, 2022
2 parents 95e3ba6 + c26074a commit be3bc0a
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 5 deletions.
20 changes: 16 additions & 4 deletions src/librustdoc/html/static/js/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ function loadCss(cssUrl) {
if (event.ctrlKey || event.altKey || event.metaKey) {
return;
}
window.hideAllModals(false);
addClass(getSettingsButton(), "rotate");
event.preventDefault();
// Sending request for the CSS and the JS files at the same time so it will
Expand Down Expand Up @@ -377,7 +378,7 @@ function loadCss(cssUrl) {
}
ev.preventDefault();
searchState.defocus();
window.hidePopoverMenus();
window.hideAllModals(true); // true = reset focus for notable traits
}

function handleShortcut(ev) {
Expand Down Expand Up @@ -767,6 +768,7 @@ function loadCss(cssUrl) {
};

function showSidebar() {
window.hideAllModals(false);
window.rustdocMobileScrollLock();
const sidebar = document.getElementsByClassName("sidebar")[0];
addClass(sidebar, "shown");
Expand Down Expand Up @@ -843,7 +845,7 @@ function loadCss(cssUrl) {
// Make this function idempotent.
return;
}
hideNotable(false);
window.hideAllModals(false);
const ty = e.getAttribute("data-ty");
const wrapper = document.createElement("div");
wrapper.innerHTML = "<div class=\"docblock\">" + window.NOTABLE_TRAITS[ty] + "</div>";
Expand Down Expand Up @@ -1049,14 +1051,24 @@ function loadCss(cssUrl) {
return container;
}

/**
* Hide popover menus, notable trait tooltips, and the sidebar (if applicable).
*
* Pass "true" to reset focus for notable traits.
*/
window.hideAllModals = function(switchFocus) {
hideSidebar();
window.hidePopoverMenus();
hideNotable(switchFocus);
};

/**
* Hide all the popover menus.
*/
window.hidePopoverMenus = function() {
onEachLazy(document.querySelectorAll(".search-form .popover"), elem => {
elem.style.display = "none";
});
hideNotable(false);
};

/**
Expand All @@ -1081,7 +1093,7 @@ function loadCss(cssUrl) {
function showHelp() {
const menu = getHelpMenu(true);
if (menu.style.display === "none") {
window.hidePopoverMenus();
window.hideAllModals();
menu.style.display = "";
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/html/static/js/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@
event.preventDefault();
const shouldDisplaySettings = settingsMenu.style.display === "none";

window.hidePopoverMenus();
window.hideAllModals();
if (shouldDisplaySettings) {
displaySettings();
}
Expand Down
25 changes: 25 additions & 0 deletions src/test/rustdoc-gui/notable-trait.goml
Original file line number Diff line number Diff line change
Expand Up @@ -200,12 +200,14 @@ move-cursor-to: "//*[@class='notable popover']"
assert-count: ("//*[@class='notable popover']", 1)
press-key: "Escape"
assert-count: ("//*[@class='notable popover']", 0)
assert: "#method\.create_an_iterator_from_read .notable-traits:focus"

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

// Check that pressing tab over and over works.
click: "//*[@id='method.create_an_iterator_from_read']//*[@class='notable-traits']"
Expand Down Expand Up @@ -249,3 +251,26 @@ click: "#settings-menu a"
press-key: "Escape"
// We ensure we didn't come back to the previous focused item.
assert-window-property-false: {"scrollY": |scroll|}

// Opening the mobile sidebar should close the popover.
size: (650, 600)
click: "//*[@id='method.create_an_iterator_from_read']//*[@class='notable-traits']"
assert-count: ("//*[@class='notable popover']", 1)
click: ".sidebar-menu-toggle"
assert: "//*[@class='sidebar shown']"
assert-count: ("//*[@class='notable popover']", 0)
assert-false: "#method\.create_an_iterator_from_read .notable-traits:focus"
// Clicking a notable popover should close the sidebar.
click: "//*[@id='method.create_an_iterator_from_read']//*[@class='notable-traits']"
assert-count: ("//*[@class='notable popover']", 1)
assert-false: "//*[@class='sidebar shown']"

// Also check the focus handling for the help button.
size: (1100, 600)
reload:
assert-count: ("//*[@class='notable popover']", 0)
click: "//*[@id='method.create_an_iterator_from_read']//*[@class='notable-traits']"
assert-count: ("//*[@class='notable popover']", 1)
click: "#help-button a"
assert-count: ("//*[@class='notable popover']", 0)
assert-false: "#method\.create_an_iterator_from_read .notable-traits:focus"
21 changes: 21 additions & 0 deletions src/test/rustdoc-gui/pocket-menu.goml
Original file line number Diff line number Diff line change
Expand Up @@ -75,3 +75,24 @@ assert-css: (
)
compare-elements-css: ("#help-button .popover", "#help-button .top", ["border-color"])
compare-elements-css: ("#help-button .popover", "#help-button .bottom", ["border-color"])

// Opening the mobile sidebar should close the settings popover.
size: (650, 600)
click: "#settings-menu a"
assert-css: ("#settings-menu .popover", {"display": "block"})
click: ".sidebar-menu-toggle"
assert: "//*[@class='sidebar shown']"
assert-css: ("#settings-menu .popover", {"display": "none"})
// Opening the settings popover should close the sidebar.
click: "#settings-menu a"
assert-css: ("#settings-menu .popover", {"display": "block"})
assert-false: "//*[@class='sidebar shown']"

// Opening the settings popover at start (which async loads stuff) should also close.
reload:
click: ".sidebar-menu-toggle"
assert: "//*[@class='sidebar shown']"
assert-false: "#settings-menu .popover"
click: "#settings-menu a"
assert-false: "//*[@class='sidebar shown']"
wait-for: "#settings-menu .popover"
6 changes: 6 additions & 0 deletions src/test/rustdoc-gui/sidebar-mobile.goml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@ assert-css: ("//nav[contains(@class, 'sidebar')]//h2/a[text()='In test_docs']/pa
click: "body"
assert-css: (".sidebar", {"display": "block", "left": "-1000px"})

// Open the sidebar menu, and make sure pressing Escape closes it.
click: ".sidebar-menu-toggle"
assert-css: (".sidebar", {"left": "0px"})
press-key: "Escape"
assert-css: (".sidebar", {"display": "block", "left": "-1000px"})

// Check that the topbar is visible
assert-property: (".mobile-topbar", {"clientHeight": "45"})

Expand Down

0 comments on commit be3bc0a

Please sign in to comment.