From 11e04118fbed3f1708168bd394743a7fa003f9f4 Mon Sep 17 00:00:00 2001 From: Paul Ivanov Date: Thu, 27 Jun 2024 02:58:34 -0700 Subject: [PATCH] use ellipsis to truncate breadcrumbs, and test with playwright (#1861) This PR: - Fixes breadcrumb truncation to use CSS `overflow: ellipsis` - Provides an example for combining playwright and sphinx_build_factory to more thoroughly test our theme components. - Applies that approach to testing breadcrumb truncation via ellipsis when the breadcrumb is placed in various parts of our layout. joint work with @munkm and @drammock closes #1583 closes #1568 lays groundwork for addressing #229 --------- Co-authored-by: Daniel McCloy Co-authored-by: Madicken Munk Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: gabalafou --- .../styles/components/_breadcrumbs.scss | 12 +++ .../styles/sections/_header-article.scss | 9 -- .../components/breadcrumbs.html | 9 +- tests/conftest.py | 50 ++++++++++- tests/sites/breadcrumbs/Makefile | 20 +++++ .../breadcrumbs/_static/emptydarklogo.png | 0 tests/sites/breadcrumbs/_static/emptylogo.png | 0 tests/sites/breadcrumbs/conf.py | 32 +++++++ .../sites/breadcrumbs/hansel/gretel/house.rst | 2 + tests/sites/breadcrumbs/index.rst | 17 ++++ tests/sites/breadcrumbs/missing_url.json | 14 +++ tests/sites/breadcrumbs/switcher.json | 15 ++++ tests/test_a11y.py | 77 +++------------- tests/test_playwright.py | 87 +++++++++++++++++++ 14 files changed, 259 insertions(+), 85 deletions(-) create mode 100644 tests/sites/breadcrumbs/Makefile create mode 100644 tests/sites/breadcrumbs/_static/emptydarklogo.png create mode 100644 tests/sites/breadcrumbs/_static/emptylogo.png create mode 100644 tests/sites/breadcrumbs/conf.py create mode 100644 tests/sites/breadcrumbs/hansel/gretel/house.rst create mode 100644 tests/sites/breadcrumbs/index.rst create mode 100644 tests/sites/breadcrumbs/missing_url.json create mode 100644 tests/sites/breadcrumbs/switcher.json create mode 100644 tests/test_playwright.py diff --git a/src/pydata_sphinx_theme/assets/styles/components/_breadcrumbs.scss b/src/pydata_sphinx_theme/assets/styles/components/_breadcrumbs.scss index a2d02ab26..d2e511b1b 100644 --- a/src/pydata_sphinx_theme/assets/styles/components/_breadcrumbs.scss +++ b/src/pydata_sphinx_theme/assets/styles/components/_breadcrumbs.scss @@ -13,6 +13,18 @@ ul.bd-breadcrumbs { li.breadcrumb-item { display: flex; align-items: center; + white-space: nowrap; + overflow-x: hidden; + + a, + .ellipsis { + overflow-x: hidden; + text-overflow: ellipsis; + + // Need to add margin, otherwise the overflow: hidden rule on the parent + // will hide the focus ring + margin: $focus-ring-width; + } // Style should look like heavier in-page links // keeping visited in the default link colour diff --git a/src/pydata_sphinx_theme/assets/styles/sections/_header-article.scss b/src/pydata_sphinx_theme/assets/styles/sections/_header-article.scss index 8c1855831..d8826e331 100644 --- a/src/pydata_sphinx_theme/assets/styles/sections/_header-article.scss +++ b/src/pydata_sphinx_theme/assets/styles/sections/_header-article.scss @@ -1,18 +1,9 @@ .header-article__inner { - display: flex; padding: 0 0.5rem; // The items define the height so that it disappears if there are no items .header-article-item { min-height: var(--pst-header-article-height); - height: var(--pst-header-article-height); - } - - .header-article-items__start, - .header-article-items__end { - display: flex; - align-items: start; - gap: 0.5rem; } .header-article-items__end { diff --git a/src/pydata_sphinx_theme/theme/pydata_sphinx_theme/components/breadcrumbs.html b/src/pydata_sphinx_theme/theme/pydata_sphinx_theme/components/breadcrumbs.html index 28d3dc896..8a15a4dda 100644 --- a/src/pydata_sphinx_theme/theme/pydata_sphinx_theme/components/breadcrumbs.html +++ b/src/pydata_sphinx_theme/theme/pydata_sphinx_theme/components/breadcrumbs.html @@ -1,12 +1,5 @@ {# Displays (and links to) the parent section(s) of the currently viewed page. #} {%- block breadcrumbs %} -{# - If we have more than 3 parents (excluding the home page) then we remove - The ones in the middle and add an ellipsis. -#} -{% if parents|length>2 %} -{% set parents=[parents[0], {"title": ''}, parents[-1]] %} -{% endif %} {#- Hide breadcrumbs on the home page #} {% if title and pagename != root_doc %} @@ -25,7 +18,7 @@
  • {{ doc.title }}
    • {% endif %} {%- endfor %} -
  • {{ title|truncate(15, False) }}
    • +
  • {{ title }}
    • {% endif %} diff --git a/tests/conftest.py b/tests/conftest.py index 78464af00..c6e7b866f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,9 +1,12 @@ """Configuration of the pytest session.""" import re +import time +from http.client import HTTPConnection from os import environ from pathlib import Path from shutil import copytree +from subprocess import PIPE, Popen from typing import Callable import pytest @@ -14,7 +17,9 @@ pytest_plugins = "sphinx.testing.fixtures" -path_tests = Path(__file__).parent +tests_path = Path(__file__).parent +repo_path = tests_path.parent +docs_build_path = repo_path / "docs" / "_build" / "html" # -- Utils method ------------------------------------------------------------ @@ -81,8 +86,49 @@ def _func(src_folder: str, **kwargs) -> SphinxBuild: srcdir = sphinx_path(srcdir) - copytree(path_tests / "sites" / src_folder, tmp_path / src_folder) + copytree(tests_path / "sites" / src_folder, tmp_path / src_folder) app = make_app(srcdir=srcdir, **kwargs) return SphinxBuild(app, tmp_path / src_folder) yield _func + + +@pytest.fixture(scope="module") +def url_base(): + """Start local server on built docs and return the localhost URL as the base URL.""" + # Use a port that is not commonly used during development or else you will + # force the developer to stop running their dev server in order to run the + # tests. + port = "8213" + host = "localhost" + url = f"http://{host}:{port}" + + # Try starting the server + process = Popen( + ["python", "-m", "http.server", port, "--directory", docs_build_path], + stdout=PIPE, + ) + + # Try connecting to the server + retries = 5 + while retries > 0: + conn = HTTPConnection(host, port) + try: + conn.request("HEAD", "/") + response = conn.getresponse() + if response is not None: + yield url + break + except ConnectionRefusedError: + time.sleep(1) + retries -= 1 + + # If the code above never yields a URL, then we were never able to connect + # to the server and retries == 0. + if not retries: + raise RuntimeError("Failed to start http server in 5 seconds") + else: + # Otherwise the server started and this fixture is done now and we clean + # up by stopping the server. + process.terminate() + process.wait() diff --git a/tests/sites/breadcrumbs/Makefile b/tests/sites/breadcrumbs/Makefile new file mode 100644 index 000000000..d4bb2cbb9 --- /dev/null +++ b/tests/sites/breadcrumbs/Makefile @@ -0,0 +1,20 @@ +# Minimal makefile for Sphinx documentation +# + +# You can set these variables from the command line, and also +# from the environment for the first two. +SPHINXOPTS ?= +SPHINXBUILD ?= sphinx-build +SOURCEDIR = . +BUILDDIR = _build + +# Put it first so that "make" without argument is like "make help". +help: + @$(SPHINXBUILD) -M help "$(SOURCEDIR)" "$(BUILDDIR)" $(SPHINXOPTS) $(O) + +.PHONY: help Makefile + +# Catch-all target: route all unknown targets to Sphinx using the new +# "make mode" option. $(O) is meant as a shortcut for $(SPHINXOPTS). +%: Makefile + @$(SPHINXBUILD) -M $@ "$(SOURCEDIR)" "$(BUILDDIR)" $(SPHINXOPTS) $(O) diff --git a/tests/sites/breadcrumbs/_static/emptydarklogo.png b/tests/sites/breadcrumbs/_static/emptydarklogo.png new file mode 100644 index 000000000..e69de29bb diff --git a/tests/sites/breadcrumbs/_static/emptylogo.png b/tests/sites/breadcrumbs/_static/emptylogo.png new file mode 100644 index 000000000..e69de29bb diff --git a/tests/sites/breadcrumbs/conf.py b/tests/sites/breadcrumbs/conf.py new file mode 100644 index 000000000..777fc8dab --- /dev/null +++ b/tests/sites/breadcrumbs/conf.py @@ -0,0 +1,32 @@ +"""Test conf file.""" + +# -- Project information ----------------------------------------------------- + +project = "PyData Tests" +copyright = "2020, Pydata community" +author = "Pydata community" + +root_doc = "index" + +# -- General configuration --------------------------------------------------- + +# Add any Sphinx extension module names here, as strings. They can be +# extensions coming with Sphinx (named 'sphinx.ext.*') or your custom +# ones. +extensions = [] +html_theme = "pydata_sphinx_theme" +html_logo = "_static/emptylogo.png" +html_copy_source = True +html_sourcelink_suffix = "" + +# Base options, we can add other key/vals later +html_sidebars = {"section1/index": ["sidebar-nav-bs.html"]} + +html_theme_options = { + "footer_start": ["breadcrumbs"], + "footer_center": ["breadcrumbs"], + "footer_end": ["breadcrumbs"], + "primary_sidebar_end": ["breadcrumbs"], + "secondary_sidebar_items": ["breadcrumbs"], + "article_header_start": ["breadcrumbs"], +} diff --git a/tests/sites/breadcrumbs/hansel/gretel/house.rst b/tests/sites/breadcrumbs/hansel/gretel/house.rst new file mode 100644 index 000000000..05f2e413a --- /dev/null +++ b/tests/sites/breadcrumbs/hansel/gretel/house.rst @@ -0,0 +1,2 @@ +In the oven with my sister, so hot right now. Soooo. Hotttt. +============================================================ diff --git a/tests/sites/breadcrumbs/index.rst b/tests/sites/breadcrumbs/index.rst new file mode 100644 index 000000000..3997d83e7 --- /dev/null +++ b/tests/sites/breadcrumbs/index.rst @@ -0,0 +1,17 @@ +Index ``with code`` in title +============================ + +.. toctree:: + :caption: My caption + :numbered: + + hansel/gretel/house + +A header +-------- + +A sub-header +~~~~~~~~~~~~ + +A sub-sub-header +```````````````` diff --git a/tests/sites/breadcrumbs/missing_url.json b/tests/sites/breadcrumbs/missing_url.json new file mode 100644 index 000000000..b7e92ecca --- /dev/null +++ b/tests/sites/breadcrumbs/missing_url.json @@ -0,0 +1,14 @@ +[ + { + "name": "v0.7.1 (stable)", + "version": "0.7.1", + "url": "https://pydata-sphinx-theme.readthedocs.io/en/v0.7.1/" + }, + { + "version": "0.7.0", + "url": "https://pydata-sphinx-theme.readthedocs.io/en/v0.7.0/" + }, + { + "version": "0.6.3" + } +] diff --git a/tests/sites/breadcrumbs/switcher.json b/tests/sites/breadcrumbs/switcher.json new file mode 100644 index 000000000..bfbeb35b7 --- /dev/null +++ b/tests/sites/breadcrumbs/switcher.json @@ -0,0 +1,15 @@ +[ + { + "name": "v0.7.1 (stable)", + "version": "0.7.1", + "url": "https://pydata-sphinx-theme.readthedocs.io/en/v0.7.1/" + }, + { + "version": "0.7.0", + "url": "https://pydata-sphinx-theme.readthedocs.io/en/v0.7.0/" + }, + { + "version": "0.6.3", + "url": "https://pydata-sphinx-theme.readthedocs.io/en/v0.6.3/" + } +] diff --git a/tests/test_a11y.py b/tests/test_a11y.py index 1ef16feb9..4523be64a 100644 --- a/tests/test_a11y.py +++ b/tests/test_a11y.py @@ -1,9 +1,5 @@ """Using Axe-core, scan the Kitchen Sink pages for accessibility violations.""" -import time -from http.client import HTTPConnection -from pathlib import Path -from subprocess import PIPE, Popen from urllib.parse import urljoin import pytest @@ -25,50 +21,6 @@ # people. It just means that page is free of common testable accessibility # pitfalls. -path_repo = Path(__file__).parent.parent -path_docs_build = path_repo / "docs" / "_build" / "html" - - -@pytest.fixture(scope="module") -def url_base(): - """Start local server on built docs and return the localhost URL as the base URL.""" - # Use a port that is not commonly used during development or else you will - # force the developer to stop running their dev server in order to run the - # tests. - port = "8213" - host = "localhost" - url = f"http://{host}:{port}" - - # Try starting the server - process = Popen( - ["python", "-m", "http.server", port, "--directory", path_docs_build], - stdout=PIPE, - ) - - # Try connecting to the server - retries = 5 - while retries > 0: - conn = HTTPConnection(host, port) - try: - conn.request("HEAD", "/") - response = conn.getresponse() - if response is not None: - yield url - break - except ConnectionRefusedError: - time.sleep(1) - retries -= 1 - - # If the code above never yields a URL, then we were never able to connect - # to the server and retries == 0. - if not retries: - raise RuntimeError("Failed to start http server in 5 seconds") - else: - # Otherwise the server started and this fixture is done now and we clean - # up by stopping the server. - process.terminate() - process.wait() - def filter_ignored_violations(violations, url_pathname): """Filter out ignored axe-core violations. @@ -227,24 +179,6 @@ def test_axe_core( assert len(filtered_violations) == 0, format_violations(filtered_violations) -def test_version_switcher_highlighting(page: Page, url_base: str) -> None: - """This isn't an a11y test, but needs a served site for Javascript to inject the version menu.""" - page.goto(url=url_base) - # no need to include_hidden here ↓↓↓, we just need to get the active version name - button = page.get_by_role("button").filter(has_text="dev") - active_version_name = button.get_attribute("data-active-version-name") - # here we do include_hidden, so sidebar & topbar menus should each have a matching entry: - entries = page.get_by_role("option", include_hidden=True).filter( - has_text=active_version_name - ) - assert entries.count() == 2 - # make sure they're highlighted - for entry in entries.all(): - light_mode = "rgb(10, 125, 145)" # pst-color-primary - # dark_mode = "rgb(63, 177, 197)" - expect(entry).to_have_css("color", light_mode) - - @pytest.mark.a11y def test_code_block_tab_stop(page: Page, url_base: str) -> None: """Code blocks that have scrollable content should be tab stops.""" @@ -306,3 +240,14 @@ def test_notebook_ipywidget_output_tab_stop(page: Page, url_base: str) -> None: # ...and so our js code on the page should make it keyboard-focusable # (tabIndex = 0) assert ipywidget.evaluate("el => el.tabIndex") == 0 + + +def test_breadcrumb_expansion(page: Page, url_base: str) -> None: + """Foo.""" + # page.goto(urljoin(url_base, "community/practices/merge.html")) + # expect(page.get_by_label("Breadcrumb").get_by_role("list")).to_contain_text("Merge and review policy") + page.set_viewport_size({"width": 1440, "height": 720}) + page.goto(urljoin(url_base, "community/topics/config.html")) + expect(page.get_by_label("Breadcrumb").get_by_role("list")).to_contain_text( + "Update Sphinx configuration during the build" + ) diff --git a/tests/test_playwright.py b/tests/test_playwright.py new file mode 100644 index 000000000..d327dc8c0 --- /dev/null +++ b/tests/test_playwright.py @@ -0,0 +1,87 @@ +"""Build minimal test sites with sphinx_build_factory and test them with Playwright.""" + +from pathlib import Path +from urllib.parse import urljoin + +import pytest + +# Using importorskip to ensure these tests are only loaded if Playwright is installed. +playwright = pytest.importorskip("playwright") +from playwright.sync_api import Page, expect # noqa: E402 + +path_repo = Path(__file__).parents[1] +path_docs_build = path_repo / "docs" / "_build" / "html" + + +def _is_overflowing(element): + """Check if an element is being shortened via CSS due to text-overflow property. + + We can't check the rendered text because we can't easily get that; all we can get + is the text as it exists in the DOM (prior to its truncation/elision). Thus we must + directly compare the rendered clientWidth to the scrollWidth required to display the + text. + """ + return element.evaluate("e => e.clientWidth < e.scrollWidth", element) + + +def test_version_switcher_highlighting(page: Page, url_base: str) -> None: + """In sidebar and topbar - version switcher should apply highlight color to currently selected version.""" + page.goto(url=url_base) + # no need to include_hidden here ↓↓↓, we just need to get the active version name + button = page.get_by_role("button").filter(has_text="dev") + active_version_name = button.get_attribute("data-active-version-name") + # here we do include_hidden, so sidebar & topbar menus should each have a matching entry: + entries = page.get_by_role("option", include_hidden=True).filter( + has_text=active_version_name + ) + assert entries.count() == 2 + # make sure they're highlighted + for entry in entries.all(): + light_mode = "rgb(10, 125, 145)" # pst-color-primary + # dark_mode = "rgb(63, 177, 197)" + expect(entry).to_have_css("color", light_mode) + + +def test_breadcrumb_expansion(page: Page, url_base: str) -> None: + """Test breadcrumb text-overflow.""" + # wide viewport width → no truncation + page.set_viewport_size({"width": 1440, "height": 720}) + page.goto(urljoin(url_base, "community/topics/config.html")) + expect(page.get_by_label("Breadcrumb").get_by_role("list")).to_contain_text( + "Update Sphinx configuration during the build" + ) + el = page.get_by_text("Update Sphinx configuration during the build").nth(1) + expect(el).to_have_css("overflow-x", "hidden") + expect(el).to_have_css("text-overflow", "ellipsis") + assert not _is_overflowing(el) + # narrow viewport width → truncation + page.set_viewport_size({"width": 150, "height": 720}) + assert _is_overflowing(el) + + +def test_breadcrumbs_everywhere( + sphinx_build_factory, page: Page, url_base: str +) -> None: + """Test building the base html template and config.""" + sphinx_build = sphinx_build_factory("breadcrumbs") + + # Basic build with defaults + sphinx_build.build() + assert (sphinx_build.outdir / "index.html").exists(), sphinx_build.outdir.glob("*") + symlink_path = path_docs_build / "playwright_tests" / "breadcrumbs" + symlink_path.parent.mkdir(exist_ok=True) + try: + symlink_path.symlink_to(sphinx_build.outdir, True) + page.goto( + urljoin(url_base, "playwright_tests/breadcrumbs/hansel/gretel/house.html") + ) + # sidebar should overflow + text = "In the oven with my sister, so hot right now. Soooo. Hotttt." + el = page.locator("#main-content").get_by_text(text).last + assert _is_overflowing(el) + # footer containers will never trigger ellipsis overflow because... their min-width is content? TODO + el = page.locator(".footer-items__center > .footer-item") + assert not _is_overflowing(el) + finally: + symlink_path.unlink() + symlink_path.parent.rmdir()