Unify run button display with "copy code" button and with mdbook buttons #128394

GuillaumeGomez · 2024-07-30T13:29:18Z

Follow-up of #128339.

It looks like this (coherency++, yeay!):

Can be tested here.

r? @notriddle

rustbot · 2024-07-30T13:29:27Z

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez, @jsha

GuillaumeGomez · 2024-07-30T16:00:37Z

This time it seems like I updated all the tests correctly. :D

notriddle · 2024-07-30T18:19:19Z

Okay, one thing I noticed while poking around the demo that I wish I'd noticed last time: clicking the Copy button does the same thing as clicking the text box itself, so it toggles the visibility lock. It probably shouldn't do that.

If you're using a desktop mouse, you hover over the text box, the buttons appear, and you click the Copy button. The buttons are now in "sticky" mode and don't go away, even when the mouse leaves the box.

If you're using a touchscreen, you tap the box and the buttons appear, then you click the Copy button and it instantly vanishes, so you never see the confirmation checkmark.

GuillaumeGomez · 2024-07-30T18:42:52Z

Oh that's a great point! Fixing that and adding a GUI test for it.

GuillaumeGomez · 2024-07-30T18:58:38Z

Done and also updated online demo.

notriddle · 2024-07-30T19:03:51Z

@rfcbot fcp merge

notriddle · 2024-07-30T19:05:25Z

@rfcbot fcp merge

rfcbot · 2024-07-30T19:05:32Z

Team member @notriddle has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

camelid

@rfcbot reviewed

I'd like us to use the mdBook copy icon too, but this is a very good start in any case. I left comments for changes I'd like on the details.

camelid · 2024-07-31T04:48:32Z

src/librustdoc/html/static/css/rustdoc.css

-	color: var(--test-arrow-hover-color);
-	background-color: var(--test-arrow-hover-background-color);
+a.test-arrow::before {
+	content: url('data:image/svg+xml,<svg viewBox="0 0 20 20" width="18" height="20" \


It feels a little large relative to the copy button; what do you think about this? It looks more proportional to my eye. Other code might need to be updated to fix the baseline position too.

Suggested change

content: url('data:image/svg+xml,<svg viewBox="0 0 20 20" width="18" height="20" \

content: url('data:image/svg+xml,<svg viewBox="0 0 20 20" width="15" height="15" \

Also would it be better to move this to a CSS variable like the clipboard icon and others?

It's only used in one place, so not sure it would be a good idea. Since you suggested to change the image for the copy button, I'll resize when I'll update it.

I’m not sure changing the copy icon is a good idea. mdbook might be using a different one, but crates.io and docs.rs use this one.

Then I'll send a PR to mdBook. ;)

I’m not sure changing the copy icon is a good idea. mdbook might be using a different one, but crates.io and docs.rs use this one.

Good point, I didn't notice that. I do think mdBook's version (which is from Font Awesome) is more common in UIs generally than the rustdoc/crates.io/docs.rs one, but we should definitely be consistent, and it sounds like mdBook is the one out of line here.

camelid · 2024-07-31T04:56:11Z

Oh, I should clarify that my approval was meant for FCP purposes. I didn't review the code in detail since I think notriddle already did that :)

rfcbot · 2024-07-31T04:56:26Z

🔔 This is now entering its final comment period, as per the review above. 🔔

GuillaumeGomez · 2024-08-01T15:45:34Z

I opened the PR to unify mdBook "copy code" button with rustdoc, docs.rs and crates.io in rust-lang/mdBook#2421.

bors · 2024-08-04T00:35:19Z

☔ The latest upstream changes (presumably #128614) made this pull request unmergeable. Please resolve the merge conflicts.

…buttons visibility

rfcbot · 2024-08-11T18:26:21Z

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

GuillaumeGomez · 2024-08-12T08:19:21Z

@bors r=t-rustdoc

bors · 2024-08-12T08:19:23Z

📌 Commit 59cb159 has been approved by t-rustdoc

It is now in the queue for this repository.

…tdoc Unify run button display with "copy code" button and with mdbook buttons Follow-up of rust-lang#128339. It looks like this (coherency++, yeay!): ![Screenshot from 2024-07-30 15-16-31](https://github.com/user-attachments/assets/5e262e5b-f338-4085-94ca-e223033a43db) Can be tested [here](https://rustdoc.crud.net/imperio/run-button/foo/struct.Bar.html). r? `@notriddle`

…llaumeGomez Rollup of 7 pull requests Successful merges: - rust-lang#126245 (Greatly speed up doctests by compiling compatible doctests in one file) - rust-lang#128394 (Unify run button display with "copy code" button and with mdbook buttons) - rust-lang#128878 (Slightly refactor `Flags` in bootstrap) - rust-lang#128886 (Get rid of some `#[allow(rustc::untranslatable_diagnostic)]`) - rust-lang#128929 (Fix codegen-units tests that were disabled 8 years ago) - rust-lang#128967 (std::fs::get_path freebsd update.) - rust-lang#128978 (Use `assert_matches` around the compiler more) r? `@ghost` `@rustbot` modify labels: rollup

…llaumeGomez Rollup of 10 pull requests Successful merges: - rust-lang#128149 (nontemporal_store: make sure that the intrinsic is truly just a hint) - rust-lang#128394 (Unify run button display with "copy code" button and with mdbook buttons) - rust-lang#128537 (const vector passed through to codegen) - rust-lang#128632 (std: do not overwrite style in `get_backtrace_style`) - rust-lang#128878 (Slightly refactor `Flags` in bootstrap) - rust-lang#128886 (Get rid of some `#[allow(rustc::untranslatable_diagnostic)]`) - rust-lang#128929 (Fix codegen-units tests that were disabled 8 years ago) - rust-lang#128937 (Fix warnings in rmake tests on `x86_64-unknown-linux-gnu`) - rust-lang#128978 (Use `assert_matches` around the compiler more) - rust-lang#128994 (Fix bug in `Parser::look_ahead`.) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#128394 - GuillaumeGomez:run-button, r=t-rustdoc Unify run button display with "copy code" button and with mdbook buttons Follow-up of rust-lang#128339. It looks like this (coherency++, yeay!): ![Screenshot from 2024-07-30 15-16-31](https://github.com/user-attachments/assets/5e262e5b-f338-4085-94ca-e223033a43db) Can be tested [here](https://rustdoc.crud.net/imperio/run-button/foo/struct.Bar.html). r? `@notriddle`

rustbot assigned notriddle Jul 30, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Jul 30, 2024

This comment has been minimized.

Sign in to view

GuillaumeGomez force-pushed the run-button branch from f607557 to 5b5fd94 Compare July 30, 2024 13:41

This comment has been minimized.

Sign in to view

This comment was marked as outdated.

Sign in to view

rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Jul 30, 2024

notriddle added T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output. and removed T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Jul 30, 2024

This comment was marked as outdated.

Sign in to view

rfcbot removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Jul 30, 2024

rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Jul 30, 2024

This comment has been minimized.

Sign in to view

GuillaumeGomez force-pushed the run-button branch from 68c2b05 to 27dd6d3 Compare July 30, 2024 20:09

camelid approved these changes Jul 31, 2024

View reviewed changes

rfcbot added the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label Jul 31, 2024

rfcbot removed the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Jul 31, 2024

GuillaumeGomez added 4 commits August 5, 2024 11:04

Unify run button display with "copy code" button and with mdbook buttons

a282e52

Update GUI tests for code example buttons

be71bd9

Update rustdoc tests

e2da2fb

Prevent clicking on a link or on a button to toggle the code example …

59cb159

…buttons visibility

GuillaumeGomez force-pushed the run-button branch from 27dd6d3 to 59cb159 Compare August 5, 2024 09:06

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 Aug 12, 2024

GuillaumeGomez mentioned this pull request Aug 12, 2024

Rollup of 7 pull requests #129006

Closed

GuillaumeGomez mentioned this pull request Aug 12, 2024

Rollup of 10 pull requests #129008

Merged

bors merged commit c6e3385 into rust-lang:master Aug 12, 2024
6 checks passed

rustbot added this to the 1.82.0 milestone Aug 12, 2024

GuillaumeGomez deleted the run-button branch August 12, 2024 18:58

apiraino removed the to-announce Announce this issue on triage meeting label Aug 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unify run button display with "copy code" button and with mdbook buttons #128394

Unify run button display with "copy code" button and with mdbook buttons #128394

GuillaumeGomez commented Jul 30, 2024

rustbot commented Jul 30, 2024

This comment has been minimized.

This comment has been minimized.

GuillaumeGomez commented Jul 30, 2024

notriddle commented Jul 30, 2024

GuillaumeGomez commented Jul 30, 2024

GuillaumeGomez commented Jul 30, 2024

notriddle commented Jul 30, 2024

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

notriddle commented Jul 30, 2024

rfcbot commented Jul 30, 2024 •

edited by camelid

Loading

This comment has been minimized.

camelid left a comment

camelid Jul 31, 2024

camelid Jul 31, 2024

GuillaumeGomez Jul 31, 2024

notriddle Jul 31, 2024 •

edited

Loading

GuillaumeGomez Jul 31, 2024

camelid Jul 31, 2024 •

edited

Loading

camelid commented Jul 31, 2024

rfcbot commented Jul 31, 2024

GuillaumeGomez commented Aug 1, 2024

bors commented Aug 4, 2024

rfcbot commented Aug 11, 2024

GuillaumeGomez commented Aug 12, 2024

bors commented Aug 12, 2024

	content: url('data:image/svg+xml,<svg viewBox="0 0 20 20" width="18" height="20" \
	content: url('data:image/svg+xml,<svg viewBox="0 0 20 20" width="15" height="15" \

Unify run button display with "copy code" button and with mdbook buttons #128394

Unify run button display with "copy code" button and with mdbook buttons #128394

Conversation

GuillaumeGomez commented Jul 30, 2024

rustbot commented Jul 30, 2024

This comment has been minimized.

This comment has been minimized.

GuillaumeGomez commented Jul 30, 2024

notriddle commented Jul 30, 2024

GuillaumeGomez commented Jul 30, 2024

GuillaumeGomez commented Jul 30, 2024

notriddle commented Jul 30, 2024

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

notriddle commented Jul 30, 2024

rfcbot commented Jul 30, 2024 • edited by camelid Loading

This comment has been minimized.

camelid left a comment

Choose a reason for hiding this comment

camelid Jul 31, 2024

Choose a reason for hiding this comment

camelid Jul 31, 2024

Choose a reason for hiding this comment

GuillaumeGomez Jul 31, 2024

Choose a reason for hiding this comment

notriddle Jul 31, 2024 • edited Loading

Choose a reason for hiding this comment

GuillaumeGomez Jul 31, 2024

Choose a reason for hiding this comment

camelid Jul 31, 2024 • edited Loading

Choose a reason for hiding this comment

camelid commented Jul 31, 2024

rfcbot commented Jul 31, 2024

GuillaumeGomez commented Aug 1, 2024

bors commented Aug 4, 2024

rfcbot commented Aug 11, 2024

GuillaumeGomez commented Aug 12, 2024

bors commented Aug 12, 2024

rfcbot commented Jul 30, 2024 •

edited by camelid

Loading

notriddle Jul 31, 2024 •

edited

Loading

camelid Jul 31, 2024 •

edited

Loading