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

Hide JS-required features if JS disabled #2912

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

Pandapip1
Copy link

@Pandapip1 Pandapip1 commented Sep 6, 2023

Things like light/dark mode and searching don't work properly when scripts are disabled. This hides those broken features when they are, well, broken.

@Pandapip1 Pandapip1 marked this pull request as ready for review September 6, 2023 20:00
brc-dd
brc-dd previously approved these changes Sep 10, 2023
@brc-dd
Copy link
Member

brc-dd commented Sep 10, 2023

There are actually many more things that will be broken. For example all of these need js-required:

image

copy code, code groups, sidebar groups, vplocalnav, vpflyout, last updated, ... all of these will need js-required.

@Pandapip1
Copy link
Author

Pandapip1 commented Sep 10, 2023

It looks like code groups and last updated are rendered server-side, so this should be fine.

Also - I would really like it if localnav could be modified to work without JS. I think it's possible.

@github-actions github-actions bot added the stale label Nov 6, 2023
@github-actions github-actions bot removed the stale label Nov 25, 2023
@github-actions github-actions bot added the stale label Feb 2, 2024
@Pandapip1 Pandapip1 requested a review from brc-dd March 11, 2024 22:33
@github-actions github-actions bot removed the stale label Apr 12, 2024
@Pandapip1
Copy link
Author

CC @brc-dd for re-review

@kiaking
Copy link
Member

kiaking commented Apr 23, 2024

To be honest... wouldn't it make more sense if we create a new JS free theme if we need one 🤔 ? If we add this, anytime we add new feature to the theme, we have to be very careful. And probably end up having give up slightly complicated feature because of this limitation.

I don't think majority of users uses VitePress site without JS enabled 👀

@Pandapip1
Copy link
Author

Pandapip1 commented Apr 23, 2024

I agree that JavaScript-disabled users are likely to be an extreme minority of vitepress visitors. That's why I think this solution is better.

The point of this is pull is just a one-off improvement that ensures that most of the stuff that is broken when JavaScript is disabled is hidden. I don't want to add any more maintenance burden onto the maintainers of this project. I know how overworked the maintainers of all major open source projects already are. If someone later notices that something doesn't work without JS, they can create pull request in that adds the js-required class themself.

@kiaking
Copy link
Member

kiaking commented Apr 24, 2024

he point of this is pull is just a one-off improvement that ensures that most of the stuff that is broken when JavaScript is disabled is hidden.

It wouldn't be one-off, since we must keep this in mind forever in the future whenever we add new feature to the theme. And also "most of the stuff" would be an issue to because if we miss 1 part, we have to fix it once this change is introduced.

If someone later notices that something doesn't work without JS, they can create pull request in that adds the js-required class themself.

This would be a bit unrealistic. Once any feature is added to the core, we can't ignore those features just because it wasn't added by the team.

I still believe creating a new theme that is carefully tailored in a way that works smoothly without JS would be much simpler 👀

@Pandapip1
Copy link
Author

Pandapip1 commented Apr 24, 2024

I still believe creating a new theme that is carefully tailored in a way that works smoothly without JS would be much simpler 👀

I disagree; I can't see a way to create an entirely new theme from scratch that is somehow moreless effort than this. I would love to be proven wrong, but I don't feel that I have enough time to learn how to do that properly. Also, it still wouldn't change the fact that there are broken features with the default theme with javascript off.

This would be a bit unrealistic. Once any feature is added to the core, we can't ignore those features just because it wasn't added by the team.

Yes, you absolutely can ignore them. You're a volunteer maintaining a FOSS project. Don't feel that just because you've helped make something, that you have to maintain it for all of eternity :)

It's unreasonable to assume that a feature will be maintained if nobody is interested in maintaining it.

@kiaking
Copy link
Member

kiaking commented Apr 25, 2024

I disagree; I can't see a way to create an entirely new theme from scratch that is somehow more effort than this.

Fork this repo then apply this change. Done.

@Pandapip1
Copy link
Author

Sorry, brain fart. Meant less, not more.

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

3 participants