Remember state of top-level collapse toggle widget #48631
Conversation
|
Some changes occurred in HTML/CSS. |
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @steveklabnik (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
rust-highfive
added
the
S-waiting-on-review
src/librustdoc/html/static/main.js
Outdated
| @@ -1623,6 +1623,7 @@ | |||
| function toggleAllDocs() { | |||
| var toggle = document.getElementById("toggle-all-docs"); | |||
| if (hasClass(toggle, "will-expand")) { | |||
| updateLocalStorage("collapse", "false"); | |||
There was a problem hiding this comment.
It seems like "too simple" for a name. Can you replace it with "rustdoc-collapse" please?
There was a problem hiding this comment.
Also, please use booleans instead of strings.
There was a problem hiding this comment.
The values end up in localStorage, which converts all values to strings so I thought it clearer to avoid any type coercion and use strings always.
There was a problem hiding this comment.
Oh I didn't know, thanks for the clarification! Then just ignore this comment.
src/librustdoc/html/static/main.js
Outdated
| @@ -1632,6 +1633,7 @@ | |||
| collapseDocs(e, "show"); | |||
| }); | |||
| } else { | |||
| updateLocalStorage("collapse", "true"); | |||
src/librustdoc/html/static/main.js
Outdated
| @@ -1972,6 +1974,10 @@ | |||
| window.onresize = function() { | |||
| hideSidebar(); | |||
| }; | |||
|
|
|||
| if (getCurrentValue("collapse") === "true") { | |||
There was a problem hiding this comment.
Don't forget to update this line as well. ;)
|
The author addressed @GuillaumeGomez concerns. Could someone from @rust-lang/docs review this? |
|
This is great! Thanks for following up on @GuillaumeGomez's comment. This is something i've wanted myself, so i'm glad you were able to get it rolling. @bors r+ rollup |
|
📌 Commit 2dd81c8 has been approved by |
bors
added
S-waiting-on-bors
…g, r=QuietMisdreavus Remember state of top-level collapse toggle widget This change allows the big top-right expand/collapse toggle to remember its setting across navigation or page reloads. Prior to this change, there was this annoyance: - browse to some docs - Click the minus button to collapse them - browse to other docs (or reload the page) - Everything is expanded again The solution is based on storing a simple boolean flag in localStorage. I think it's a good improvement, but it does introduce the following potentially surprising behavior: - browse to some docs - click the minus button to collapse them - click to expand a particular item (not the main top-right big one) - reload the page, everything is collapsed Paired with @DebugSteven on this.
This change allows the big top-right expand/collapse toggle to remember its setting across navigation or page reloads. Prior to this change, there was this annoyance:
The solution is based on storing a simple boolean flag in localStorage. I think it's a good improvement, but it does introduce the following potentially surprising behavior:
Paired with @DebugSteven on this.