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

Fix: Always load nojs.css irrespective of build_search_index setting #131

Merged
merged 1 commit into from
Aug 2, 2023
Merged

Fix: Always load nojs.css irrespective of build_search_index setting #131

merged 1 commit into from
Aug 2, 2023

Conversation

welpo
Copy link
Contributor

@welpo welpo commented Aug 2, 2023

Currently, the nojs.css file is only loaded when config.build_search_index returns false. This file is responsible for hiding JavaScript elements when JavaScript is disabled in the user's browser.

In practice, this means JavaScript elements like the theme switcher button or the encoded email (in the socials section) are mistakenly visible when JavaScript is disabled, as long as build_search_index is not set to true in config.toml. This is contrary to the expected behavior where these elements should be hidden when JavaScript is disabled, irrespective of the build_search_index setting.

This PR fixes this issue by removing the conditional, effectively ensuring that the nojs.css file is always loaded when JavaScript is disabled. This change aligns the behavior with the intended functionality of hiding specific JavaScript-dependent elements when JavaScript is not enabled.

@netlify
Copy link

netlify bot commented Aug 2, 2023

Deploy Preview for abridge ready!

Name Link
🔨 Latest commit 0c0c0e1
🔍 Latest deploy log https://app.netlify.com/sites/abridge/deploys/64cad8a2f7dea30008bd5897
😎 Deploy Preview https://deploy-preview-131--abridge.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@Jieiku
Copy link
Owner

Jieiku commented Aug 2, 2023

ah another relic of the past, at one point there was only a couple of js features, and I think I only hid the js search initially.

Thanks for spotting this, appreciate it.

@Jieiku Jieiku merged commit dfa3de9 into Jieiku:master Aug 2, 2023
4 of 5 checks passed
@welpo welpo deleted the fix/nojs-css-loading branch August 3, 2023 11:03
@welpo
Copy link
Contributor Author

welpo commented Aug 3, 2023

Understandable! I was working on hiding the theme switcher button on my theme, and at first I made it hide by default but become visible with the theme switcher js.

I decided to check out how you did it with abridge and I liked the idea of making it more versatile for possible future additions. So I copied your way of doing it (welpo/tabi@113a7f4) and noticed the issue with build_search_index.

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.

2 participants