Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use ellipsis to truncate breadcrumbs, and test with playwright #1861

Merged
merged 16 commits into from
Jun 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 {
Comment on lines +19 to +20
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now the text-overflow: ellipsis rule applies to all the breadcrumbs not just the last/current one

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);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing the height rule should prevent the issue in #1451, in which the breadcrumbs overlap the article title

}

.header-article-items__start,
.header-article-items__end {
display: flex;
align-items: start;
gap: 0.5rem;
Comment on lines -14 to -15
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These rules have no meaning after display: flex was deleted, so I deleted them.

}

.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
Loading