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.

Sign up for GitHub

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

Jump to bottom

rustdoc: Convert sub-variant toggle to HTML #84321

Merged
merged 2 commits into from
Apr 23, 2021
Merged

rustdoc: Convert sub-variant toggle to HTML #84321

merged 2 commits into from
Apr 23, 2021

Conversation

Swatinem
Copy link
Contributor

@Swatinem Swatinem commented Apr 18, 2021 •
edited
Loading

Instead of creating a JS toggle, this injects details/summary for
sub-variants of enums. This also fixes the CSS so that the toggle button
does not jump when expanding/collapsing.

Takes inspiration from #83337 and should be considered part of #83332. Not quite sure if the .sub-variant selectors could be further simplified? AFAICS it is only used in that place, and that does not seem to allow any recursion.

@rust-highfive
Copy link
Collaborator

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez

@rust-highfive
Copy link
Collaborator

r? @ollie27

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 18, 2021
@rust-log-analyzer

This comment has been minimized.

Sign in to view
jsha
jsha requested changes Apr 19, 2021
View reviewed changes
Copy link
Contributor

@jsha jsha left a comment

Choose a reason for hiding this comment

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

Overall looks great to me! Thanks for working on this.

@jsha jsha mentioned this pull request Apr 19, 2021
Closed
@Swatinem Swatinem force-pushed the subvariant-details branch from 7987f91 to 6461087 Compare April 19, 2021 07:31
@Swatinem
rustdoc: Convert sub-variant toggle to HTML
9f8eeca 
Instead of creating a JS toggle, this injects details/summary for
sub-variants of enums. This also fixes the CSS so that the toggle button
does not jump when expanding/collapsing.
@Swatinem Swatinem force-pushed the subvariant-details branch from 6461087 to 9f8eeca Compare April 19, 2021 07:38
@GuillaumeGomez
Copy link
Member

Also, before I forget: can you add a test in src/test/rustdoc-gui to be sure that if there is a url fragment, the corresponding element is displayed when we arrive on the page please? If you have questions about it, don't hesitate to ask me.

@Swatinem
Copy link
Contributor Author

can you add a test in src/test/rustdoc-gui

I tried to do, however I have troubles getting those tests to run on my Windows. Maybe I will try this on my Linux another time.

@GuillaumeGomez
Copy link
Member

I definitely can't help you for running things on windows, sorry. :-/

To give you a little help: here you need to have a "hidden" item by default and to go to a page with an ID referring to that item and then check that this item isn't hidden (stating the obvious, sorry haha). For the second part, you need to use the assert command (you can see the documentation here).

Good luck!

@GuillaumeGomez
Copy link
Member

Let's merge this for the moment, I'll add the rustdoc-gui test later on. Opening an issue to not forget about it. Don't hesitate to claim it if you want to give it a try!

@bors: r+

@bors
Copy link
Contributor

bors commented Apr 22, 2021

📌 Commit 8cf1b7734a5d9c50a4448c8e709a889228f15136 has been approved by GuillaumeGomez

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 22, 2021
@GuillaumeGomez GuillaumeGomez mentioned this pull request Apr 22, 2021
Closed
@Swatinem
Copy link
Contributor Author

@GuillaumeGomez just noticed while self-reviewing that I messed up the rustdoc-gui test (edited the wrong file after copying). If you are tracking followup work already I think its fine, otherwise I can fix the patch as well :-D

@GuillaumeGomez
Copy link
Member

No, please fix it now. :)

@bors: r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 22, 2021
@Swatinem Swatinem force-pushed the subvariant-details branch from 8cf1b77 to 00d0e79 Compare April 22, 2021 20:48
@Swatinem
Copy link
Contributor Author

@GuillaumeGomez fixed the accidental file mixup, however I didn’t quite get around to actually trying the ui test on my linux

@GuillaumeGomez
Copy link
Member

It's fine, I opened the issue for that. Please remove your test, it's not testing much in here so better remove it for the moment. :)

@Swatinem Swatinem force-pushed the subvariant-details branch from 00d0e79 to 85879fe Compare April 23, 2021 16:13
@Swatinem
Copy link
Contributor Author

Please remove your test

done

@GuillaumeGomez
Copy link
Member

Thanks!

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Apr 23, 2021

📌 Commit 85879fe has been approved by GuillaumeGomez

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 23, 2021
@JohnTitor JohnTitor mentioned this pull request Apr 23, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 23, 2021
@bors
Auto merge of rust-lang#84490 - JohnTitor:rollup-wrdj4ko, r=JohnTitor
bb491ed 
Rollup of 11 pull requests

Successful merges:

 - rust-lang#80805 (Improve `Iterator::by_ref` example)
 - rust-lang#84248 (Remove duplicated fn(Box<[T]>) -> Vec<T>)
 - rust-lang#84321 (rustdoc: Convert sub-variant toggle to HTML)
 - rust-lang#84359 (:arrow_up: rust-analyzer)
 - rust-lang#84374 (Clean up .gitignore)
 - rust-lang#84387 (Move `sys_common::poison` to `sync::poison`)
 - rust-lang#84430 (doc/platform-support: clarify UEFI support)
 - rust-lang#84433 (Prevent control, shift and alt keys to make search input lose focus)
 - rust-lang#84444 (doc: Get rid of "[+] show undocumented items" toggle on numeric From impls)
 - rust-lang#84456 (Fix ICE if original_span(fn_sig) returns a span not in body sourcefile)
 - rust-lang#84469 (Update comment on `PrimTy::name_str`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit d05e28d into rust-lang:master Apr 23, 2021
@rustbot rustbot added this to the 1.53.0 milestone Apr 23, 2021
@Swatinem Swatinem deleted the subvariant-details branch November 3, 2021 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jsha jsha jsha requested changes

@GuillaumeGomez GuillaumeGomez GuillaumeGomez left review comments

Assignees

@ollie27 ollie27

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.53.0
Development

Successfully merging this pull request may close these issues.
8 participants
@Swatinem @rust-highfive @rust-log-analyzer @GuillaumeGomez @bors @jsha @ollie27 @rustbot