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

"Clippy Lints" page forgets "Lint groups" selection #8510

Closed
smoelius opened this issue Mar 7, 2022 · 13 comments · Fixed by #10834
Closed

"Clippy Lints" page forgets "Lint groups" selection #8510

smoelius opened this issue Mar 7, 2022 · 13 comments · Fixed by #10834
Assignees
Labels
A-website Area: Improving the clippy website C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages good-first-issue These issues are a good way to get started with Clippy

Comments

@smoelius
Copy link
Contributor

smoelius commented Mar 7, 2022

Description

If you search for a Clippy lint by selecting one or more "Lint groups," click "View Source," and then click the back button, the lint groups will still be selected, but the selection will not be respected.

What I did:

  1. Go to the Clippy Lints page.
  2. Unselect all "Lint groups" but "Restriction."
  3. Expand "indexing_slicing."
  4. Click "View Source."
  5. Click the browser back button.

What I saw: more than just "Restriction" lints.

What I expected to see: just "Restriction" lints.

In fact, after clicking the back button, if you select and unselect a lint group, you can see those lints disappearing from the listing.

Clicking "Related Issues" seems to exhibit the same behavior.

Version

No response

Additional Labels

No response

@xFrednet xFrednet added A-website Area: Improving the clippy website C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages good-first-issue These issues are a good way to get started with Clippy labels Mar 9, 2022
@smoelius
Copy link
Contributor Author

smoelius commented Mar 9, 2022

I was considering cc-ing you, @xFrednet. Do you know what is required to fix this?

@xFrednet
Copy link
Member

xFrednet commented Mar 9, 2022

Nice, I really like the lint list, thanks for the thought. 🙃

Yes, the implementation simply has to store the selection in localstorage and retrieve it once the website loads. This is already done for the theme.

Here is the complete snippet The important lines are:

  • 607 as that stores the actual value
  • 613 as that loads the theme on page load

It's a bit hacky as I'm a novice with JS and angular.js, but it works fairly well. You can use the same approach if you want 🙃. You are welcome to ping me for the review or any questions or just if you think something could be interesting to me 🙃

@xFrednet
Copy link
Member

xFrednet commented Mar 9, 2022

Side note: we might merge #8070 soon (I just asked if we want to merge it ^^). That might cause some conflict, but those should be simple to solve, or you base your branch on that PR 🙃

@pitaj
Copy link
Contributor

pitaj commented Mar 25, 2022

I can't reproduce this in Firefox. Either it's already been fixed or isn't an issue on some browsers.

@xFrednet
Copy link
Member

xFrednet commented Mar 25, 2022

I can reproduce it on my Firefox (99.0b7 (64-bit)). I simply open the lint list change the group selection (Just select or deselect something).

image

When I reload the page the selection is reset to the default.

image

Your browser might cache the website but even that shouldn't effect this behavior as it's a static site 🤔

@smoelius
Copy link
Contributor Author

I too still see the problem with Chrome Version 99.0.4844.82 (Official Build) snap (64-bit).

But thank you for checking @pitaj. 🙏

@pitaj
Copy link
Contributor

pitaj commented Mar 25, 2022

@xFrednet Reloading the page will reset it, but viewing the source then hitting back seems to be cached by Firefox, at least on my end.

@xFrednet
Copy link
Member

Interesting :), if you want you can claim the issue. Note that this only involve JavaScript changes and nothing written in Rust 🙃

@whee
Copy link
Contributor

whee commented May 25, 2023

How do people feel about storing "Lint levels", "Lint groups", "Version", and perhaps the "Filter" value in the URL parameters?

Unlike theme, I personally don't expect the filters to retain their status if I were to leave the page entirely and come back. It seems like localStorage would have that sort of behavior, if I understand correctly.

Putting them in the URL parameters lets you share the settings with others, bookmark them, and solves the forward-back problem.

@smoelius
Copy link
Contributor Author

IMHO, your idea is a good one, @whee.

@whee
Copy link
Contributor

whee commented May 26, 2023

@rustbot claim

@xFrednet
Copy link
Member

I agree that this is a good idea :)

@smoelius
Copy link
Contributor Author

smoelius commented Jun 3, 2023

Very nice, @whee . :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-website Area: Improving the clippy website C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages good-first-issue These issues are a good way to get started with Clippy
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants