diff --git a/CHANGELOG.md b/CHANGELOG.md index b2d4b10..f5b813a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,17 @@ ## Unreleased +- 🐛 FIX: Synced tabs stay in sync when activated by click-and-drag or keyboard, + by syncing on the radio input's `change` event rather than a label click + ({pr}`284`, {issue}`46`) +- ✨ NEW: A URL fragment that targets a tab (e.g. `page.html#tab-name`), or an + element inside a tab panel, now opens that tab (both on load and on + `hashchange`) ({pr}`284`, {issue}`239`) +- 👌 IMPROVE: Tab selections now persist across browsing sessions, via + `localStorage` instead of `sessionStorage` (a behaviour change), and the + storage key prefix is configurable with the new `sd_tabs_storage_prefix` + option (set it to an empty string to disable persistence) ({pr}`284`, + {issue}`103`) - 👌 IMPROVE: Restore keyboard focus rings on tab labels, dropdown summaries, buttons and stretched-link cards, using `:focus-visible` so they are visible for keyboard users but not shown on mouse click, thanks to {user}`gabalafou` diff --git a/sphinx_design/config.py b/sphinx_design/config.py index fb136b1..03df979 100644 --- a/sphinx_design/config.py +++ b/sphinx_design/config.py @@ -172,6 +172,16 @@ class SdConfig: "help": "Render fontawesome icons in LaTeX output", }, ) + tabs_storage_prefix: str = dc.field( + default="sphinx-design-tab-id-", + metadata={ + "validator": instance_of(str), + "help": ( + "localStorage key prefix for persisting synced tab selections " + "(an empty string disables persistence)" + ), + }, + ) def __post_init__(self) -> None: validate_fields(self) diff --git a/sphinx_design/extension.py b/sphinx_design/extension.py index d32c481..370ddc7 100644 --- a/sphinx_design/extension.py +++ b/sphinx_design/extension.py @@ -10,7 +10,7 @@ from .article_info import setup_article_info from .badges_buttons import setup_badges_and_buttons from .cards import setup_cards -from .config import setup_sd_config +from .config import get_sd_config, setup_sd_config from .dropdown import setup_dropdown from .grids import setup_grids from .icons import setup_icons @@ -79,7 +79,16 @@ def add_static_assets(app: Sphinx) -> None: return app.config.html_static_path.append(str(STATIC_DIR)) app.add_css_file("sphinx-design.min.css") - app.add_js_file("design-tabs.js") + # deliver the (configurable) tab-storage key prefix declaratively, + # as a data attribute on the script tag (read by design-tabs.js at startup) + sd_config = get_sd_config(app.env) + js_attributes: dict[str, str] = { + "data-sd-tabs-storage-prefix": sd_config.tabs_storage_prefix + } + # the ignore is because mypy checks the unpacked values against the + # explicit ``priority: int`` parameter, but they only ever land in + # ``**kwargs`` (the HTML attributes) + app.add_js_file("design-tabs.js", **js_attributes) # type: ignore[arg-type] def visit_container(self, node: nodes.Node): diff --git a/sphinx_design/static/design-tabs.js b/sphinx_design/static/design-tabs.js index b25bd6a..9aaa1c2 100644 --- a/sphinx_design/static/design-tabs.js +++ b/sphinx_design/static/design-tabs.js @@ -1,13 +1,22 @@ // @ts-check -// Extra JS capability for selected tabs to be synced -// The selection is stored in local storage so that it persists across page loads. +// Extra JS capability for selected tabs to be synced. +// The selection is stored in the browser's local storage, so that it persists +// across page loads and browsing sessions. The storage key prefix is +// configurable (and can be set to an empty string to disable persistence). /** * @type {Record} */ let sd_id_to_elements = {}; -const storageKeyPrefix = "sphinx-design-tab-id-"; + +// The storage key prefix is delivered as a `data-` attribute on this script +// tag, and must be captured here at eval time: `document.currentScript` is only +// defined while the script is first executing, not inside later callbacks such +// as `ready`. An empty string disables persistence entirely. +const storageKeyPrefix = + document.currentScript?.getAttribute("data-sd-tabs-storage-prefix") ?? + "sphinx-design-tab-id-"; /** * Create a key for a tab element. @@ -22,6 +31,61 @@ function create_key(el) { return [syncGroup, syncId, syncGroup + "--" + syncId]; } +/** + * Get the radio input associated with a tab label. + * + * Per ``TabSetHtmlTransform`` the HTML DOM order is input, label, content, so + * the input directly precedes the label; the label's ``for`` attribute also + * points at the input's id. + * + * @param {HTMLElement} label - The tab label. + * @returns {HTMLInputElement | null} - The associated radio input, if found. + */ +function get_label_input(label) { + const forId = label.getAttribute("for"); + const el = forId + ? document.getElementById(forId) + : label.previousElementSibling; + return el instanceof HTMLInputElement ? el : null; +} + +/** + * Read the stored sync id for a group (returns null if persistence is disabled). + * + * Storage access may throw (e.g. a SecurityError when the browser blocks site + * data, or ``window.localStorage`` itself being inaccessible), so failures are + * swallowed: persistence is then simply unavailable, but tab syncing and + * hash handling must keep working. + * + * @param {string} group - The sync group. + * @returns {string | null} - The stored sync id, if any. + */ +function get_stored_id(group) { + if (!storageKeyPrefix) return null; + try { + return window.localStorage.getItem(storageKeyPrefix + group); + } catch (err) { + return null; + } +} + +/** + * Persist the selected sync id for a group (no-op if persistence is disabled). + * + * See ``get_stored_id`` regarding swallowed storage failures. + * + * @param {string} group - The sync group. + * @param {string} id - The selected sync id. + */ +function set_stored_id(group, id) { + if (!storageKeyPrefix) return; + try { + window.localStorage.setItem(storageKeyPrefix + group, id); + } catch (err) { + // persistence unavailable; ignore + } +} + /** * Initialize the tab selection. * @@ -38,9 +102,13 @@ function ready() { if (data) { let [group, id, key] = data; - // add click event listener - // @ts-ignore - label.onclick = onSDLabelClick; + // Sync on the radio input's `change` event, rather than the label's + // click: `change` fires exactly once per selection regardless of the + // activation method (mouse, click+drag release, keyboard arrows, JS). + const input = get_label_input(label); + if (input) { + input.addEventListener("change", onSDInputChange); + } // store map of key to elements if (!sd_id_to_elements[key]) { @@ -50,52 +118,110 @@ function ready() { if (groups.indexOf(group) === -1) { groups.push(group); - // Check if a specific tab has been selected via URL parameter + // Check if a specific tab has been selected via URL query parameter const tabParam = new URLSearchParams(window.location.search).get( group ); if (tabParam) { - console.log( - "sphinx-design: Selecting tab id for group '" + - group + - "' from URL parameter: " + - tabParam - ); - window.sessionStorage.setItem(storageKeyPrefix + group, tabParam); + set_stored_id(group, tabParam); } } - // Check is a specific tab has been selected previously - let previousId = window.sessionStorage.getItem( - storageKeyPrefix + group - ); - if (previousId === id) { - // console.log( - // "sphinx-design: Selecting tab from session storage: " + id - // ); - // @ts-ignore - label.previousElementSibling.checked = true; + // Check if a specific tab has been selected previously + let previousId = get_stored_id(group); + if (previousId === id && input) { + // set `.checked` directly (does not re-fire `change`) + input.checked = true; } } } }); + + // Open the tab targeted by the URL fragment (on load and on later changes) + select_tab_from_hash(); + window.addEventListener("hashchange", select_tab_from_hash); } /** - * Activate other tabs with the same sync id. + * Activate the other tabs sharing the changed input's sync id. * - * @this {HTMLElement} - The element that was clicked. + * @param {Event} event - The `change` event fired by a tab's radio input. */ -function onSDLabelClick() { - let data = create_key(this); +function onSDInputChange(event) { + const input = event.currentTarget; + if (!(input instanceof HTMLInputElement) || !input.checked) return; + // the label carries the sync data, and directly follows the input + const label = input.nextElementSibling; + if (!(label instanceof HTMLElement)) return; + let data = create_key(label); if (!data) return; let [group, id, key] = data; - for (const label of sd_id_to_elements[key]) { - if (label === this) continue; - // @ts-ignore - label.previousElementSibling.checked = true; + for (const other of sd_id_to_elements[key]) { + if (other === label) continue; + const otherInput = get_label_input(other); + if (otherInput) { + // set `.checked` directly: this does NOT re-fire `change`, so the sync + // stays idempotent (no `.click()`, no event cascade between tab-sets) + otherInput.checked = true; + } } - window.sessionStorage.setItem(storageKeyPrefix + group, id); + set_stored_id(group, id); +} + +/** + * Select the tab targeted by the current URL fragment, if any. + * + * The fragment may target a `.sd-tab-label` directly (via a tab-item `:name:`), + * or an element inside a `.sd-tab-content` panel; in the latter case the + * panel's own radio input is selected. Per ``TabSetHtmlTransform`` the DOM + * order is input, label, content, so a content panel's input is its + * ``previousElementSibling.previousElementSibling``. + * + * Tab-sets can be nested inside other tabs' panels, so every enclosing + * `.sd-tab-content` ancestor is opened as well — otherwise the target would + * stay hidden inside a non-selected outer tab. + */ +function select_tab_from_hash() { + const hash = window.location.hash; + if (!hash) return; + const target = document.getElementById(hash.slice(1)); + if (!target) return; + + let opened = false; + + // the hash may target a tab label directly (via a tab-item `:name:`) + const label = target.closest(".sd-tab-label"); + if (label instanceof HTMLElement) { + const input = get_label_input(label); + if (input) { + input.checked = true; + opened = true; + } + } + + // open every enclosing panel: starting from the target (or, for a label, + // from the label itself, which sits outside its own panel), walk up the + // `.sd-tab-content` ancestors and select each panel's radio input + const start = label instanceof HTMLElement ? label : target; + for ( + let content = start.closest(".sd-tab-content"); + content; + content = content.parentElement + ? content.parentElement.closest(".sd-tab-content") + : null + ) { + const input = content.previousElementSibling?.previousElementSibling; + if (input instanceof HTMLInputElement) { + input.checked = true; + opened = true; + } + } + + if (!opened) return; + + // re-scroll the target into view, since panel visibility (and hence the + // page layout) has just changed + target.scrollIntoView(); } document.addEventListener("DOMContentLoaded", ready, false); diff --git a/tests/test_misc.py b/tests/test_misc.py index 5e83169..19dec9b 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -507,6 +507,7 @@ def test_button_i18n_translated(sphinx_builder): INVALID_CONFIG_VALUES = { "custom_directives": (["not", "a", "dict"], "must be a dictionary"), "fontawesome_latex": ("not-a-bool", "must be of type"), + "tabs_storage_prefix": (123, "must be of type"), } """An invalidly typed value (and expected warning) for every ``SdConfig`` field.""" @@ -609,6 +610,7 @@ def test_config_toml_round_trip(): """ toml_str = """\ fontawesome_latex = true + tabs_storage_prefix = "sphinx-design-tab-id-" [custom_directives.dropdown-syntax] inherit = "dropdown" @@ -624,6 +626,7 @@ def test_config_toml_round_trip(): ) config = SdConfig(**data) assert config.fontawesome_latex is True + assert config.tabs_storage_prefix == "sphinx-design-tab-id-" assert config.custom_directives == { "dropdown-syntax": { "inherit": "dropdown", diff --git a/tests/test_tabs_js.py b/tests/test_tabs_js.py new file mode 100644 index 0000000..b5bcf4e --- /dev/null +++ b/tests/test_tabs_js.py @@ -0,0 +1,200 @@ +"""Tests for the tabs JavaScript integration (``design-tabs.js``). + +Covers the Python side of the tab-syncing behaviour: the configurable +``sd_tabs_storage_prefix`` and its delivery to the browser as a ``data-`` +attribute on the script tag, plus the HTML DOM ordering that the JavaScript +relies on (input, label, content siblings). + +The JavaScript behaviour itself has no automated harness here; the DOM-order +assertions in :func:`test_tab_dom_order` pin the structural assumptions that +``design-tabs.js`` makes (``label.previousElementSibling`` is the input, +``content.previousElementSibling.previousElementSibling`` is the input, and a +``:name:`` id lands on the label). + +These tests deliberately use plain reStructuredText and only the +``sphinx_design`` extension, so that they also run under ``py311-no-myst`` (the +one exception, :func:`test_tab_dom_order`, is parametrized and guards its myst +case with the shared ``MYST_PARAM`` skip marker). +""" + +import re + +import pytest + +from sphinx_design.config import SdConfig + +try: + import myst_parser # noqa: F401 + + MYST_INSTALLED = True +except ImportError: + MYST_INSTALLED = False + +MYST_PARAM = pytest.param( + "myst", + marks=pytest.mark.skipif(not MYST_INSTALLED, reason="myst-parser not installed"), +) + +# Keep the config myst-free so these tests run without myst-parser installed. +SD_CONF = {"extensions": ["sphinx_design"]} + +INDEX_RST = """ +Test Document +============= + +Some content. +""" + + +def _script_tag(builder) -> str: + """Build the project and return the ``design-tabs.js`` script tag.""" + builder.src_path.joinpath("index.rst").write_text(INDEX_RST, encoding="utf8") + builder.build() + index_html = builder.out_path.joinpath("index.html").read_text(encoding="utf8") + match = re.search(r"]*design-tabs\.js[^>]*>", index_html) + assert match, "no design-tabs.js script tag found in the built HTML" + return match.group(0) + + +def test_script_tag_default_prefix(sphinx_builder): + """By default the script tag carries the default storage-prefix attribute.""" + builder = sphinx_builder(conf_kwargs=SD_CONF) + tag = _script_tag(builder) + assert 'data-sd-tabs-storage-prefix="sphinx-design-tab-id-"' in tag + # the native cache-busting suffix must not be lost by adding the attribute + assert "design-tabs.js?v=" in tag + + +def test_script_tag_custom_prefix(sphinx_builder): + """A customized ``sd_tabs_storage_prefix`` is forwarded to the script tag.""" + builder = sphinx_builder( + conf_kwargs={**SD_CONF, "sd_tabs_storage_prefix": "custom-"} + ) + tag = _script_tag(builder) + assert 'data-sd-tabs-storage-prefix="custom-"' in tag + + +def test_script_tag_empty_prefix(sphinx_builder): + """An empty ``sd_tabs_storage_prefix`` renders an empty attribute value. + + (In the browser this disables persistence, since ``getAttribute`` returns + the empty string rather than ``null``.) + """ + builder = sphinx_builder(conf_kwargs={**SD_CONF, "sd_tabs_storage_prefix": ""}) + tag = _script_tag(builder) + assert 'data-sd-tabs-storage-prefix=""' in tag + + +def test_config_tabs_storage_prefix_invalid_type(): + """Directly instantiating ``SdConfig`` with a non-string prefix should raise.""" + with pytest.raises(TypeError, match="'tabs_storage_prefix' must be of type"): + SdConfig(tabs_storage_prefix=123) + + +TAB_SET = { + "rst": """ +Test +==== + +.. tab-set:: + + .. tab-item:: Label A + :name: my-tab-a + :sync: a + + Content A. + + .. tab-item:: Label B + :sync: b + + Content B. + +.. tab-set:: + + .. tab-item:: Label A2 + :sync: a + + Second A. + + .. tab-item:: Label B2 + :sync: b + + Second B. +""", + "myst": """ +# Test + +````{tab-set} +```{tab-item} Label A +:name: my-tab-a +:sync: a + +Content A. +``` +```{tab-item} Label B +:sync: b + +Content B. +``` +```` + +````{tab-set} +```{tab-item} Label A2 +:sync: a + +Second A. +``` +```{tab-item} Label B2 +:sync: b + +Second B. +``` +```` +""", +} + + +@pytest.mark.parametrize("fmt", ["rst", MYST_PARAM]) +def test_tab_dom_order(fmt, sphinx_builder): + """The built HTML must place input, label and content as siblings in that + order, since ``design-tabs.js`` navigates between them by DOM position: + + - ``label.previousElementSibling`` (and ``for``) must resolve to the input; + - ``content.previousElementSibling.previousElementSibling`` must be the + input (for hashes targeting content inside a hidden panel); + - a ``:name:`` id must land on the label (so ``#name`` selects the tab). + """ + if fmt == "rst": + builder = sphinx_builder(conf_kwargs=SD_CONF) + builder.src_path.joinpath("index.rst").write_text( + TAB_SET["rst"], encoding="utf8" + ) + else: + builder = sphinx_builder() + builder.src_path.joinpath("index.md").write_text( + TAB_SET["myst"], encoding="utf8" + ) + builder.build() # asserts no warnings + html = builder.out_path.joinpath("index.html").read_text(encoding="utf8") + + # the first three component tags, in document (== sibling) order + tags = re.findall( + r']*\bid="sd-tab-item[^>]*>' + r'|]*\bclass="sd-tab-label[^>]*>' + r'|]*\bclass="sd-tab-content', + html, + ) + assert tags[0].startswith("]*\bclass="sd-tab-label[^>]*\bid="my-tab-a"[^>]*>', html + ) + assert named_label, "the :name: id should be rendered on the tab label"