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 noscript for search js notification #245

Merged

Conversation

jdknight
Copy link
Contributor

The "search.html" template will generate a warning admonition about requiring JavaScript for search and automatically hiding the element when supported. While functional, if a client renders the page slowly, the warning notification may be visible to the user for a moment.

Instead of relying on JavaScript to suppress this warning, use a noscript tag to hide the warning for clients who do not support JavaScript. This also has the benefit of one less JavaScript call required by a client.


See also: sphinx-doc/sphinx#9617

@pradyunsg
Copy link
Owner

nit: Could you change "js" to "JS" in the commit message, since the shorter form is an acronym?

Copy link
Owner

@pradyunsg pradyunsg left a comment

Choose a reason for hiding this comment

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

LGTM, other than the nit pick on the commit mesage.

The "search.html" template will generate a warning admonition about
requiring JavaScript for search and automatically hiding the element
when supported. While functional, if a client renders the page slowly,
the warning notification may be visible to the user for a moment.

Instead of relying on JavaScript to suppress this warning, use a
`noscript` tag to hide the warning for clients who do not support
JavaScript. This also has the benefit of one less JavaScript call
required by a client.

Signed-off-by: James Knight <james.d.knight@live.com>
@jdknight jdknight force-pushed the use-noscript-for-search-js-detection branch from 0396587 to 2573e84 Compare September 12, 2021 00:45
@pradyunsg pradyunsg merged commit 4012c4d into pradyunsg:main Sep 22, 2021
@pradyunsg
Copy link
Owner

Thanks @jdknight!

@jdknight jdknight deleted the use-noscript-for-search-js-detection branch September 24, 2021 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants