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

Maint: do not use document.write #1921

Merged
merged 5 commits into from
Aug 6, 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
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,17 @@
{# use alt_text if given. If not, only add alt text if no additional branding text given. #}
{% set alt = theme_logo.get("alt_text", "" if theme_logo.get("text") else "%s - Home" % docstitle) %}
{% if is_logo %}
{# Theme switching is only available when JavaScript is enabled.
# Thus we should add the extra image using JavaScript, defaulting
# depending on the value of default_mode; and light if unset.
{#
Theme switching is only available when JavaScript is enabled.
We thus adds elements that should be present only when javscipt is
enabled with a pst-js-only class
#}
{% if default_mode is undefined or default_mode == "auto" %}
{% set default_mode = "light" %}
{% endif %}
{% set js_mode = "light" if default_mode == "dark" else "dark" %}
<img src="{{ theme_logo['image_relative'][default_mode] }}" class="logo__image only-{{ default_mode }}" alt="{{ alt }}"/>
<script>document.write(`<img src="{{ theme_logo['image_relative'][js_mode] }}" class="logo__image only-{{ js_mode }}" alt="{{ alt }}"/>`);</script>
<img src="{{ theme_logo['image_relative'][js_mode] }}" class="logo__image only-{{ js_mode }} pst-js-only" alt="{{ alt }}"/>
{% endif %}
{% if not is_logo or theme_logo.get("text") %}
<p class="title logo__title">{{ theme_logo.get("text") or docstitle }}</p>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
{# Displays a search field image that opens a search overlay when clicked. #}
{# As this function will only work when JavaScript is enabled, we add it through JavaScript. #}
<script>
document.write(`
<button class="btn search-button-field search-button__button" title="{{ _('Search') }}" aria-label="{{ _('Search') }}" data-bs-placement="bottom" data-bs-toggle="tooltip">
<i class="fa-solid fa-magnifying-glass"></i>
<span class="search-button__default-text">{{ _('Search') }}</span>
<span class="search-button__kbd-shortcut"><kbd class="kbd-shortcut__modifier">Ctrl</kbd>+<kbd class="kbd-shortcut__modifier">K</kbd></span>
</button>
`);
</script>
{#
As this function will only work when JavaScript is enabled,
we add a class that will hide it if js is disable.
#}
<button class="btn search-button-field search-button__button pst-js-only" title="{{ _('Search') }}" aria-label="{{ _('Search') }}" data-bs-placement="bottom" data-bs-toggle="tooltip">
<i class="fa-solid fa-magnifying-glass"></i>
<span class="search-button__default-text">{{ _('Search') }}</span>
<span class="search-button__kbd-shortcut"><kbd class="kbd-shortcut__modifier">Ctrl</kbd>+<kbd class="kbd-shortcut__modifier">K</kbd></span>
</button>
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
{# Displays a magnifying glass icon that opens a search overlay when clicked. #}
{# As this function will only work when JavaScript is enabled, we add it through JavaScript. #}
<script>
document.write(`
<button class="btn btn-sm pst-navbar-icon search-button search-button__button" title="{{ _('Search') }}" aria-label="{{ _('Search') }}" data-bs-placement="bottom" data-bs-toggle="tooltip">
{# As this function will only work when JavaScript is enabled, we hide it with pst-js-only #}
<button class="btn btn-sm pst-navbar-icon search-button search-button__button pst-js-only" title="{{ _('Search') }}" aria-label="{{ _('Search') }}" data-bs-placement="bottom" data-bs-toggle="tooltip">
<i class="fa-solid fa-magnifying-glass fa-lg"></i>
</button>
</button>
`);
</script>
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
{# Displays an icon to switch between light mode, dark mode, and auto (use browser's setting). #}
{# As the theme switcher will only work when JavaScript is enabled, we add it through JavaScript. #}
<script>
document.write(`
<button class="btn btn-sm nav-link pst-navbar-icon theme-switch-button" title="{{ _('light/dark') }}" aria-label="{{ _('light/dark') }}" data-bs-placement="bottom" data-bs-toggle="tooltip">
<i class="theme-switch fa-solid fa-sun fa-lg" data-mode="light"></i>
<i class="theme-switch fa-solid fa-moon fa-lg" data-mode="dark"></i>
<i class="theme-switch fa-solid fa-circle-half-stroke fa-lg" data-mode="auto"></i>
</button>
`);
</script>
{# As the theme switcher will only work when JavaScript is enabled, we hide it with `pst-js-only`. #}
<button class="btn btn-sm nav-link pst-navbar-icon theme-switch-button pst-js-only" title="{{ _('light/dark') }}" aria-label="{{ _('light/dark') }}" data-bs-placement="bottom" data-bs-toggle="tooltip">
<i class="theme-switch fa-solid fa-sun fa-lg" data-mode="light"></i>
<i class="theme-switch fa-solid fa-moon fa-lg" data-mode="dark"></i>
<i class="theme-switch fa-solid fa-circle-half-stroke fa-lg" data-mode="auto"></i>
</button>
Original file line number Diff line number Diff line change
@@ -1,26 +1,22 @@
{# Displays a dropdown box for switching among different versions of your documentation. #}
{%- set button_id = unique_html_id("pst-version-switcher-button") -%}
{%- set dropdown_id = unique_html_id("pst-version-switcher-list") -%}
{# As the version switcher will only work when JavaScript is enabled, we add it through JavaScript. #}
<script>
document.write(`
<div class="version-switcher__container dropdown">
<button id="{{ button_id }}"
type="button"
class="version-switcher__button btn btn-sm dropdown-toggle"
data-bs-toggle="dropdown"
aria-haspopup="listbox"
aria-controls="{{ dropdown_id }}"
aria-label="Version switcher list"
>
Choose version <!-- this text may get changed later by javascript -->
<span class="caret"></span>
</button>
<div id="{{ dropdown_id }}"
class="version-switcher__menu dropdown-menu list-group-flush py-0"
role="listbox" aria-labelledby="{{ button_id }}">
<!-- dropdown will be populated by javascript on page load -->
</div>
{# As the version switcher will only work when JavaScript is enabled, we hide it with pst-js-only #}
<div class="version-switcher__container dropdown pst-js-only">
<button id="{{ button_id }}"
type="button"
class="version-switcher__button btn btn-sm dropdown-toggle"
data-bs-toggle="dropdown"
aria-haspopup="listbox"
aria-controls="{{ dropdown_id }}"
aria-label="Version switcher list"
>
Choose version <!-- this text may get changed later by javascript -->
<span class="caret"></span>
</button>
<div id="{{ dropdown_id }}"
class="version-switcher__menu dropdown-menu list-group-flush py-0"
role="listbox" aria-labelledby="{{ button_id }}">
<!-- dropdown will be populated by javascript on page load -->
</div>
`);
</script>
</div>
9 changes: 9 additions & 0 deletions src/pydata_sphinx_theme/theme/pydata_sphinx_theme/layout.html
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,15 @@
document.documentElement.dataset.mode = localStorage.getItem("mode") || "{{ default_mode }}";
document.documentElement.dataset.theme = localStorage.getItem("theme") || "{{ default_mode }}";
</script>
<!--
this give us a css class that will be invisible only if js is disabled
-->
<noscript>
<style>
.pst-js-only { display: none !important; }

</style>
</noscript>
{{ _webpack.head_pre_assets() }}
{{ _webpack.head_pre_icons() }}
{{- css() }}
Expand Down
17 changes: 7 additions & 10 deletions tests/test_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ def test_primary_logo_is_light_when_no_default_mode(sphinx_build_factory) -> Non
index_html = sphinx_build.html_tree("index.html")
navbar_brand = index_html.select(".navbar-brand")[0]
assert navbar_brand.find("img", class_="only-light") is not None
assert navbar_brand.find("script", string=re.compile("only-dark")) is not None
assert navbar_brand.find("img", class_="only-dark") is not None


def test_primary_logo_is_light_when_default_mode_is_set_to_auto(
Expand All @@ -199,7 +199,7 @@ def test_primary_logo_is_light_when_default_mode_is_set_to_auto(
index_html = sphinx_build.html_tree("index.html")
navbar_brand = index_html.select(".navbar-brand")[0]
assert navbar_brand.find("img", class_="only-light") is not None
assert navbar_brand.find("script", string=re.compile("only-dark")) is not None
assert navbar_brand.find("img", class_="only-dark") is not None


def test_primary_logo_is_light_when_default_mode_is_light(sphinx_build_factory) -> None:
Expand All @@ -212,7 +212,7 @@ def test_primary_logo_is_light_when_default_mode_is_light(sphinx_build_factory)
index_html = sphinx_build.html_tree("index.html")
navbar_brand = index_html.select(".navbar-brand")[0]
assert navbar_brand.find("img", class_="only-light") is not None
assert navbar_brand.find("script", string=re.compile("only-dark")) is not None
assert navbar_brand.find("img", class_="only-dark") is not None


def test_primary_logo_is_dark_when_default_mode_is_dark(sphinx_build_factory) -> None:
Expand All @@ -225,7 +225,7 @@ def test_primary_logo_is_dark_when_default_mode_is_dark(sphinx_build_factory) ->
index_html = sphinx_build.html_tree("index.html")
navbar_brand = index_html.select(".navbar-brand")[0]
assert navbar_brand.find("img", class_="only-dark") is not None
assert navbar_brand.find("script", string=re.compile("only-light")) is not None
assert navbar_brand.find("img", class_="only-light") is not None


def test_logo_missing_image(sphinx_build_factory) -> None:
Expand Down Expand Up @@ -805,8 +805,9 @@ def test_version_switcher_error_states(
if url == "switcher.json": # this should work
index = sphinx_build.html_tree("index.html")
switcher = index.select(".navbar-header-items")[0].find(
"script", string=re.compile(".version-switcher__container")
"div", class_="version-switcher__container"
)
assert switcher is not None
file_regression.check(
switcher.prettify(), basename="navbar_switcher", extension=".html"
)
Expand All @@ -826,11 +827,7 @@ def test_version_switcher_error_states(
def test_theme_switcher(sphinx_build_factory, file_regression) -> None:
"""Regression test for the theme switcher button."""
sphinx_build = sphinx_build_factory("base").build()
switcher = (
sphinx_build.html_tree("index.html")
.find(string=re.compile("theme-switch-button"))
.find_parent("script")
)
switcher = sphinx_build.html_tree("index.html").find(class_="theme-switch-button")
file_regression.check(
switcher.prettify(), basename="navbar_theme", extension=".html"
)
Expand Down
33 changes: 11 additions & 22 deletions tests/test_build/navbar_switcher.html
Original file line number Diff line number Diff line change
@@ -1,22 +1,11 @@
<script>
document.write(`
<div class="version-switcher__container dropdown">
<button id="pst-version-switcher-button-2"
type="button"
class="version-switcher__button btn btn-sm dropdown-toggle"
data-bs-toggle="dropdown"
aria-haspopup="listbox"
aria-controls="pst-version-switcher-list-2"
aria-label="Version switcher list"
>
Choose version <!-- this text may get changed later by javascript -->
<span class="caret"></span>
</button>
<div id="pst-version-switcher-list-2"
class="version-switcher__menu dropdown-menu list-group-flush py-0"
role="listbox" aria-labelledby="pst-version-switcher-button-2">
<!-- dropdown will be populated by javascript on page load -->
</div>
</div>
`);
</script>
<div class="version-switcher__container dropdown pst-js-only">
<button aria-controls="pst-version-switcher-list-2" aria-haspopup="listbox" aria-label="Version switcher list" class="version-switcher__button btn btn-sm dropdown-toggle" data-bs-toggle="dropdown" id="pst-version-switcher-button-2" type="button">
Choose version
<!-- this text may get changed later by javascript -->
<span class="caret">
</span>
</button>
<div aria-labelledby="pst-version-switcher-button-2" class="version-switcher__menu dropdown-menu list-group-flush py-0" id="pst-version-switcher-list-2" role="listbox">
<!-- dropdown will be populated by javascript on page load -->
</div>
</div>
17 changes: 8 additions & 9 deletions tests/test_build/navbar_theme.html
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
<script>
document.write(`
<button class="btn btn-sm nav-link pst-navbar-icon theme-switch-button" title="light/dark" aria-label="light/dark" data-bs-placement="bottom" data-bs-toggle="tooltip">
<i class="theme-switch fa-solid fa-sun fa-lg" data-mode="light"></i>
<i class="theme-switch fa-solid fa-moon fa-lg" data-mode="dark"></i>
<i class="theme-switch fa-solid fa-circle-half-stroke fa-lg" data-mode="auto"></i>
</button>
`);
</script>
<button aria-label="light/dark" class="btn btn-sm nav-link pst-navbar-icon theme-switch-button pst-js-only" data-bs-placement="bottom" data-bs-toggle="tooltip" title="light/dark">
<i class="theme-switch fa-solid fa-sun fa-lg" data-mode="light">
</i>
<i class="theme-switch fa-solid fa-moon fa-lg" data-mode="dark">
</i>
<i class="theme-switch fa-solid fa-circle-half-stroke fa-lg" data-mode="auto">
</i>
</button>
17 changes: 8 additions & 9 deletions tests/test_build/sidebar_subpage.html
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,14 @@
</div>
<div class="sidebar-header-items__end">
<div class="navbar-item">
<script>
document.write(`
<button class="btn btn-sm nav-link pst-navbar-icon theme-switch-button" title="light/dark" aria-label="light/dark" data-bs-placement="bottom" data-bs-toggle="tooltip">
<i class="theme-switch fa-solid fa-sun fa-lg" data-mode="light"></i>
<i class="theme-switch fa-solid fa-moon fa-lg" data-mode="dark"></i>
<i class="theme-switch fa-solid fa-circle-half-stroke fa-lg" data-mode="auto"></i>
</button>
`);
</script>
<button aria-label="light/dark" class="btn btn-sm nav-link pst-navbar-icon theme-switch-button pst-js-only" data-bs-placement="bottom" data-bs-toggle="tooltip" title="light/dark">
<i class="theme-switch fa-solid fa-sun fa-lg" data-mode="light">
</i>
<i class="theme-switch fa-solid fa-moon fa-lg" data-mode="dark">
</i>
<i class="theme-switch fa-solid fa-circle-half-stroke fa-lg" data-mode="auto">
</i>
</button>
</div>
</div>
</div>
Expand Down
Loading