Inhibit clicks on summary's children by jsha · Pull Request #91103 · rust-lang/rust

jsha · 2021-11-21T08:48:39Z

A byproduct of using <details> and <summary> to show/hide detailed documentation was that clicking any part of a method heading (or impl heading) would show or hide the documentation. This was not super noticeable because clicking a link inside the method heading would navigate to that link. But clicking any unlinked black text in a method heading would trigger the behavior.

That behavior was somewhat unexpected, and means that if you try to click a type name in a method heading, but miss by a few pixels, you get a confusing surprise.

This change inhibits that behavior by putting an event listener on most summaries that cancels the event unless the event target was the summary itself. In practice, that means it cancels the event unless the target was the "[+]" / "[-]", because the rest of the heading is wrapped inside a <div>, which is the target for anything that doesn't have a more specific target.

r? @Manishearth

rust-highfive · 2021-11-21T08:48:41Z

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez

A byproduct of using `<details>` and `<summary>` to show/hide detailed documentation was that clicking any part of a method heading (or impl heading) would show or hide the documentation. This was not super noticeable because clicking a link inside the method heading would navigate to that link. But clicking any unlinked black text in a method heading would trigger the behavior. That behavior was somewhat unexpected, and means that if you try to click a type name in a method heading, but miss by a few pixels, you get a confusing surprise. This change inhibits that behavior by putting an event listener on most summaries that cancels the event unless the event target was the summary itself. In practice, that means it cancels the event unless the target was the "[+]" / "[-]", because the rest of the heading is wrapped inside a `<div>`, which is the target for anything that doesn't have a more specific target.

GuillaumeGomez · 2021-11-21T13:53:30Z

Please add a GUI test, otherwise it seems like a good idea. It often happens that when I highlight something, it closes the the doc, which is a bit annoying.

GuillaumeGomez · 2021-11-22T08:55:16Z

You can click directly on h4.code-header. I'd much prefer avoiding to use hard coded positions if possible. :)

GuillaumeGomez · 2021-11-22T08:55:23Z

GuillaumeGomez · 2021-11-22T08:55:39Z

Once you're done, please remove this one. ;)

jsha · 2021-11-22T09:06:41Z

I wrote a GUI test, but I couldn't get it to work. I tried to find the coordinates on which to simulate a click by loading the page in a regular browser, but when I tell the tester to click on the coordinates that correspond to the toggle, it doesn't appear to do anything. I tried using --no-headless but the browser window appears and disappears instantly. Any ideas how to debug?

GuillaumeGomez · 2021-11-22T09:10:36Z

Ah right, for that I use wait-for: 600000 (for 10 min). It allows to keep the windows around to debug in peace.

GuillaumeGomez · 2021-11-22T09:15:18Z

Looks good to me, one nit remaining but can be ignored.

GuillaumeGomez · 2021-11-22T09:21:10Z

Thanks! r=me once CI pass

jsha · 2021-11-22T19:01:42Z

@bors try

bors · 2021-11-22T19:01:56Z

⌛ Trying commit 7f35556 with merge 057c2213acefad12356c4f990ecff30c5b32bdf6...

GuillaumeGomez · 2021-11-22T19:04:19Z

Why the try?

jsha · 2021-11-22T19:56:53Z

Perhaps I misread the bors docs? Was attempting to trigger a normal re-run of the tests, since I can't reproduce the failure locally. And the CI status reported here doesn't show a failure, just a cancelled check? https://bors.tech/documentation/

GuillaumeGomez · 2021-11-22T20:00:49Z

It's easier this way. ;)

jsha · 2021-11-23T00:09:24Z

@bors r=Manishearth,GuillaumeGomez

bors · 2021-11-23T00:09:26Z

📌 Commit 7f35556 has been approved by Manishearth,GuillaumeGomez

…, r=Manishearth,GuillaumeGomez Inhibit clicks on summary's children A byproduct of using `<details>` and `<summary>` to show/hide detailed documentation was that clicking any part of a method heading (or impl heading) would show or hide the documentation. This was not super noticeable because clicking a link inside the method heading would navigate to that link. But clicking any unlinked black text in a method heading would trigger the behavior. That behavior was somewhat unexpected, and means that if you try to click a type name in a method heading, but miss by a few pixels, you get a confusing surprise. This change inhibits that behavior by putting an event listener on most summaries that cancels the event unless the event target was the summary itself. In practice, that means it cancels the event unless the target was the "[+]" / "[-]", because the rest of the heading is wrapped inside a `<div>`, which is the target for anything that doesn't have a more specific target. r? `@Manishearth`

…askrgr Rollup of 6 pull requests Successful merges: - rust-lang#90856 (Suggestion to wrap inner types using 'allocator_api' in tuple) - rust-lang#91103 (Inhibit clicks on summary's children) - rust-lang#91137 (Give people a single link they can click in the contributing guide) - rust-lang#91140 (Split inline const to two feature gates and mark expression position inline const complete) - rust-lang#91148 (Use `derive_default_enum` in the compiler) - rust-lang#91153 (kernel_copy: avoid panic on unexpected OS error) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

…eGomez Fix toggle-click-deadspace rustdoc-gui test In rust-lang#91103 I introduced a rustdoc-gui test for clicks on toggles. I introduced some documentation on a method in lib2/struct.Foo.html so there would be something to toggle, but accidentally left the test checking test_docs/struct.Foo.html. That caused the test to reliably fail. I'm not sure how that test got past GitHub Actions and bors, but it's manifesting in test failures at rust-lang#91062 (comment) and rust-lang#91170 (comment). This fixes by pointing at the right file. r? `@GuillaumeGomez`

jsha · 2021-11-21T08:48:39Z

A byproduct of using <details> and <summary> to show/hide detailed documentation was that clicking any part of a method heading (or impl heading) would show or hide the documentation. This was not super noticeable because clicking a link inside the method heading would navigate to that link. But clicking any unlinked black text in a method heading would trigger the behavior.

That behavior was somewhat unexpected, and means that if you try to click a type name in a method heading, but miss by a few pixels, you get a confusing surprise.

This change inhibits that behavior by putting an event listener on most summaries that cancels the event unless the event target was the summary itself. In practice, that means it cancels the event unless the target was the "[+]" / "[-]", because the rest of the heading is wrapped inside a <div>, which is the target for anything that doesn't have a more specific target.

r? @Manishearth

rust-highfive · 2021-11-21T08:48:41Z

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez

A byproduct of using `<details>` and `<summary>` to show/hide detailed documentation was that clicking any part of a method heading (or impl heading) would show or hide the documentation. This was not super noticeable because clicking a link inside the method heading would navigate to that link. But clicking any unlinked black text in a method heading would trigger the behavior. That behavior was somewhat unexpected, and means that if you try to click a type name in a method heading, but miss by a few pixels, you get a confusing surprise. This change inhibits that behavior by putting an event listener on most summaries that cancels the event unless the event target was the summary itself. In practice, that means it cancels the event unless the target was the "[+]" / "[-]", because the rest of the heading is wrapped inside a `<div>`, which is the target for anything that doesn't have a more specific target.

GuillaumeGomez · 2021-11-21T13:53:30Z

Please add a GUI test, otherwise it seems like a good idea. It often happens that when I highlight something, it closes the the doc, which is a bit annoying.

GuillaumeGomez · 2021-11-22T08:55:16Z

You can click directly on h4.code-header. I'd much prefer avoiding to use hard coded positions if possible. :)

GuillaumeGomez · 2021-11-22T08:55:23Z

GuillaumeGomez · 2021-11-22T08:55:39Z

Once you're done, please remove this one. ;)

jsha · 2021-11-22T09:06:41Z

I wrote a GUI test, but I couldn't get it to work. I tried to find the coordinates on which to simulate a click by loading the page in a regular browser, but when I tell the tester to click on the coordinates that correspond to the toggle, it doesn't appear to do anything. I tried using --no-headless but the browser window appears and disappears instantly. Any ideas how to debug?

GuillaumeGomez · 2021-11-22T09:10:36Z

Ah right, for that I use wait-for: 600000 (for 10 min). It allows to keep the windows around to debug in peace.

GuillaumeGomez · 2021-11-22T09:15:18Z

Looks good to me, one nit remaining but can be ignored.

GuillaumeGomez · 2021-11-22T09:21:10Z

Thanks! r=me once CI pass

jsha · 2021-11-22T19:01:42Z

@bors try

bors · 2021-11-22T19:01:56Z

⌛ Trying commit 7f35556 with merge 057c2213acefad12356c4f990ecff30c5b32bdf6...

GuillaumeGomez · 2021-11-22T19:04:19Z

Why the try?

jsha · 2021-11-22T19:56:53Z

Perhaps I misread the bors docs? Was attempting to trigger a normal re-run of the tests, since I can't reproduce the failure locally. And the CI status reported here doesn't show a failure, just a cancelled check? https://bors.tech/documentation/

GuillaumeGomez · 2021-11-22T20:00:49Z

It's easier this way. ;)

jsha · 2021-11-23T00:09:24Z

@bors r=Manishearth,GuillaumeGomez

bors · 2021-11-23T00:09:26Z

📌 Commit 7f35556 has been approved by Manishearth,GuillaumeGomez

…, r=Manishearth,GuillaumeGomez Inhibit clicks on summary's children A byproduct of using `<details>` and `<summary>` to show/hide detailed documentation was that clicking any part of a method heading (or impl heading) would show or hide the documentation. This was not super noticeable because clicking a link inside the method heading would navigate to that link. But clicking any unlinked black text in a method heading would trigger the behavior. That behavior was somewhat unexpected, and means that if you try to click a type name in a method heading, but miss by a few pixels, you get a confusing surprise. This change inhibits that behavior by putting an event listener on most summaries that cancels the event unless the event target was the summary itself. In practice, that means it cancels the event unless the target was the "[+]" / "[-]", because the rest of the heading is wrapped inside a `<div>`, which is the target for anything that doesn't have a more specific target. r? `@Manishearth`

…askrgr Rollup of 6 pull requests Successful merges: - rust-lang#90856 (Suggestion to wrap inner types using 'allocator_api' in tuple) - rust-lang#91103 (Inhibit clicks on summary's children) - rust-lang#91137 (Give people a single link they can click in the contributing guide) - rust-lang#91140 (Split inline const to two feature gates and mark expression position inline const complete) - rust-lang#91148 (Use `derive_default_enum` in the compiler) - rust-lang#91153 (kernel_copy: avoid panic on unexpected OS error) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

…eGomez Fix toggle-click-deadspace rustdoc-gui test In rust-lang#91103 I introduced a rustdoc-gui test for clicks on toggles. I introduced some documentation on a method in lib2/struct.Foo.html so there would be something to toggle, but accidentally left the test checking test_docs/struct.Foo.html. That caused the test to reliably fail. I'm not sure how that test got past GitHub Actions and bors, but it's manifesting in test failures at rust-lang#91062 (comment) and rust-lang#91170 (comment). This fixes by pointing at the right file. r? `@GuillaumeGomez`

jsha · 2021-11-21T08:48:39Z

A byproduct of using <details> and <summary> to show/hide detailed documentation was that clicking any part of a method heading (or impl heading) would show or hide the documentation. This was not super noticeable because clicking a link inside the method heading would navigate to that link. But clicking any unlinked black text in a method heading would trigger the behavior.

That behavior was somewhat unexpected, and means that if you try to click a type name in a method heading, but miss by a few pixels, you get a confusing surprise.

This change inhibits that behavior by putting an event listener on most summaries that cancels the event unless the event target was the summary itself. In practice, that means it cancels the event unless the target was the "[+]" / "[-]", because the rest of the heading is wrapped inside a <div>, which is the target for anything that doesn't have a more specific target.

r? @Manishearth

rust-highfive · 2021-11-21T08:48:41Z

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez

A byproduct of using `<details>` and `<summary>` to show/hide detailed documentation was that clicking any part of a method heading (or impl heading) would show or hide the documentation. This was not super noticeable because clicking a link inside the method heading would navigate to that link. But clicking any unlinked black text in a method heading would trigger the behavior. That behavior was somewhat unexpected, and means that if you try to click a type name in a method heading, but miss by a few pixels, you get a confusing surprise. This change inhibits that behavior by putting an event listener on most summaries that cancels the event unless the event target was the summary itself. In practice, that means it cancels the event unless the target was the "[+]" / "[-]", because the rest of the heading is wrapped inside a `<div>`, which is the target for anything that doesn't have a more specific target.

GuillaumeGomez · 2021-11-21T13:53:30Z

Please add a GUI test, otherwise it seems like a good idea. It often happens that when I highlight something, it closes the the doc, which is a bit annoying.

GuillaumeGomez · 2021-11-22T08:55:16Z

You can click directly on h4.code-header. I'd much prefer avoiding to use hard coded positions if possible. :)

GuillaumeGomez · 2021-11-22T08:55:23Z

GuillaumeGomez · 2021-11-22T08:55:39Z

Once you're done, please remove this one. ;)

jsha · 2021-11-22T09:06:41Z

I wrote a GUI test, but I couldn't get it to work. I tried to find the coordinates on which to simulate a click by loading the page in a regular browser, but when I tell the tester to click on the coordinates that correspond to the toggle, it doesn't appear to do anything. I tried using --no-headless but the browser window appears and disappears instantly. Any ideas how to debug?

GuillaumeGomez · 2021-11-22T09:10:36Z

Ah right, for that I use wait-for: 600000 (for 10 min). It allows to keep the windows around to debug in peace.

GuillaumeGomez · 2021-11-22T09:15:18Z

Looks good to me, one nit remaining but can be ignored.

GuillaumeGomez · 2021-11-22T09:21:10Z

Thanks! r=me once CI pass

jsha · 2021-11-22T19:01:42Z

@bors try

bors · 2021-11-22T19:01:56Z

⌛ Trying commit 7f35556 with merge 057c2213acefad12356c4f990ecff30c5b32bdf6...

GuillaumeGomez · 2021-11-22T19:04:19Z

Why the try?

jsha · 2021-11-22T19:56:53Z

Perhaps I misread the bors docs? Was attempting to trigger a normal re-run of the tests, since I can't reproduce the failure locally. And the CI status reported here doesn't show a failure, just a cancelled check? https://bors.tech/documentation/

GuillaumeGomez · 2021-11-22T20:00:49Z

It's easier this way. ;)

jsha · 2021-11-23T00:09:24Z

@bors r=Manishearth,GuillaumeGomez

bors · 2021-11-23T00:09:26Z

📌 Commit 7f35556 has been approved by Manishearth,GuillaumeGomez

…, r=Manishearth,GuillaumeGomez Inhibit clicks on summary's children A byproduct of using `<details>` and `<summary>` to show/hide detailed documentation was that clicking any part of a method heading (or impl heading) would show or hide the documentation. This was not super noticeable because clicking a link inside the method heading would navigate to that link. But clicking any unlinked black text in a method heading would trigger the behavior. That behavior was somewhat unexpected, and means that if you try to click a type name in a method heading, but miss by a few pixels, you get a confusing surprise. This change inhibits that behavior by putting an event listener on most summaries that cancels the event unless the event target was the summary itself. In practice, that means it cancels the event unless the target was the "[+]" / "[-]", because the rest of the heading is wrapped inside a `<div>`, which is the target for anything that doesn't have a more specific target. r? `@Manishearth`

…askrgr Rollup of 6 pull requests Successful merges: - rust-lang#90856 (Suggestion to wrap inner types using 'allocator_api' in tuple) - rust-lang#91103 (Inhibit clicks on summary's children) - rust-lang#91137 (Give people a single link they can click in the contributing guide) - rust-lang#91140 (Split inline const to two feature gates and mark expression position inline const complete) - rust-lang#91148 (Use `derive_default_enum` in the compiler) - rust-lang#91153 (kernel_copy: avoid panic on unexpected OS error) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

…eGomez Fix toggle-click-deadspace rustdoc-gui test In rust-lang#91103 I introduced a rustdoc-gui test for clicks on toggles. I introduced some documentation on a method in lib2/struct.Foo.html so there would be something to toggle, but accidentally left the test checking test_docs/struct.Foo.html. That caused the test to reliably fail. I'm not sure how that test got past GitHub Actions and bors, but it's manifesting in test failures at rust-lang#91062 (comment) and rust-lang#91170 (comment). This fixes by pointing at the right file. r? `@GuillaumeGomez`

jsha · 2021-11-21T08:48:39Z

A byproduct of using <details> and <summary> to show/hide detailed documentation was that clicking any part of a method heading (or impl heading) would show or hide the documentation. This was not super noticeable because clicking a link inside the method heading would navigate to that link. But clicking any unlinked black text in a method heading would trigger the behavior.

That behavior was somewhat unexpected, and means that if you try to click a type name in a method heading, but miss by a few pixels, you get a confusing surprise.

This change inhibits that behavior by putting an event listener on most summaries that cancels the event unless the event target was the summary itself. In practice, that means it cancels the event unless the target was the "[+]" / "[-]", because the rest of the heading is wrapped inside a <div>, which is the target for anything that doesn't have a more specific target.

r? @Manishearth

rust-highfive · 2021-11-21T08:48:41Z

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez

A byproduct of using `<details>` and `<summary>` to show/hide detailed documentation was that clicking any part of a method heading (or impl heading) would show or hide the documentation. This was not super noticeable because clicking a link inside the method heading would navigate to that link. But clicking any unlinked black text in a method heading would trigger the behavior. That behavior was somewhat unexpected, and means that if you try to click a type name in a method heading, but miss by a few pixels, you get a confusing surprise. This change inhibits that behavior by putting an event listener on most summaries that cancels the event unless the event target was the summary itself. In practice, that means it cancels the event unless the target was the "[+]" / "[-]", because the rest of the heading is wrapped inside a `<div>`, which is the target for anything that doesn't have a more specific target.

GuillaumeGomez · 2021-11-21T13:53:30Z

Please add a GUI test, otherwise it seems like a good idea. It often happens that when I highlight something, it closes the the doc, which is a bit annoying.

GuillaumeGomez · 2021-11-22T08:55:16Z

You can click directly on h4.code-header. I'd much prefer avoiding to use hard coded positions if possible. :)

GuillaumeGomez · 2021-11-22T08:55:23Z

GuillaumeGomez · 2021-11-22T08:55:39Z

Once you're done, please remove this one. ;)

jsha · 2021-11-22T09:06:41Z

I wrote a GUI test, but I couldn't get it to work. I tried to find the coordinates on which to simulate a click by loading the page in a regular browser, but when I tell the tester to click on the coordinates that correspond to the toggle, it doesn't appear to do anything. I tried using --no-headless but the browser window appears and disappears instantly. Any ideas how to debug?

GuillaumeGomez · 2021-11-22T09:10:36Z

Ah right, for that I use wait-for: 600000 (for 10 min). It allows to keep the windows around to debug in peace.

GuillaumeGomez · 2021-11-22T09:15:18Z

Looks good to me, one nit remaining but can be ignored.

GuillaumeGomez · 2021-11-22T09:21:10Z

Thanks! r=me once CI pass

jsha · 2021-11-22T19:01:42Z

@bors try

bors · 2021-11-22T19:01:56Z

⌛ Trying commit 7f35556 with merge 057c2213acefad12356c4f990ecff30c5b32bdf6...

GuillaumeGomez · 2021-11-22T19:04:19Z

Why the try?

jsha · 2021-11-22T19:56:53Z

Perhaps I misread the bors docs? Was attempting to trigger a normal re-run of the tests, since I can't reproduce the failure locally. And the CI status reported here doesn't show a failure, just a cancelled check? https://bors.tech/documentation/

GuillaumeGomez · 2021-11-22T20:00:49Z

It's easier this way. ;)

jsha · 2021-11-23T00:09:24Z

@bors r=Manishearth,GuillaumeGomez

bors · 2021-11-23T00:09:26Z

📌 Commit 7f35556 has been approved by Manishearth,GuillaumeGomez

…, r=Manishearth,GuillaumeGomez Inhibit clicks on summary's children A byproduct of using `<details>` and `<summary>` to show/hide detailed documentation was that clicking any part of a method heading (or impl heading) would show or hide the documentation. This was not super noticeable because clicking a link inside the method heading would navigate to that link. But clicking any unlinked black text in a method heading would trigger the behavior. That behavior was somewhat unexpected, and means that if you try to click a type name in a method heading, but miss by a few pixels, you get a confusing surprise. This change inhibits that behavior by putting an event listener on most summaries that cancels the event unless the event target was the summary itself. In practice, that means it cancels the event unless the target was the "[+]" / "[-]", because the rest of the heading is wrapped inside a `<div>`, which is the target for anything that doesn't have a more specific target. r? `@Manishearth`

…askrgr Rollup of 6 pull requests Successful merges: - rust-lang#90856 (Suggestion to wrap inner types using 'allocator_api' in tuple) - rust-lang#91103 (Inhibit clicks on summary's children) - rust-lang#91137 (Give people a single link they can click in the contributing guide) - rust-lang#91140 (Split inline const to two feature gates and mark expression position inline const complete) - rust-lang#91148 (Use `derive_default_enum` in the compiler) - rust-lang#91153 (kernel_copy: avoid panic on unexpected OS error) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

…eGomez Fix toggle-click-deadspace rustdoc-gui test In rust-lang#91103 I introduced a rustdoc-gui test for clicks on toggles. I introduced some documentation on a method in lib2/struct.Foo.html so there would be something to toggle, but accidentally left the test checking test_docs/struct.Foo.html. That caused the test to reliably fail. I'm not sure how that test got past GitHub Actions and bors, but it's manifesting in test failures at rust-lang#91062 (comment) and rust-lang#91170 (comment). This fixes by pointing at the right file. r? `@GuillaumeGomez`

jsha · 2021-11-21T08:48:39Z

A byproduct of using <details> and <summary> to show/hide detailed documentation was that clicking any part of a method heading (or impl heading) would show or hide the documentation. This was not super noticeable because clicking a link inside the method heading would navigate to that link. But clicking any unlinked black text in a method heading would trigger the behavior.

That behavior was somewhat unexpected, and means that if you try to click a type name in a method heading, but miss by a few pixels, you get a confusing surprise.

This change inhibits that behavior by putting an event listener on most summaries that cancels the event unless the event target was the summary itself. In practice, that means it cancels the event unless the target was the "[+]" / "[-]", because the rest of the heading is wrapped inside a <div>, which is the target for anything that doesn't have a more specific target.

r? @Manishearth

rust-highfive · 2021-11-21T08:48:41Z

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez

A byproduct of using `<details>` and `<summary>` to show/hide detailed documentation was that clicking any part of a method heading (or impl heading) would show or hide the documentation. This was not super noticeable because clicking a link inside the method heading would navigate to that link. But clicking any unlinked black text in a method heading would trigger the behavior. That behavior was somewhat unexpected, and means that if you try to click a type name in a method heading, but miss by a few pixels, you get a confusing surprise. This change inhibits that behavior by putting an event listener on most summaries that cancels the event unless the event target was the summary itself. In practice, that means it cancels the event unless the target was the "[+]" / "[-]", because the rest of the heading is wrapped inside a `<div>`, which is the target for anything that doesn't have a more specific target.

GuillaumeGomez · 2021-11-21T13:53:30Z

Please add a GUI test, otherwise it seems like a good idea. It often happens that when I highlight something, it closes the the doc, which is a bit annoying.

GuillaumeGomez · 2021-11-22T08:55:16Z

You can click directly on h4.code-header. I'd much prefer avoiding to use hard coded positions if possible. :)

GuillaumeGomez · 2021-11-22T08:55:23Z

GuillaumeGomez · 2021-11-22T08:55:39Z

Once you're done, please remove this one. ;)

jsha · 2021-11-22T09:06:41Z

I wrote a GUI test, but I couldn't get it to work. I tried to find the coordinates on which to simulate a click by loading the page in a regular browser, but when I tell the tester to click on the coordinates that correspond to the toggle, it doesn't appear to do anything. I tried using --no-headless but the browser window appears and disappears instantly. Any ideas how to debug?

GuillaumeGomez · 2021-11-22T09:10:36Z

Ah right, for that I use wait-for: 600000 (for 10 min). It allows to keep the windows around to debug in peace.

GuillaumeGomez · 2021-11-22T09:15:18Z

Looks good to me, one nit remaining but can be ignored.

GuillaumeGomez · 2021-11-22T09:21:10Z

Thanks! r=me once CI pass

jsha · 2021-11-22T19:01:42Z

@bors try

bors · 2021-11-22T19:01:56Z

⌛ Trying commit 7f35556 with merge 057c2213acefad12356c4f990ecff30c5b32bdf6...

GuillaumeGomez · 2021-11-22T19:04:19Z

Why the try?

jsha · 2021-11-22T19:56:53Z

Perhaps I misread the bors docs? Was attempting to trigger a normal re-run of the tests, since I can't reproduce the failure locally. And the CI status reported here doesn't show a failure, just a cancelled check? https://bors.tech/documentation/

GuillaumeGomez · 2021-11-22T20:00:49Z

It's easier this way. ;)

jsha · 2021-11-23T00:09:24Z

@bors r=Manishearth,GuillaumeGomez

bors · 2021-11-23T00:09:26Z

📌 Commit 7f35556 has been approved by Manishearth,GuillaumeGomez

…, r=Manishearth,GuillaumeGomez Inhibit clicks on summary's children A byproduct of using `<details>` and `<summary>` to show/hide detailed documentation was that clicking any part of a method heading (or impl heading) would show or hide the documentation. This was not super noticeable because clicking a link inside the method heading would navigate to that link. But clicking any unlinked black text in a method heading would trigger the behavior. That behavior was somewhat unexpected, and means that if you try to click a type name in a method heading, but miss by a few pixels, you get a confusing surprise. This change inhibits that behavior by putting an event listener on most summaries that cancels the event unless the event target was the summary itself. In practice, that means it cancels the event unless the target was the "[+]" / "[-]", because the rest of the heading is wrapped inside a `<div>`, which is the target for anything that doesn't have a more specific target. r? `@Manishearth`

…askrgr Rollup of 6 pull requests Successful merges: - rust-lang#90856 (Suggestion to wrap inner types using 'allocator_api' in tuple) - rust-lang#91103 (Inhibit clicks on summary's children) - rust-lang#91137 (Give people a single link they can click in the contributing guide) - rust-lang#91140 (Split inline const to two feature gates and mark expression position inline const complete) - rust-lang#91148 (Use `derive_default_enum` in the compiler) - rust-lang#91153 (kernel_copy: avoid panic on unexpected OS error) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

…eGomez Fix toggle-click-deadspace rustdoc-gui test In rust-lang#91103 I introduced a rustdoc-gui test for clicks on toggles. I introduced some documentation on a method in lib2/struct.Foo.html so there would be something to toggle, but accidentally left the test checking test_docs/struct.Foo.html. That caused the test to reliably fail. I'm not sure how that test got past GitHub Actions and bors, but it's manifesting in test failures at rust-lang#91062 (comment) and rust-lang#91170 (comment). This fixes by pointing at the right file. r? `@GuillaumeGomez`

jsha · 2021-11-21T08:48:39Z

A byproduct of using <details> and <summary> to show/hide detailed documentation was that clicking any part of a method heading (or impl heading) would show or hide the documentation. This was not super noticeable because clicking a link inside the method heading would navigate to that link. But clicking any unlinked black text in a method heading would trigger the behavior.

That behavior was somewhat unexpected, and means that if you try to click a type name in a method heading, but miss by a few pixels, you get a confusing surprise.

This change inhibits that behavior by putting an event listener on most summaries that cancels the event unless the event target was the summary itself. In practice, that means it cancels the event unless the target was the "[+]" / "[-]", because the rest of the heading is wrapped inside a <div>, which is the target for anything that doesn't have a more specific target.

r? @Manishearth

rust-highfive · 2021-11-21T08:48:41Z

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez

A byproduct of using `<details>` and `<summary>` to show/hide detailed documentation was that clicking any part of a method heading (or impl heading) would show or hide the documentation. This was not super noticeable because clicking a link inside the method heading would navigate to that link. But clicking any unlinked black text in a method heading would trigger the behavior. That behavior was somewhat unexpected, and means that if you try to click a type name in a method heading, but miss by a few pixels, you get a confusing surprise. This change inhibits that behavior by putting an event listener on most summaries that cancels the event unless the event target was the summary itself. In practice, that means it cancels the event unless the target was the "[+]" / "[-]", because the rest of the heading is wrapped inside a `<div>`, which is the target for anything that doesn't have a more specific target.

GuillaumeGomez · 2021-11-21T13:53:30Z

Please add a GUI test, otherwise it seems like a good idea. It often happens that when I highlight something, it closes the the doc, which is a bit annoying.

GuillaumeGomez · 2021-11-22T08:55:16Z

You can click directly on h4.code-header. I'd much prefer avoiding to use hard coded positions if possible. :)

GuillaumeGomez · 2021-11-22T08:55:23Z

GuillaumeGomez · 2021-11-22T08:55:39Z

Once you're done, please remove this one. ;)

jsha · 2021-11-22T09:06:41Z

I wrote a GUI test, but I couldn't get it to work. I tried to find the coordinates on which to simulate a click by loading the page in a regular browser, but when I tell the tester to click on the coordinates that correspond to the toggle, it doesn't appear to do anything. I tried using --no-headless but the browser window appears and disappears instantly. Any ideas how to debug?

GuillaumeGomez · 2021-11-22T09:10:36Z

Ah right, for that I use wait-for: 600000 (for 10 min). It allows to keep the windows around to debug in peace.

GuillaumeGomez · 2021-11-22T09:15:18Z

Looks good to me, one nit remaining but can be ignored.

GuillaumeGomez · 2021-11-22T09:21:10Z

Thanks! r=me once CI pass

jsha · 2021-11-22T19:01:42Z

@bors try

bors · 2021-11-22T19:01:56Z

⌛ Trying commit 7f35556 with merge 057c2213acefad12356c4f990ecff30c5b32bdf6...

GuillaumeGomez · 2021-11-22T19:04:19Z

Why the try?

jsha · 2021-11-22T19:56:53Z

Perhaps I misread the bors docs? Was attempting to trigger a normal re-run of the tests, since I can't reproduce the failure locally. And the CI status reported here doesn't show a failure, just a cancelled check? https://bors.tech/documentation/

GuillaumeGomez · 2021-11-22T20:00:49Z

It's easier this way. ;)

jsha · 2021-11-23T00:09:24Z

@bors r=Manishearth,GuillaumeGomez

bors · 2021-11-23T00:09:26Z

📌 Commit 7f35556 has been approved by Manishearth,GuillaumeGomez

…, r=Manishearth,GuillaumeGomez Inhibit clicks on summary's children A byproduct of using `<details>` and `<summary>` to show/hide detailed documentation was that clicking any part of a method heading (or impl heading) would show or hide the documentation. This was not super noticeable because clicking a link inside the method heading would navigate to that link. But clicking any unlinked black text in a method heading would trigger the behavior. That behavior was somewhat unexpected, and means that if you try to click a type name in a method heading, but miss by a few pixels, you get a confusing surprise. This change inhibits that behavior by putting an event listener on most summaries that cancels the event unless the event target was the summary itself. In practice, that means it cancels the event unless the target was the "[+]" / "[-]", because the rest of the heading is wrapped inside a `<div>`, which is the target for anything that doesn't have a more specific target. r? `@Manishearth`

…askrgr Rollup of 6 pull requests Successful merges: - rust-lang#90856 (Suggestion to wrap inner types using 'allocator_api' in tuple) - rust-lang#91103 (Inhibit clicks on summary's children) - rust-lang#91137 (Give people a single link they can click in the contributing guide) - rust-lang#91140 (Split inline const to two feature gates and mark expression position inline const complete) - rust-lang#91148 (Use `derive_default_enum` in the compiler) - rust-lang#91153 (kernel_copy: avoid panic on unexpected OS error) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

…eGomez Fix toggle-click-deadspace rustdoc-gui test In rust-lang#91103 I introduced a rustdoc-gui test for clicks on toggles. I introduced some documentation on a method in lib2/struct.Foo.html so there would be something to toggle, but accidentally left the test checking test_docs/struct.Foo.html. That caused the test to reliably fail. I'm not sure how that test got past GitHub Actions and bors, but it's manifesting in test failures at rust-lang#91062 (comment) and rust-lang#91170 (comment). This fixes by pointing at the right file. r? `@GuillaumeGomez`

jsha · 2021-11-21T08:48:39Z

A byproduct of using <details> and <summary> to show/hide detailed documentation was that clicking any part of a method heading (or impl heading) would show or hide the documentation. This was not super noticeable because clicking a link inside the method heading would navigate to that link. But clicking any unlinked black text in a method heading would trigger the behavior.

That behavior was somewhat unexpected, and means that if you try to click a type name in a method heading, but miss by a few pixels, you get a confusing surprise.

This change inhibits that behavior by putting an event listener on most summaries that cancels the event unless the event target was the summary itself. In practice, that means it cancels the event unless the target was the "[+]" / "[-]", because the rest of the heading is wrapped inside a <div>, which is the target for anything that doesn't have a more specific target.

r? @Manishearth

rust-highfive · 2021-11-21T08:48:41Z

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez

A byproduct of using `<details>` and `<summary>` to show/hide detailed documentation was that clicking any part of a method heading (or impl heading) would show or hide the documentation. This was not super noticeable because clicking a link inside the method heading would navigate to that link. But clicking any unlinked black text in a method heading would trigger the behavior. That behavior was somewhat unexpected, and means that if you try to click a type name in a method heading, but miss by a few pixels, you get a confusing surprise. This change inhibits that behavior by putting an event listener on most summaries that cancels the event unless the event target was the summary itself. In practice, that means it cancels the event unless the target was the "[+]" / "[-]", because the rest of the heading is wrapped inside a `<div>`, which is the target for anything that doesn't have a more specific target.

GuillaumeGomez · 2021-11-21T13:53:30Z

Please add a GUI test, otherwise it seems like a good idea. It often happens that when I highlight something, it closes the the doc, which is a bit annoying.

GuillaumeGomez · 2021-11-22T08:55:16Z

You can click directly on h4.code-header. I'd much prefer avoiding to use hard coded positions if possible. :)

GuillaumeGomez · 2021-11-22T08:55:23Z

GuillaumeGomez · 2021-11-22T08:55:39Z

Once you're done, please remove this one. ;)

jsha · 2021-11-22T09:06:41Z

I wrote a GUI test, but I couldn't get it to work. I tried to find the coordinates on which to simulate a click by loading the page in a regular browser, but when I tell the tester to click on the coordinates that correspond to the toggle, it doesn't appear to do anything. I tried using --no-headless but the browser window appears and disappears instantly. Any ideas how to debug?

GuillaumeGomez · 2021-11-22T09:10:36Z

Ah right, for that I use wait-for: 600000 (for 10 min). It allows to keep the windows around to debug in peace.

GuillaumeGomez · 2021-11-22T09:15:18Z

Looks good to me, one nit remaining but can be ignored.

GuillaumeGomez · 2021-11-22T09:21:10Z

Thanks! r=me once CI pass

jsha · 2021-11-22T19:01:42Z

@bors try

bors · 2021-11-22T19:01:56Z

⌛ Trying commit 7f35556 with merge 057c2213acefad12356c4f990ecff30c5b32bdf6...

GuillaumeGomez · 2021-11-22T19:04:19Z

Why the try?

jsha · 2021-11-22T19:56:53Z

Perhaps I misread the bors docs? Was attempting to trigger a normal re-run of the tests, since I can't reproduce the failure locally. And the CI status reported here doesn't show a failure, just a cancelled check? https://bors.tech/documentation/

GuillaumeGomez · 2021-11-22T20:00:49Z

It's easier this way. ;)

jsha · 2021-11-23T00:09:24Z

@bors r=Manishearth,GuillaumeGomez

bors · 2021-11-23T00:09:26Z

📌 Commit 7f35556 has been approved by Manishearth,GuillaumeGomez

…, r=Manishearth,GuillaumeGomez Inhibit clicks on summary's children A byproduct of using `<details>` and `<summary>` to show/hide detailed documentation was that clicking any part of a method heading (or impl heading) would show or hide the documentation. This was not super noticeable because clicking a link inside the method heading would navigate to that link. But clicking any unlinked black text in a method heading would trigger the behavior. That behavior was somewhat unexpected, and means that if you try to click a type name in a method heading, but miss by a few pixels, you get a confusing surprise. This change inhibits that behavior by putting an event listener on most summaries that cancels the event unless the event target was the summary itself. In practice, that means it cancels the event unless the target was the "[+]" / "[-]", because the rest of the heading is wrapped inside a `<div>`, which is the target for anything that doesn't have a more specific target. r? `@Manishearth`

…askrgr Rollup of 6 pull requests Successful merges: - rust-lang#90856 (Suggestion to wrap inner types using 'allocator_api' in tuple) - rust-lang#91103 (Inhibit clicks on summary's children) - rust-lang#91137 (Give people a single link they can click in the contributing guide) - rust-lang#91140 (Split inline const to two feature gates and mark expression position inline const complete) - rust-lang#91148 (Use `derive_default_enum` in the compiler) - rust-lang#91153 (kernel_copy: avoid panic on unexpected OS error) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

…eGomez Fix toggle-click-deadspace rustdoc-gui test In rust-lang#91103 I introduced a rustdoc-gui test for clicks on toggles. I introduced some documentation on a method in lib2/struct.Foo.html so there would be something to toggle, but accidentally left the test checking test_docs/struct.Foo.html. That caused the test to reliably fail. I'm not sure how that test got past GitHub Actions and bors, but it's manifesting in test failures at rust-lang#91062 (comment) and rust-lang#91170 (comment). This fixes by pointing at the right file. r? `@GuillaumeGomez`

jsha · 2021-11-21T08:48:39Z

A byproduct of using <details> and <summary> to show/hide detailed documentation was that clicking any part of a method heading (or impl heading) would show or hide the documentation. This was not super noticeable because clicking a link inside the method heading would navigate to that link. But clicking any unlinked black text in a method heading would trigger the behavior.

That behavior was somewhat unexpected, and means that if you try to click a type name in a method heading, but miss by a few pixels, you get a confusing surprise.

This change inhibits that behavior by putting an event listener on most summaries that cancels the event unless the event target was the summary itself. In practice, that means it cancels the event unless the target was the "[+]" / "[-]", because the rest of the heading is wrapped inside a <div>, which is the target for anything that doesn't have a more specific target.

r? @Manishearth

rust-highfive · 2021-11-21T08:48:41Z

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez

A byproduct of using `<details>` and `<summary>` to show/hide detailed documentation was that clicking any part of a method heading (or impl heading) would show or hide the documentation. This was not super noticeable because clicking a link inside the method heading would navigate to that link. But clicking any unlinked black text in a method heading would trigger the behavior. That behavior was somewhat unexpected, and means that if you try to click a type name in a method heading, but miss by a few pixels, you get a confusing surprise. This change inhibits that behavior by putting an event listener on most summaries that cancels the event unless the event target was the summary itself. In practice, that means it cancels the event unless the target was the "[+]" / "[-]", because the rest of the heading is wrapped inside a `<div>`, which is the target for anything that doesn't have a more specific target.

GuillaumeGomez · 2021-11-21T13:53:30Z

Please add a GUI test, otherwise it seems like a good idea. It often happens that when I highlight something, it closes the the doc, which is a bit annoying.

GuillaumeGomez · 2021-11-22T08:55:16Z

You can click directly on h4.code-header. I'd much prefer avoiding to use hard coded positions if possible. :)

GuillaumeGomez · 2021-11-22T08:55:23Z

GuillaumeGomez · 2021-11-22T08:55:39Z

Once you're done, please remove this one. ;)

jsha · 2021-11-22T09:06:41Z

I wrote a GUI test, but I couldn't get it to work. I tried to find the coordinates on which to simulate a click by loading the page in a regular browser, but when I tell the tester to click on the coordinates that correspond to the toggle, it doesn't appear to do anything. I tried using --no-headless but the browser window appears and disappears instantly. Any ideas how to debug?

GuillaumeGomez · 2021-11-22T09:10:36Z

Ah right, for that I use wait-for: 600000 (for 10 min). It allows to keep the windows around to debug in peace.

GuillaumeGomez · 2021-11-22T09:15:18Z

Looks good to me, one nit remaining but can be ignored.

GuillaumeGomez · 2021-11-22T09:21:10Z

Thanks! r=me once CI pass

jsha · 2021-11-22T19:01:42Z

@bors try

bors · 2021-11-22T19:01:56Z

⌛ Trying commit 7f35556 with merge 057c2213acefad12356c4f990ecff30c5b32bdf6...

GuillaumeGomez · 2021-11-22T19:04:19Z

Why the try?

jsha · 2021-11-22T19:56:53Z

Perhaps I misread the bors docs? Was attempting to trigger a normal re-run of the tests, since I can't reproduce the failure locally. And the CI status reported here doesn't show a failure, just a cancelled check? https://bors.tech/documentation/

GuillaumeGomez · 2021-11-22T20:00:49Z

It's easier this way. ;)

jsha · 2021-11-23T00:09:24Z

@bors r=Manishearth,GuillaumeGomez

bors · 2021-11-23T00:09:26Z

📌 Commit 7f35556 has been approved by Manishearth,GuillaumeGomez

…, r=Manishearth,GuillaumeGomez Inhibit clicks on summary's children A byproduct of using `<details>` and `<summary>` to show/hide detailed documentation was that clicking any part of a method heading (or impl heading) would show or hide the documentation. This was not super noticeable because clicking a link inside the method heading would navigate to that link. But clicking any unlinked black text in a method heading would trigger the behavior. That behavior was somewhat unexpected, and means that if you try to click a type name in a method heading, but miss by a few pixels, you get a confusing surprise. This change inhibits that behavior by putting an event listener on most summaries that cancels the event unless the event target was the summary itself. In practice, that means it cancels the event unless the target was the "[+]" / "[-]", because the rest of the heading is wrapped inside a `<div>`, which is the target for anything that doesn't have a more specific target. r? `@Manishearth`

…askrgr Rollup of 6 pull requests Successful merges: - rust-lang#90856 (Suggestion to wrap inner types using 'allocator_api' in tuple) - rust-lang#91103 (Inhibit clicks on summary's children) - rust-lang#91137 (Give people a single link they can click in the contributing guide) - rust-lang#91140 (Split inline const to two feature gates and mark expression position inline const complete) - rust-lang#91148 (Use `derive_default_enum` in the compiler) - rust-lang#91153 (kernel_copy: avoid panic on unexpected OS error) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

…eGomez Fix toggle-click-deadspace rustdoc-gui test In rust-lang#91103 I introduced a rustdoc-gui test for clicks on toggles. I introduced some documentation on a method in lib2/struct.Foo.html so there would be something to toggle, but accidentally left the test checking test_docs/struct.Foo.html. That caused the test to reliably fail. I'm not sure how that test got past GitHub Actions and bors, but it's manifesting in test failures at rust-lang#91062 (comment) and rust-lang#91170 (comment). This fixes by pointing at the right file. r? `@GuillaumeGomez`

jsha added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-rustdoc-ui Area: Rustdoc UI (generated HTML) labels Nov 21, 2021

rust-highfive assigned Manishearth Nov 21, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 21, 2021

jsha force-pushed the non-toggle-click-doesnt-toggle branch from bd1b456 to 9aef9a2Compare November 21, 2021 08:53

This comment has been minimized.
Sign in to view

GuillaumeGomez reviewed Nov 22, 2021
View reviewed changes

Comment threadsrc/test/rustdoc-gui/toggle-click-deadspace.goml Outdated
GuillaumeGomezNov 22, 2021
Copy link
Copy Markdown
Member
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.

GuillaumeGomez reviewed Nov 22, 2021
View reviewed changes

Comment threadsrc/test/rustdoc-gui/toggle-click-deadspace.goml Outdated
GuillaumeGomezNov 22, 2021
Copy link
Copy Markdown
Member
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once you're done, please remove this one. ;)

jsha force-pushed the non-toggle-click-doesnt-toggle branch from fa0e2c3 to 3f3a7a2Compare November 22, 2021 09:10

GuillaumeGomez reviewed Nov 22, 2021
View reviewed changes

Comment threadsrc/test/rustdoc-gui/toggle-click-deadspace.goml Outdated

GuillaumeGomez approved these changes Nov 22, 2021
View reviewed changes

Add GUI test for clicking on non-toggle summary
7f35556

jsha force-pushed the non-toggle-click-doesnt-toggle branch from 3f3a7a2 to 7f35556Compare November 22, 2021 09:17

GuillaumeGomez approved these changes Nov 22, 2021
View reviewed changes

GuillaumeGomez closed this Nov 22, 2021

GuillaumeGomez reopened this Nov 22, 2021

Manishearth approved these changes Nov 22, 2021
View reviewed changes

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 Nov 23, 2021

matthiaskrgr mentioned this pull request Nov 23, 2021
Rollup of 7 pull requests #91157
Closed

matthiaskrgr mentioned this pull request Nov 23, 2021
Rollup of 6 pull requests #91159
Merged

bors merged commit eb5c2d3 into rust-lang:masterNov 23, 2021

rustbot added this to the 1.58.0 milestone Nov 23, 2021

jsha mentioned this pull request Nov 24, 2021
Fix toggle-click-deadspace rustdoc-gui test #91171
Merged

jyn514 mentioned this pull request Nov 24, 2021
rustdoc: Can't click any links in methods that use details#91184
Closed