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.

Sign up for GitHub

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

Jump to bottom

Degrade gracefully when JavaScript is disabled #1146

Merged
merged 1 commit into from
Feb 9, 2023
Merged

Degrade gracefully when JavaScript is disabled #1146

merged 1 commit into from
Feb 9, 2023

Conversation

lunar-debian
Copy link
Contributor

Support for light/dark themes has been implemented using a data-theme attribute set on the <html/> tag. As this attribute is set using JavaScript, this meant that it was left unset when visitors had JavaScript disabled. This resulted in several important CSS rules not being matched and a “broken feeling” due to wrong colors, and logo or images shown twice.

To better support browsers with JavaScript disabled:

  1. Add the same CSS rules as for the light theme when the data-theme attribute is not set. This creates a safe fallback in every situation.
  2. If default_mode is set, write its value to the data-theme attribute when writing the HTML files. This enables theme users to present their preferred mode to visitors with JavaScript disabled.
  3. Use JavaScript to add the search and theme switcher buttons as they require JavaScript to work. This avoid unusable UI elements to be shown to visitors with JavaScript disabled.
  4. Use JavaScript to write the logo for the “other theme”, depending on the value of default_mode, defaulting to “light” if unset.

While this last change might seem redundant considering the other ones, it does make the resulting pages better for search engines and text browsers.

Closes: #1145

@lunar-debian lunar-debian mentioned this pull request Feb 2, 2023
Closed
12rambau
12rambau reviewed Feb 2, 2023
View reviewed changes
Copy link
Collaborator

@12rambau 12rambau left a comment

Choose a reason for hiding this comment

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

thanks a lot for this PR. I approved the CI to see how it behaves on our side.
$I have some implementation questions as I'm not very familiar with the situation you describe.

from the general point of view, what would you think if default_theme is set to "light" for all documentation by default (https://github.com/pydata/pydata-sphinx-theme/blob/main/src/pydata_sphinx_theme/theme/pydata_sphinx_theme/theme.conf)

src/pydata_sphinx_theme/theme/pydata_sphinx_theme/components/search-button.html Show resolved Hide resolved
src/pydata_sphinx_theme/theme/pydata_sphinx_theme/layout.html Show resolved Hide resolved
src/pydata_sphinx_theme/theme/pydata_sphinx_theme/components/navbar-logo.html Outdated Show resolved Hide resolved
drammock
drammock approved these changes Feb 2, 2023
View reviewed changes
Copy link
Collaborator

@drammock drammock left a comment

Choose a reason for hiding this comment

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

this LGTM although I'm maybe not the best qualified to review :)

@12rambau 12rambau mentioned this pull request Feb 2, 2023
Merged
@lunar-debian lunar-debian force-pushed the degrade-gracefully-when-javascript-is-disabled branch from 659aede to 51a975c Compare February 6, 2023 15:35
@lunar-debian
Copy link
Contributor Author

Rebased and improved navbar-logo.html. Updated and added tests.

12rambau
12rambau requested changes Feb 7, 2023
View reviewed changes
Copy link
Collaborator

@12rambau 12rambau left a comment

Choose a reason for hiding this comment

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

I checked the pre-build documentation and there are other components that are failing due to JS absence:

  • the disclaimer at the top of the site
  • the version switcher

Could you update them as well?

Degrade gracefully when JavaScript is disabled
4ed402f 
Support for light/dark themes has been implemented using a `data-theme`
attribute set on the `<html/>` tag. As this attribute is set using
JavaScript, this meant that it was left unset when visitors had
JavaScript disabled. This resulted in several important CSS rules not
being matched and a “broken feeling” due to wrong colors, and logo or
images shown twice.

To better support browsers with JavaScript disabled:

1. Add the same CSS rules as for the light theme when the `data-theme`
   attribute is not set. This creates a safe fallback in every
   situation.
2. If `default_mode` is set, write its value to the `data-theme` attribute
   when writing the HTML files. This enables theme users to present
   their preferred mode to visitors with JavaScript disabled.
3. Use JavaScript to add the search, theme switcher and version switcher
   interface as they require JavaScript to work. This avoid unusable UI
   elements to be shown to visitors with JavaScript disabled.
4. Use JavaScript to write the logo for the “other theme”, depending on
   the value of `default_mode`, defaulting to “light” if unset.
5. Use JavaScript to write the announcement block to the HTML if the
   announcement is a remote URL.

While this last change might seem redundant considering the other ones,
it does make the resulting pages better for search engines and text
browsers.

Closes: pydata#1145
@lunar-debian lunar-debian force-pushed the degrade-gracefully-when-javascript-is-disabled branch from 51a975c to 4ed402f Compare February 8, 2023 10:55
@lunar-debian
Copy link
Contributor Author

Updated to degrade gracefully the version switcher and announcement block.

12rambau
12rambau approved these changes Feb 8, 2023
View reviewed changes
Copy link
Collaborator

@12rambau 12rambau left a comment

Choose a reason for hiding this comment

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

Very nice improvement, thanks a lot @lunar-debian !

@drammock, @jarrodmillman, @choldgraf should we wait for 0.13 final release before merging this one (as rc1 is already out) ?

@drammock
Copy link
Collaborator

drammock commented Feb 9, 2023

I would say go ahead and include this in 0.13 if you're satisfied with it @12rambau

@12rambau 12rambau merged commit fb03af0 into pydata:main Feb 9, 2023
@drammock drammock mentioned this pull request Feb 14, 2023
Closed
12rambau added a commit to 12rambau/pydata-sphinx-theme that referenced this pull request Mar 2, 2023
@12rambau @choldgraf
@drammock @nicoa @dependabot @bheberlein @trallard @lunar-debian @jeanas @JoerivanEngelen @jpmckinney @jayaddison @vkbo
update branch (#7)
3e02d4d 
* Fix extra whitespace in sidebars (pydata#1115)

* Fix extra whitespace in sidebars

* Searchbox

* Update src/pydata_sphinx_theme/__init__.py

Co-authored-by: Daniel McCloy <dan@mccloy.info>

* make test pass

* Fix template filter to remove empty files

* ABlog in template test

* Move clear search button to primary sidebar

* Move search clear button to top of article

Co-authored-by: Daniel McCloy <dan@mccloy.info>

* FIX: Use logo_url instead deprecated logo in theme (pydata#1094) (pydata#1097)

resolves pydata#1094

* ENH/MAINT: avoid overwriting the HtmlTranslator (pydata#1105)

Co-authored-by: Chris Holdgraf <choldgraf@gmail.com>
Fix pydata#143
Fix pydata#94

* fix: align sidebar sliding with the buttons (pydata#1123)

* fix: aline the sidebar sliding with the buttons

* build: force test to run on all platform
if one platform is failing we cannot see if it's platform related as the other were closed

* fix: use correct path for documentation logo

* MAINT: Improve font sizing (pydata#1129)

Fix pydata#1001

* MAINT: Refactor workflows to reduce test dependencies (pydata#1136)

* MAINT: update prerelease workflow (pydata#1140)

* ABlog: Updates for new HTML structure (pydata#1118)

* ABlog: Updates for new HTML structure

* Update templates for latest release

* Bump to dev0

* Standardize logo image behavior between Sphinx and this theme (pydata#1132)

Co-authored-by: Daniel McCloy <dan@mccloy.info>
Co-authored-by: Chris Holdgraf <choldgraf@gmail.com>
Co-authored-by: Chris Holdgraf <choldgraf@berkeley.edu>

* 0.13.0rc1

* Build(deps): Bump http-cache-semantics from 4.1.0 to 4.1.1 (pydata#1154)

* DOC: Use only shield.io for badges in README (pydata#1152)

* Copyright semicolon (pydata#1160)

* FIX: Flex behavior should shrink header items instead of brand (pydata#1158)

Co-authored-by: Chris Holdgraf <choldgraf@gmail.com>
Fixes pydata#1143

* STYLE: lint the documentation with Doc8 (pydata#1150)

Fix pydata#1139

* Add test for internationalization and translations (pydata#1138)

* FIX: Javascript incorrect check for variable (pydata#1166)

* MAINT: update pypi classifiers (pydata#1153)

Fix pydata#1106

* remove emoji from landing page (pydata#1151)

* add fa icons instead of emoji

* remove fix for emojis

* use markup for readme emojis

* use pst-color-primary instead of sd-text-primary

* make our semantic colors available as classes

* try again

---------

Co-authored-by: Daniel McCloy <dan@mccloy.info>

* FIX: Narrow scope of style rule for GitHub & GitLab link shortening (pydata#1167)

Fixes pydata#1156

* ENH: Add breadcrumbs to article header (pydata#1142)

* ENH: Add breadcrumbs to article header

* Update src/pydata_sphinx_theme/theme/pydata_sphinx_theme/components/breadcrumbs.html

Co-authored-by: Tania Allard <taniar.allard@gmail.com>

* More fixes

* Improving nested page behavior

* Documenting breadcrumbs

* Update src/pydata_sphinx_theme/assets/styles/components/_breadcrumbs.scss

Co-authored-by: Rambaud Pierrick <12rambau@users.noreply.github.com>

* Breacrumbs have link color

---------

Co-authored-by: Tania Allard <taniar.allard@gmail.com>
Co-authored-by: Rambaud Pierrick <12rambau@users.noreply.github.com>

* Degrade gracefully when JavaScript is disabled (pydata#1146)

* Fix header vertical spacing and jupyter-sphinx cells (pydata#1164)

Fixes undefined

* RLS: v0.13.0rc2 (pydata#1170)

* DOCS: admonition customization (pydata#1155)

* first draft of the admonition customization

* typo in doc link

* flesh out admon. customization example; DRY

* use :code:rst instead of :literal:

* Update docs/_static/custom.css

---------

Co-authored-by: Daniel McCloy <dan@mccloy.info>

* Fix article header CSS (pydata#1171)

* “Edit this page” → “Edit on GitHub/GitLab/Bitbucket” (pydata#1177)

* “Edit this page” → “Edit on GitHub/GitLab/Bitbucket”

Fixes pydata#1172

* Add tests

* Fix typo

* Properly handle default_mode=auto when writing logos (pydata#1183)

We used to only defaulting to the light version when `default_mode` was
undefined, not when it was explicitly set to `auto`. We also need to
handle the latter, as the new test shows.

Closes pydata#1180

Co-authored-by: Jérémy Bobbio (Lunar) <lunar@softwareheritage.org>

* fix: correctly add DOM listeners (pydata#1179)

fix adding DOM listeners

* maint: update GitLab URL tests (pydata#1186)

Co-authored-by: JoerivanEngelen <joerivanengelen@hotmail.com>

* Standardize template structure in more sections (pydata#1184)

* Standardize template structure in all sections

* Fixing footer behavior

* Update docs/user_guide/layout.rst

Co-authored-by: Daniel McCloy <dan@mccloy.info>

* Remove use of id= as much as possible

---------

Co-authored-by: Daniel McCloy <dan@mccloy.info>

* maint: remove sphinx-panels support; remove deprecated config shims (pydata#1188)

* Minor style improvements to ablog (pydata#1185)

* RLS: v0.13.0rc3

* dev0

* FIX: Some style bugs (pydata#1191)

* FIX: Some style bugs

* Move link word wrap to global rule

* DOCS: Add internationalization instructions (pydata#1178)

Co-authored-by: James McKinney <26463+jpmckinney@users.noreply.github.com>

* Refactor contributing docs to be more modular (pydata#1173)

* Dev0

* Fix github gitlab brand (pydata#1194)

* RLS: v0.13.0rc4

* FIX: Make wide equations scroll (pydata#1196)

* Fix math scrollbars for realz (pydata#1198)

* Fix math scrollbars for realz

* Update src/pydata_sphinx_theme/assets/styles/content/_math.scss

* Update src/pydata_sphinx_theme/assets/styles/content/_math.scss

* copy_logo_images: do not render dynamic Sphinx template content (pydata#1204)

* copy_logo_images: do not render dynamic Sphinx template content when copying logo image files

* Update src/pydata_sphinx_theme/__init__.py

---------

Co-authored-by: Chris Holdgraf <choldgraf@gmail.com>

* Add conditional check for last-updated template (pydata#1201)

* Add conditional check for last-updated template

* Whitespace

* Properly set configuration with app.builder.theme_options (pydata#1199)

* Properly set configuration

* Dict to values

* Making it explicit in a function

* Better name

* Fix test

* Foot

* Revert complex config set

* Clarify docs

* Use CSS transform for skip link (pydata#1206)

* feat: Add full i18n support (pydata#1192)

Co-authored-by: Daniel McCloy <dan@mccloy.info>
Co-authored-by: James McKinney <26463+jpmckinney@users.noreply.github.com>
Co-authored-by: Chris Holdgraf <choldgraf@berkeley.edu>
Co-authored-by: Chris Holdgraf <choldgraf@gmail.com>

* Dev0

* FIX: Remove icon links component when no icon links given (pydata#1209)

* RLS: 0.13.0rc5

* dev0

* FIX: Get theme options in a more robust way (pydata#1214)

* RLS: v0.13.0rc6

* Make heading-style use the font-weight-heading value (pydata#1213)

* Make heading-style use the font-weight-heading value

* Separate font-weight setting for content headers and admonitions

* Flip var to be consistent with --pst-font-weight-heading instead

* RLS: v0.13.0

* bump: dev0

* DOCS: Remove <p> from announcement sample text (pydata#1223)

---------

Co-authored-by: Chris Holdgraf <choldgraf@berkeley.edu>
Co-authored-by: Daniel McCloy <dan@mccloy.info>
Co-authored-by: Nico Albers <nico.albers@aboutyou.com>
Co-authored-by: Chris Holdgraf <choldgraf@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Brendan Heberlein <bheberlein@wisc.edu>
Co-authored-by: Tania Allard <taniar.allard@gmail.com>
Co-authored-by: Lunar <lunar@debian.org>
Co-authored-by: Jean Abou-Samra <jean@abou-samra.fr>
Co-authored-by: Jérémy Bobbio (Lunar) <lunar@softwareheritage.org>
Co-authored-by: JoerivanEngelen <joerivanengelen@hotmail.com>
Co-authored-by: James McKinney <26463+jpmckinney@users.noreply.github.com>
Co-authored-by: James Addison <55152140+jayaddison@users.noreply.github.com>
Co-authored-by: Veronica Berglyd Olsen <1619840+vkbo@users.noreply.github.com>
@12rambau 12rambau mentioned this pull request Mar 21, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@trallard trallard trallard left review comments

@drammock drammock drammock approved these changes

@12rambau 12rambau 12rambau approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Styles are missing when JavaScript is disabled
4 participants
@lunar-debian @drammock @12rambau @trallard