Skip to content

Commit

Permalink
use ellipsis to truncate breadcrumbs, and test with playwright (#1861)
Browse files Browse the repository at this point in the history
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 <[email protected]>
Co-authored-by: Madicken Munk <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: gabalafou <[email protected]>
  • Loading branch information
5 people committed Jun 27, 2024
1 parent 1c338cb commit 11e0411
Show file tree
Hide file tree
Showing 14 changed files with 259 additions and 85 deletions.
12 changes: 12 additions & 0 deletions src/pydata_sphinx_theme/assets/styles/components/_breadcrumbs.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down
Original file line number Diff line number Diff line change
@@ -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": '<i class="fa-solid fa-ellipsis"></i>'}, parents[-1]] %}
{% endif %}

{#- Hide breadcrumbs on the home page #}
{% if title and pagename != root_doc %}
Expand All @@ -25,7 +18,7 @@
<li class="breadcrumb-item">{{ doc.title }}</li>
{% endif %}
{%- endfor %}
<li class="breadcrumb-item active" aria-current="page">{{ title|truncate(15, False) }}</li>
<li class="breadcrumb-item active" aria-current="page"><span class="ellipsis">{{ title }}</span></li>
</ul>
</nav>
{% endif %}
Expand Down
50 changes: 48 additions & 2 deletions tests/conftest.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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 ------------------------------------------------------------

Expand Down Expand Up @@ -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()
20 changes: 20 additions & 0 deletions tests/sites/breadcrumbs/Makefile
Original file line number Diff line number Diff line change
@@ -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)
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
32 changes: 32 additions & 0 deletions tests/sites/breadcrumbs/conf.py
Original file line number Diff line number Diff line change
@@ -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"],
}
2 changes: 2 additions & 0 deletions tests/sites/breadcrumbs/hansel/gretel/house.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
In the oven with my sister, so hot right now. Soooo. Hotttt.
============================================================
17 changes: 17 additions & 0 deletions tests/sites/breadcrumbs/index.rst
Original file line number Diff line number Diff line change
@@ -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
````````````````
14 changes: 14 additions & 0 deletions tests/sites/breadcrumbs/missing_url.json
Original file line number Diff line number Diff line change
@@ -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"
}
]
15 changes: 15 additions & 0 deletions tests/sites/breadcrumbs/switcher.json
Original file line number Diff line number Diff line change
@@ -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/"
}
]
77 changes: 11 additions & 66 deletions tests/test_a11y.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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."""
Expand Down Expand Up @@ -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"
)
Loading

0 comments on commit 11e0411

Please sign in to comment.