Skip to content

Commit 5e6cf0b

Browse files
drammockjpfeuffertrallard
authored
BUG: fix highlighting of current version in switcher menu (#1357)
* cherry-pick changes from 1305 Co-authored-by: Julianus Pfeuffer <[email protected]> * update switcher IDs and fix broken tests * fix another one * fix rebase snafu * JS fixes * update highlighting test for new color theme * more rebase cruft * fix ARIA roles Co-authored-by: Tania Allard <[email protected]> * fix tests * Update src/pydata_sphinx_theme/theme/pydata_sphinx_theme/components/version-switcher.html Co-authored-by: Tania Allard <[email protected]> * Update tests/test_build/navbar_switcher.html --------- Co-authored-by: Julianus Pfeuffer <[email protected]> Co-authored-by: Tania Allard <[email protected]>
1 parent 00c3575 commit 5e6cf0b

File tree

5 files changed

+30
-10
lines changed

5 files changed

+30
-10
lines changed

src/pydata_sphinx_theme/assets/scripts/pydata-sphinx-theme.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,7 @@ if (versionSwitcherBtns.length) {
357357
const anchor = document.createElement("a");
358358
anchor.setAttribute("class", "list-group-item list-group-item-action py-1");
359359
anchor.setAttribute("href", `${entry.url}${currentFilePath}`);
360+
anchor.setAttribute("role", "option");
360361
const span = document.createElement("span");
361362
span.textContent = `${entry.name}`;
362363
anchor.appendChild(span);
@@ -368,10 +369,11 @@ if (versionSwitcherBtns.length) {
368369
// this version, rather than using sphinx's {{ version }} variable.
369370
// also highlight the dropdown entry for the currently-viewed
370371
// version's entry
371-
if (entry.version == DOCUMENTATION_OPTIONS.version_switcher_version_match) {
372+
if (entry.version == DOCUMENTATION_OPTIONS.theme_switcher_version_match) {
372373
anchor.classList.add("active");
373374
versionSwitcherBtns.forEach((btn) => {
374-
btn.innerText = btn.dataset["activeVersionName"] = entry.name;
375+
btn.innerText = entry.name;
376+
btn.dataset["activeVersionName"] = entry.name;
375377
btn.dataset["activeVersion"] = entry.version;
376378
});
377379
}

src/pydata_sphinx_theme/theme/pydata_sphinx_theme/components/version-switcher.html

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,11 @@
33
<script>
44
document.write(`
55
<div class="version-switcher__container dropdown">
6-
<button type="button" class="version-switcher__button btn btn-sm navbar-btn dropdown-toggle" data-bs-toggle="dropdown">
6+
<button id="versionswitcherbutton" type="button" role="button" class="version-switcher__button btn btn-sm navbar-btn dropdown-toggle" data-bs-toggle="dropdown" aria-haspopup="listbox" aria-controls="versionswitcherlist" aria-label="Version switcher list">
77
{{ theme_switcher.get('version_match') }} <!-- this text may get changed later by javascript -->
88
<span class="caret"></span>
99
</button>
10-
<div class="version-switcher__menu dropdown-menu list-group-flush py-0">
10+
<div id="versionswitcherlist" class="version-switcher__menu dropdown-menu list-group-flush py-0" role="listbox" aria-labelledby="versionswitcherbutton">
1111
<!-- dropdown will be populated by javascript on page load -->
1212
</div>
1313
</div>

tests/test_a11y.py

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212

1313
# Using importorskip to ensure these tests are only loaded if Playwright is installed.
1414
playwright = pytest.importorskip("playwright")
15-
from playwright.sync_api import Page # noqa: E402
15+
from playwright.sync_api import Page, expect # noqa: E402
1616

1717
# Important note: automated accessibility scans can only find a fraction of
1818
# potential accessibility issues.
@@ -112,3 +112,19 @@ def test_axe_core_kitchen_sink(
112112

113113
# Expect Axe-core to have found 0 accessibility violations
114114
assert len(results["violations"]) == 0, pretty_axe_results(results, selector)
115+
116+
117+
def test_version_switcher_highlighting(page: Page, url_base: str) -> None:
118+
"""This isn't an a11y test, but needs a served site for Javascript to inject the version menu."""
119+
page.goto(url=url_base)
120+
# no need to include_hidden here ↓↓↓, we just need to get the active version name
121+
button = page.get_by_role("button").filter(has_text="dev")
122+
active_version_name = button.get_attribute("data-active-version-name")
123+
# here we do include_hidden, so sidebar & topbar menus should each have a matching entry:
124+
entries = page.get_by_role("option", include_hidden=True).filter(
125+
has_text=active_version_name
126+
)
127+
assert entries.count() == 2
128+
# make sure they're highlighted
129+
for entry in entries.all():
130+
expect(entry).to_have_css("color", "rgb(10, 125, 145)")

tests/test_build.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -648,12 +648,14 @@ def test_show_nav_level(sphinx_build_factory) -> None:
648648
assert "checked" in checkbox.attrs
649649

650650

651+
# the switcher files tested in test_version_switcher_error_states, not all of them exist
651652
switcher_files = ["switcher.json", "http://a.b/switcher.json", "missing_url.json"]
652-
"the switcher files tested in test_version_switcher, not all of them exist"
653653

654654

655655
@pytest.mark.parametrize("url", switcher_files)
656-
def test_version_switcher(sphinx_build_factory, file_regression, url) -> None:
656+
def test_version_switcher_error_states(
657+
sphinx_build_factory, file_regression, url
658+
) -> None:
657659
"""Regression test the version switcher dropdown HTML.
658660
659661
Note that a lot of the switcher HTML gets populated by JavaScript,
@@ -878,7 +880,7 @@ def test_translations(sphinx_build_factory) -> None:
878880
assert "Créé en utilisant" in str(footer)
879881
assert "Construit avec le" in str(footer)
880882

881-
footer_article = index.select(".prev-next-footer")[0]
883+
footer_article = index.select(".prev-next-area")[0]
882884
assert "précédent" in str(footer_article)
883885
assert "page suivante" in str(footer_article)
884886

tests/test_build/navbar_switcher.html

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
<script>
22
document.write(`
33
<div class="version-switcher__container dropdown">
4-
<button type="button" class="version-switcher__button btn btn-sm navbar-btn dropdown-toggle" data-bs-toggle="dropdown">
4+
<button id="versionswitcherbutton" type="button" role="button" class="version-switcher__button btn btn-sm navbar-btn dropdown-toggle" data-bs-toggle="dropdown" aria-haspopup="listbox" aria-controls="versionswitcherlist" aria-label="Version switcher list">
55
0.7.1 <!-- this text may get changed later by javascript -->
66
<span class="caret"></span>
77
</button>
8-
<div class="version-switcher__menu dropdown-menu list-group-flush py-0">
8+
<div id="versionswitcherlist" class="version-switcher__menu dropdown-menu list-group-flush py-0" role="listbox" aria-labelledby="versionswitcherbutton">
99
<!-- dropdown will be populated by javascript on page load -->
1010
</div>
1111
</div>

0 commit comments

Comments
 (0)