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

No more hidden elements #66123

Merged
merged 3 commits into from
Nov 8, 2019
Merged

Conversation

GuillaumeGomez
Copy link
Member

Fixes #66046.

Follow-up of #66082.

r? @kinnison

@GuillaumeGomez GuillaumeGomez force-pushed the no-more-hidden-elements branch from 2b37a6f to f66a331 Compare November 6, 2019 09:11
@Dylan-DPC-zz
Copy link

r? @Dylan-DPC

Comment on lines +164 to +167
function isHidden(elem) {
return elem.offsetHeight === 0;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

This function seems odd to have, there's code further down which called an isHidden() function you don't appear to have removed, so is this shadowing one?

Copy link
Member Author

Choose a reason for hiding this comment

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

You're absolutely right! I removed the old one.

Comment on lines 2655 to 2656
handleHashes();
highlightSourceLines();
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be more future-proof to just call onHashChange() here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Took me a while to think about it but I think you're right. I updated the code. Thanks!

@hdhoang hdhoang added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools labels Nov 7, 2019
@GuillaumeGomez GuillaumeGomez force-pushed the no-more-hidden-elements branch from bf2b270 to d4527b7 Compare November 7, 2019 09:40
Copy link
Contributor

@kinnison kinnison left a comment

Choose a reason for hiding this comment

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

Looks good to me. I've not confirmed behaviour because I'm in the middle of a different issue, but it seems sensible by inspection.

@GuillaumeGomez
Copy link
Member Author

Then let's wait for your confirmation. :)

@Dylan-DPC-zz
Copy link

looks good to me, ping me when ready to merge

@kinnison
Copy link
Contributor

kinnison commented Nov 7, 2019

Can confirm functioning.
@Dylan-DPC r=me

@Dylan-DPC-zz
Copy link

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Nov 7, 2019

📌 Commit d4527b7 has been approved by Dylan-DPC

@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 Nov 7, 2019
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Nov 8, 2019
…nts, r=Dylan-DPC

No more hidden elements

Fixes rust-lang#66046.

Follow-up of rust-lang#66082.

r? @kinnison
bors added a commit that referenced this pull request Nov 8, 2019
Rollup of 8 pull requests

Successful merges:

 - #65554 (Enhance the documentation of BufReader for potential data loss)
 - #65580 (Add `MaybeUninit` methods `uninit_array`, `slice_get_ref`, `slice_get_mut`)
 - #66049 (consistent handling of missing sysroot spans)
 - #66056 (rustc_metadata: Some reorganization of the module structure)
 - #66123 (No more hidden elements)
 - #66157 (Improve math log documentation examples)
 - #66165 (Ignore these tests ,since the called commands doesn't exist in VxWorks)
 - #66190 (rustc_target: inline abi::FloatTy into abi::Primitive.)

Failed merges:

 - #66188 (`MethodSig` -> `FnSig` & Use it in `ItemKind::Fn`)

r? @ghost
@bors bors merged commit d4527b7 into rust-lang:master Nov 8, 2019
@GuillaumeGomez GuillaumeGomez deleted the no-more-hidden-elements branch November 8, 2019 11:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Click to go to a hidden element should make it visible
5 participants