Repository navigation
Add link-new-tab flag to card / grid-item-card directives - #298
Open
dipinknair wants to merge 12 commits into
Open
dipinknair wants to merge 12 commits into
dipinknair wants to merge 12 commits into
Conversation
Author
|
@chrisjsewell not sure what to do with this error :( |
82 | @classmethod
83 | def create_card( # noqa: PLR0915
| ^^^^^^^^^^^
84 | cls, inst: SphinxDirective, arguments: list | None, options: dict
85 | ) -> nodes.Node: |
chrisjsewell
added a commit
that referenced
this pull request
Nov 9, 2026
…310) ## What and why MyST-Parser master (723cffc, the coming 5.2.0) reworks heading anchors (executablebooks/MyST-Parser#1158): for a heading with an explicit `(target)=`, the target becomes the section's first (primary) id. That breaks three sphinx-design tests, although sphinx-design's own output is unchanged: - `test_snippets.py::test_snippets_myst[html-card-link]` and `test_snippets_myst_post[html-card-link]`: the section's `ids` flip from `clickable-cards cards-clickable` to `cards-clickable clickable-cards`. The RST and MyST snippets share one fixture, and docutils keeps the old order, so no single fixture fits both. - `test_badge_tooltips.py::test_ref_badge_tooltip_survives_resolution[html-myst]`: it counted every `href="#my-target"` in the page (4 != 2), because the theme's headerlink and sidebar TOC now also point at `#my-target`. This PR changes tests only: 1. **Badge test:** count only the anchors with the `sd-badge` class that link to `#my-target`, which is what the assertion is about. 2. **Snippet tests:** sort each element's `ids` before serialising, in all four snippet tests (RST and MyST, pre and post transforms), and regenerate the two `card-link` fixtures. One line changes in each. The full set of ids is still compared; only their order is ignored. No other fixture changes. Landing this before the MyST-Parser 5.2.0 tag keeps sphinx-design's CI green when 5.2.0 is released (the `testing` extra allows `myst-parser>=4,<6`). ## How it was tested Python 3.13, Sphinx 9.2.0, with `pytest tests`: | Environment | `main` (before) | This branch | |---|---|---| | myst-parser 5.1.0 from PyPI, docutils 0.22.4 | 272 passed, 1 skipped | 272 passed, 1 skipped | | myst-parser master 723cffc (`git+https://github.com/executablebooks/MyST-Parser@master`), docutils 0.23 | **3 failed**, 269 passed, 1 skipped | 272 passed, 1 skipped | | `testing-no-myst` (Python 3.11, Sphinx 9.0.4) | not run | 182 passed, 91 skipped | The 5.1.0 environment was checked to be the real release (its metadata requires `docutils<0.23`; master's `__version__` is also 5.1.0). The sorting is needed. With it disabled and the new fixtures kept, `tests/test_snippets.py` fails on both versions: on 5.1.0 all four `card-link` tests fail (2 RST, 2 MyST), and on master the 2 RST `card-link` tests fail. `pre-commit run --all-files` passes. ## Notes for the reviewer - `sort_ids` is a local helper in `tests/test_snippets.py`. It is deliberately not part of the semi-public `sphinx_design.testing.normalize_doctree_xml`, so downstream extensions that use that helper do not get churn in their own fixtures. - #298 adds a card to the `card-link` snippets, so its `card-link` fixtures must be regenerated. Doing that on top of this PR records sorted ids, which do not depend on the MyST version.
Moves the `link` block of `CardDirective.create_card` into `CardDirective._create_link`, which fixes ruff's PLR0912 (too many branches) and makes the `# noqa: PLR0915` unnecessary. Also drops the parse-time `target`/`rel` on URL links: the `doctree-resolved` handler already sets them for every link type, so it is now the single mechanism.
Registers `sd_new_tab` with the test XML normalizer's boolean
attributes, so the fixtures read `sd_new_tab="True"` on every docutils
version (docutils 0.22 serializes it as "1"), and regenerates
`snippet_{pre,post}_card-link.xml` for the new snippet card.
- assert `target`/`rel` on the card's own stretched link tag, including the `rel` assertion the grid-item-card test lacked; - `rel` is only rendered by Sphinx >= 7.4, so assert it is absent below that (the declared floor is 7.2); - add a `link-type: doc` card; - add a negative test: URL and ref cards without the flag get neither `target` nor `rel`.
The new-tab handler now acts on any element marked sd_new_tab, not only the card's link container, and the button directives mark their paragraph container when new-tab is set. This resolves the "TODO open in new tab" in _ButtonDirective.run_with_defaults. The marker sits on the container because a button-ref is a pending_xref, replaced on resolution, so target and rel are set on the resolved reference. Tests cover both button kinds (including button-ref with rich content and with expand / click-parent), buttons and links without the flag, the LaTeX and text builders, and the handler on a hand-built doctree. The button docs describe the option, and both option entries now say it has no effect outside HTML and suggest telling readers about the new tab.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Opens the card link in a new browser tab (target="_blank", rel="noreferrer noopener"). Works with all link-type values (URL and internal cross-refs).