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

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

Merged
merged 4 commits into from
Aug 12, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a282e52
Unify run button display with "copy code" button and with mdbook buttons
GuillaumeGomez Jul 30, 2024
be71bd9
Update GUI tests for code example buttons
GuillaumeGomez Jul 30, 2024
e2da2fb
Update rustdoc tests
GuillaumeGomez Jul 30, 2024
59cb159
Prevent clicking on a link or on a button to toggle the code example …
GuillaumeGomez Jul 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/librustdoc/html/markdown.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,8 @@ impl<'a, I: Iterator<Item = Event<'a>>> Iterator for CodeBlocks<'_, 'a, I> {
Some(format!(
"<a class=\"test-arrow\" \
target=\"_blank\" \
href=\"{url}?code={test_escaped}{channel}&amp;edition={edition}\">Run</a>",
title=\"Run code\" \
href=\"{url}?code={test_escaped}{channel}&amp;edition={edition}\"></a>",
))
});

Expand Down
8 changes: 0 additions & 8 deletions src/librustdoc/html/static/css/noscript.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,6 @@ nav.sub {
--code-highlight-doc-comment-color: #4d4d4c;
--src-line-numbers-span-color: #c67e2d;
--src-line-number-highlighted-background-color: #fdffd3;
--test-arrow-color: #f5f5f5;
--test-arrow-background-color: rgba(78, 139, 202, 0.2);
--test-arrow-hover-color: #f5f5f5;
--test-arrow-hover-background-color: rgb(78, 139, 202);
--target-background-color: #fdffd3;
--target-border-color: #ad7c37;
--kbd-color: #000;
Expand Down Expand Up @@ -210,10 +206,6 @@ nav.sub {
--code-highlight-doc-comment-color: #8ca375;
--src-line-numbers-span-color: #3b91e2;
--src-line-number-highlighted-background-color: #0a042f;
--test-arrow-color: #dedede;
--test-arrow-background-color: rgba(78, 139, 202, 0.2);
--test-arrow-hover-color: #dedede;
--test-arrow-hover-background-color: #4e8bca;
--target-background-color: #494a3d;
--target-border-color: #bb7410;
--kbd-color: #000;
Expand Down
54 changes: 21 additions & 33 deletions src/librustdoc/html/static/css/rustdoc.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ details:not(.toggle) summary {
margin-bottom: .6em;
}

code, pre, a.test-arrow, .code-header {
code, pre, .code-header {
font-family: "Source Code Pro", monospace;
}
.docblock code, .docblock-short code {
Expand Down Expand Up @@ -946,8 +946,8 @@ because of the `[-]` element which would overlap with it. */
.main-heading a:hover,
.example-wrap .rust a:hover,
.all-items a:hover,
.docblock a:not(.test-arrow):not(.scrape-help):not(.tooltip):hover:not(.doc-anchor),
.docblock-short a:not(.test-arrow):not(.scrape-help):not(.tooltip):hover,
.docblock a:not(.scrape-help):not(.tooltip):hover:not(.doc-anchor),
.docblock-short a:not(.scrape-help):not(.tooltip):hover,
.item-info a {
text-decoration: underline;
}
Expand Down Expand Up @@ -1461,22 +1461,17 @@ documentation. */
z-index: 1;
}
a.test-arrow {
padding: 5px 7px;
border-radius: var(--button-border-radius);
font-size: 1rem;
color: var(--test-arrow-color);
background-color: var(--test-arrow-background-color);
height: var(--copy-path-height);
padding: 6px 4px 0 11px;
}
a.test-arrow:hover {
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" \
Copy link
Member

Choose a reason for hiding this comment

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

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" \

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

@notriddle notriddle Jul 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Member

@camelid camelid Jul 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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.

xmlns="http://www.w3.org/2000/svg"><path d="M0 0l18 10-18 10z"/></svg>');
}
.example-wrap .button-holder {
display: flex;
}
.example-wrap:hover > .test-arrow {
padding: 2px 7px;
}

/*
On iPad, the ":hover" state sticks around, making things work not greatly. Do work around
it, we move it into this media query. More information can be found at:
Expand All @@ -1486,29 +1481,34 @@ However, using `@media (hover: hover)` makes this rule never to be applied in GU
instead, we check that it's not a "finger" cursor.
*/
@media not (pointer: coarse) {
.example-wrap:hover > .test-arrow, .example-wrap:hover > .button-holder {
.example-wrap:hover > a.test-arrow, .example-wrap:hover > .button-holder {
visibility: visible;
}
}
.example-wrap .button-holder.keep-visible {
visibility: visible;
}
.example-wrap .button-holder .copy-button {
color: var(--copy-path-button-color);
.example-wrap .button-holder .copy-button, .example-wrap .test-arrow {
background: var(--main-background-color);
cursor: pointer;
border-radius: var(--button-border-radius);
height: var(--copy-path-height);
width: var(--copy-path-width);
}
.example-wrap .button-holder .copy-button {
margin-left: var(--button-left-margin);
padding: 2px 0 0 4px;
border: 0;
cursor: pointer;
border-radius: var(--button-border-radius);
}
.example-wrap .button-holder .copy-button::before {
.example-wrap .button-holder .copy-button::before,
.example-wrap .test-arrow::before {
filter: var(--copy-path-img-filter);
}
.example-wrap .button-holder .copy-button::before {
content: var(--clipboard-image);
}
.example-wrap .button-holder .copy-button:hover::before {
.example-wrap .button-holder .copy-button:hover::before,
.example-wrap .test-arrow:hover::before {
filter: var(--copy-path-img-hover-filter);
}
.example-wrap .button-holder .copy-button.clicked::before {
Expand Down Expand Up @@ -2552,10 +2552,6 @@ by default.
--code-highlight-doc-comment-color: #4d4d4c;
--src-line-numbers-span-color: #c67e2d;
--src-line-number-highlighted-background-color: #fdffd3;
--test-arrow-color: #f5f5f5;
--test-arrow-background-color: rgba(78, 139, 202, 0.2);
--test-arrow-hover-color: #f5f5f5;
--test-arrow-hover-background-color: rgb(78, 139, 202);
--target-background-color: #fdffd3;
--target-border-color: #ad7c37;
--kbd-color: #000;
Expand Down Expand Up @@ -2658,10 +2654,6 @@ by default.
--code-highlight-doc-comment-color: #8ca375;
--src-line-numbers-span-color: #3b91e2;
--src-line-number-highlighted-background-color: #0a042f;
--test-arrow-color: #dedede;
--test-arrow-background-color: rgba(78, 139, 202, 0.2);
--test-arrow-hover-color: #dedede;
--test-arrow-hover-background-color: #4e8bca;
--target-background-color: #494a3d;
--target-border-color: #bb7410;
--kbd-color: #000;
Expand Down Expand Up @@ -2771,10 +2763,6 @@ Original by Dempfi (https://github.com/dempfi/ayu)
--code-highlight-doc-comment-color: #a1ac88;
--src-line-numbers-span-color: #5c6773;
--src-line-number-highlighted-background-color: rgba(255, 236, 164, 0.06);
--test-arrow-color: #788797;
--test-arrow-background-color: rgba(57, 175, 215, 0.09);
--test-arrow-hover-color: #c5c5c5;
--test-arrow-hover-background-color: rgba(57, 175, 215, 0.368);
--target-background-color: rgba(255, 236, 164, 0.06);
--target-border-color: rgba(255, 180, 76, 0.85);
--kbd-color: #c5c5c5;
Expand Down
8 changes: 6 additions & 2 deletions src/librustdoc/html/static/js/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1829,10 +1829,14 @@ href="https://doc.rust-lang.org/${channel}/rustdoc/read-documentation/search.htm
function getExampleWrap(event) {
let elem = event.target;
while (!hasClass(elem, "example-wrap")) {
elem = elem.parentElement;
if (elem === document.body || hasClass(elem, "docblock")) {
if (elem === document.body ||
elem.tagName === "A" ||
elem.tagName === "BUTTON" ||
hasClass(elem, "docblock")
) {
return null;
}
elem = elem.parentElement;
}
return elem;
}
Expand Down
75 changes: 75 additions & 0 deletions tests/rustdoc-gui/code-example-buttons.goml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// This test ensures that code blocks buttons are displayed on hover and when you click on them.
go-to: "file://" + |DOC_PATH| + "/test_docs/fn.foo.html"
include: "utils.goml"

// First we check we "hover".
move-cursor-to: ".example-wrap"
Expand All @@ -19,3 +20,77 @@ click: ".example-wrap"
move-cursor-to: ".search-input"
assert-count: (".example-wrap:not(:hover) .button-holder.keep-visible", 0)
assert-css: (".example-wrap .copy-button", { "visibility": "hidden" })

// Clicking on the "copy code" button shouldn't make the buttons stick.
click: ".example-wrap .copy-button"
move-cursor-to: ".search-input"
assert-count: (".example-wrap:not(:hover) .button-holder.keep-visible", 0)
assert-css: (".example-wrap .copy-button", { "visibility": "hidden" })

define-function: (
"check-buttons",
[theme, background, filter, filter_hover],
block {
call-function: ("switch-theme", {"theme": |theme|})

assert-css: (".example-wrap .test-arrow", {"visibility": "hidden"})
assert-css: (".example-wrap .copy-button", {"visibility": "hidden"})

move-cursor-to: ".example-wrap"
assert-css: (".example-wrap .test-arrow", {
"visibility": "visible",
"background-color": |background|,
"border-radius": "2px",
})
assert-css: (".example-wrap .test-arrow::before", {
"filter": |filter|,
})
assert-css: (".example-wrap .copy-button", {
"visibility": "visible",
"background-color": |background|,
"border-radius": "2px",
})
assert-css: (".example-wrap .copy-button::before", {
"filter": |filter|,
})

move-cursor-to: ".example-wrap .test-arrow"
assert-css: (".example-wrap .test-arrow:hover", {
"visibility": "visible",
"background-color": |background|,
"border-radius": "2px",
})
assert-css: (".example-wrap .test-arrow:hover::before", {
"filter": |filter_hover|,
})

move-cursor-to: ".example-wrap .copy-button"
assert-css: (".example-wrap .copy-button:hover", {
"visibility": "visible",
"background-color": |background|,
"border-radius": "2px",
})
assert-css: (".example-wrap .copy-button:hover::before", {
"filter": |filter_hover|,
})
},
)

call-function: ("check-buttons",{
"theme": "ayu",
"background": "#0f1419",
"filter": "invert(0.7)",
"filter_hover": "invert(1)",
})
call-function: ("check-buttons",{
"theme": "dark",
"background": "#353535",
"filter": "invert(0.5)",
"filter_hover": "invert(0.65)",
})
call-function: ("check-buttons",{
"theme": "light",
"background": "#fff",
"filter": "invert(0.5)",
"filter_hover": "invert(0.35)",
})
4 changes: 2 additions & 2 deletions tests/rustdoc-gui/copy-code.goml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@ define-function: (
)

call-function: ("check-copy-button", {})
// Checking that the run button and the copy button have the same height.
// Checking that the run button and the copy button have the same height and same width.
compare-elements-size: (
".example-wrap:nth-of-type(1) .test-arrow",
".example-wrap:nth-of-type(1) .copy-button",
["height"],
["height", "width"],
)
// ... and the same y position.
compare-elements-position: (
Expand Down
54 changes: 0 additions & 54 deletions tests/rustdoc-gui/run-on-hover.goml
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion tests/rustdoc/playground-arg.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,4 @@
pub fn dummy() {}

// ensure that `extern crate foo;` was inserted into code snips automatically:
//@ matches foo/index.html '//a[@class="test-arrow"][@href="https://example.com/?code=%23!%5Ballow(unused)%5D%0A%23%5Ballow(unused_extern_crates)%5D%0Aextern+crate+r%23foo;%0Afn+main()+%7B%0A++++use+foo::dummy;%0A++++dummy();%0A%7D&edition=2015"]' "Run"
//@ matches foo/index.html '//a[@class="test-arrow"][@href="https://example.com/?code=%23!%5Ballow(unused)%5D%0A%23%5Ballow(unused_extern_crates)%5D%0Aextern+crate+r%23foo;%0Afn+main()+%7B%0A++++use+foo::dummy;%0A++++dummy();%0A%7D&edition=2015"]' ""
2 changes: 1 addition & 1 deletion tests/rustdoc/playground-syntax-error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,5 @@
pub fn bar() {}

//@ has foo/fn.bar.html
//@ has - '//a[@class="test-arrow"]' "Run"
//@ has - '//a[@class="test-arrow"]' ""
//@ has - '//*[@class="docblock"]' 'foo_recursive'
6 changes: 3 additions & 3 deletions tests/rustdoc/playground.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,6 @@
//! }
//! ```

//@ matches foo/index.html '//a[@class="test-arrow"][@href="https://www.example.com/?code=%23!%5Ballow(unused)%5D%0Afn+main()+%7B%0A++++println!(%22Hello,+world!%22);%0A%7D&edition=2015"]' "Run"
//@ matches foo/index.html '//a[@class="test-arrow"][@href="https://www.example.com/?code=%23!%5Ballow(unused)%5D%0Afn+main()+%7B%0A++++println!(%22Hello,+world!%22);%0A%7D&edition=2015"]' "Run"
//@ matches foo/index.html '//a[@class="test-arrow"][@href="https://www.example.com/?code=%23!%5Ballow(unused)%5D%0A%23!%5Bfeature(something)%5D%0A%0Afn+main()+%7B%0A++++println!(%22Hello,+world!%22);%0A%7D&version=nightly&edition=2015"]' "Run"
//@ matches foo/index.html '//a[@class="test-arrow"][@href="https://www.example.com/?code=%23!%5Ballow(unused)%5D%0Afn+main()+%7B%0A++++println!(%22Hello,+world!%22);%0A%7D&edition=2015"]' ""
//@ matches foo/index.html '//a[@class="test-arrow"][@href="https://www.example.com/?code=%23!%5Ballow(unused)%5D%0Afn+main()+%7B%0A++++println!(%22Hello,+world!%22);%0A%7D&edition=2015"]' ""
//@ matches foo/index.html '//a[@class="test-arrow"][@href="https://www.example.com/?code=%23!%5Ballow(unused)%5D%0A%23!%5Bfeature(something)%5D%0A%0Afn+main()+%7B%0A++++println!(%22Hello,+world!%22);%0A%7D&version=nightly&edition=2015"]' ""
Loading