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

Improve display of clippy lints page when JS is disabled #13585

Merged
merged 3 commits into from
Nov 2, 2024

Conversation

GuillaumeGomez
Copy link
Member

There is no point in displaying the settings menu and the filters if JS is disabled. So in this case, this PR simply hides it:

image

For the theme handling though, I need to send a fix to mdBook first. But once done, it'll look as expected (dark if system is in dark mode).

changelog: Improve clippy lints page display when JS is disabled.

r? @Alexendoo

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Oct 22, 2024
@flip1995
Copy link
Member

flip1995 commented Oct 22, 2024

2 things I also noticed with the new page:

  1. The link button doesn't work for me anymore (latest firefox) image

  2. The indentation of code blocks is broken/removed: https://rust-lang.github.io/rust-clippy/master/index.html#/cast_possible_truncation

@flip1995
Copy link
Member

flip1995 commented Oct 22, 2024

Also the link https://rust-lang.github.io/rust-clippy/master/index.html#cast_possible_truncation doesn't filter the lint list anymore (with or w/o / after the #), nor does it automatically expand the tab of the lint (no / after the #).

@GuillaumeGomez
Copy link
Member Author

Doesn't come from this PR, things are currently broken it seems. Urg. I'll add GUI tests after all fixes have been sent to prevent this situation from happening again.

Sending fix for the current page shortly.

@GuillaumeGomez
Copy link
Member Author

All mentioned bugs were fixed. This PR can be reviewed now. :)

Comment on lines 53 to 57
<noscript> {# #}
<div class="alert alert-danger" role="alert"> {# #}
Sorry, this site only works with JavaScript! :( {# #}
</div> {# #}
</noscript> {# #}
Copy link
Member

Choose a reason for hiding this comment

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

Let's leave some kind of message to say the filtering requires JS

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea!

@GuillaumeGomez
Copy link
Member Author

Added a CSS class on HTML with JS, removed the new CSS file and added back a message in case JS is disabled.

@Alexendoo
Copy link
Member

👍

@bors r+

@bors
Copy link
Contributor

bors commented Nov 2, 2024

📌 Commit ac764df has been approved by Alexendoo

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Nov 2, 2024

⌛ Testing commit ac764df with merge 52b8324...

@bors
Copy link
Contributor

bors commented Nov 2, 2024

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Alexendoo
Pushing 52b8324 to master...

@bors bors merged commit 52b8324 into rust-lang:master Nov 2, 2024
5 checks passed
@GuillaumeGomez GuillaumeGomez deleted the no-js branch November 2, 2024 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants