-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Add zoomable images feature #3127
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
Changes from all commits
71fb5a8
9de2755
6a7ef4d
73d6edf
f526b18
588066c
beab2fa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,7 +9,7 @@ use super::tokenizer::parse_html; | |
| use super::{HtmlRenderOptions, hide_lines, wrap_rust_main}; | ||
| use crate::utils::{id_from_content, unique_id}; | ||
| use ego_tree::{NodeId, NodeRef, Tree}; | ||
| use html5ever::tendril::StrTendril; | ||
| use html5ever::tendril::{SliceExt, StrTendril}; | ||
| use html5ever::tokenizer::{TagKind, Token}; | ||
| use html5ever::{LocalName, QualName}; | ||
| use indexmap::IndexMap; | ||
|
|
@@ -69,7 +69,7 @@ impl Node { | |
| } | ||
|
|
||
| /// An HTML element. | ||
| #[derive(Debug)] | ||
| #[derive(Debug, Clone)] | ||
| pub(crate) struct Element { | ||
| /// The tag name. | ||
| pub(crate) name: QualName, | ||
|
|
@@ -565,8 +565,42 @@ where | |
| } | ||
| // This will eat TagEnd::Image | ||
| let alt = self.text_for_img_alt(); | ||
| img.insert_attr("alt", alt.into()); | ||
| self.append(Node::Element(img)); | ||
| img.insert_attr("alt", alt.to_tendril()); | ||
|
|
||
| // If the image is not being rendered inside a link, we can enable the "zoom-in" | ||
| // feature. | ||
| if !self | ||
| .tag_stack | ||
| .iter() | ||
| .filter_map(|node_id| { | ||
| self.tree | ||
| .get(*node_id) | ||
| .and_then(|el| el.value().as_element()) | ||
| }) | ||
| .any(|el| *el.name.local == *"a") | ||
| { | ||
| let mut label = Element::new("label"); | ||
| label.insert_attr("class", "checkbox-label".to_tendril()); | ||
| self.push(Node::Element(label)); | ||
|
|
||
| let mut input = Element::new("input"); | ||
| input.insert_attr("class", "checkbox-img".to_tendril()); | ||
| input.insert_attr("type", "checkbox".to_tendril()); | ||
| self.append(Node::Element(input)); | ||
|
|
||
| self.append(Node::Element(img.clone())); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm a little concerned about this approach of using a duplicated
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think screen readers all handle
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried with my screen reader. It does seem to ignore the "display: none". It's a little awkward now since the screen reader navigates to the checkbox (and says you are on a checkbox, and tells you the image alt), and then the image (and the alt and title). But I suppose it should be fine. We can adjust it if we get further feedback. |
||
|
|
||
| let mut wrapper = Element::new("span"); | ||
| wrapper.insert_attr("class", "img-wrapper".to_tendril()); | ||
| self.push_no_stack(Node::Element(wrapper)); | ||
|
|
||
| self.append(Node::Element(img)); | ||
|
|
||
| // We exit the `label` and the `span` (which used push_no_stack). | ||
| self.pop(); | ||
| } else { | ||
| self.append(Node::Element(img)); | ||
| } | ||
| return; | ||
| } | ||
| Tag::MetadataBlock(_) => { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,9 @@ | ||
| # Chapter 1 | ||
|
|
||
| Side by side [](link) [](link). | ||
|
|
||
| Some text [blob](a). | ||
|
|
||
|  | ||
|
|
||
| [](https://rust-lang.org/) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,87 @@ | ||
| // This test ensures that the image zoom-in/zoom-out works as expected. | ||
|
|
||
| define-function: ( | ||
| "check-image-not-zoomed-in", | ||
| [], | ||
| block { | ||
| // The "zoomed in" image should not be displayed. | ||
| assert-css: (".checkbox-label > .img-wrapper", {"display": "none"}) | ||
| // The cursor for the image should be "zoom-in". | ||
| assert-css: (".checkbox-label > img", {"cursor": "zoom-in"}) | ||
| }, | ||
| ) | ||
|
|
||
| define-function: ( | ||
| "check-image-zoomed-in", | ||
| [], | ||
| block { | ||
| // The image wrapper should now be displayed and have a "zoom-out" cursor. | ||
| assert-css: (".checkbox-label > .img-wrapper", {"display": "flex", "cursor": "zoom-out"}) | ||
| // Same for the image it contains. | ||
| assert-css: ( | ||
| ".checkbox-label > .img-wrapper img", | ||
| {"display": "block", "cursor": "zoom-out"}, | ||
| ) | ||
| }, | ||
| ) | ||
|
|
||
| go-to: |DOC_PATH| + "basic/index.html" | ||
| show-text: true | ||
| call-function: ("check-image-not-zoomed-in", {}) | ||
|
|
||
| // We click on the image to "zoom in". | ||
| click: ".checkbox-label > img" | ||
| call-function: ("check-image-zoomed-in", {}) | ||
|
|
||
| // We click on the wrapper to "zoom out". | ||
| click: ".checkbox-label > img" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it intentional that this clicks on the image underneath the wrapper? That is, should this be something like
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, they click on the image, but the image being part of the label, since there is no |
||
| // We check that everything is back to the previous state. | ||
| call-function: ("check-image-not-zoomed-in", {}) | ||
|
|
||
| // We test that the "escape" key also hides the "zoomed in" image. | ||
| // We click on the image to "zoom in". | ||
| click: ".checkbox-label > img" | ||
| call-function: ("check-image-zoomed-in", {}) | ||
| press-key: "Escape" | ||
| // We check that everything is back to the previous state. | ||
| call-function: ("check-image-not-zoomed-in", {}) | ||
|
|
||
| // We test that we can zoom in and out using only the keyboard. | ||
| // First we focus on the link just before the image we want to zoom in. | ||
| focus: "a[href='a']" | ||
| assert: "a[href='a']:focus" | ||
| press-key: "Tab" | ||
| // We check that the checkbox to "zoom in" is focused. | ||
| assert: ".checkbox-img:focus" | ||
| // We check that the image about to be zoomed in has the outline. | ||
| store-css: (".checkbox-img:focus", {"outline": outline}) | ||
| // Because we're using "auto" to match the system outline, we cannot check the whole value directly | ||
| // so instead we check if "auto" is present. | ||
| assert-variable: (outline, "auto", CONTAINS) | ||
| // We zoom in. | ||
| press-key: "Space" | ||
| call-function: ("check-image-zoomed-in", {}) | ||
| // We press the key again to zoom out. | ||
| press-key: "Space" | ||
| call-function: ("check-image-not-zoomed-in", {}) | ||
| // We now check it works with the "Escape" key as well. | ||
| press-key: "Space" | ||
| call-function: ("check-image-zoomed-in", {}) | ||
| press-key: "Escape" | ||
| call-function: ("check-image-not-zoomed-in", {}) | ||
|
|
||
| // Now we check that images in links (in `<a>` elements) don't get the "zoom-in" feature enabled. | ||
| // There is only one zoomable image. | ||
| assert-count: ("main .checkbox-img", 1) | ||
| // We should have 5 images: 4 "normal" ones visible in the rendered book and one hidden which is | ||
| // used only for the "zoom-in" feature. | ||
| assert-count: ("main img", 5) | ||
| // We check exact paths to ensure the 5 `<img>` are different. | ||
| assert: ("main > p > label.checkbox-label > img:nth-of-type(1)") | ||
| assert: ("main > p > label.checkbox-label > span.img-wrapper > img") | ||
| assert: ("main > p:nth-of-type(1) > a:nth-of-type(1) > img") | ||
| assert: ("main > p:nth-of-type(1) > a:nth-of-type(2) > img") | ||
| assert: ("main > p:not(:nth-of-type(2)) > a > img") | ||
|
|
||
| // The cursor for the image in link should not have "zoom-in". | ||
| assert-css: ("main > p > a > img", {"cursor": "pointer"}) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,3 @@ | ||
| <h1 id="images"><a class="header" href="#images">Images</a></h1> | ||
| <p><img src="https://rust-lang.org/logos/rust-logo-256x256.png" alt="Image “alt” & " "text" & <stuff> url <em>html</em> — hard break "></p> | ||
| <p><img src="https://rust-lang.org/logos/rust-logo-256x256.png" title="Some title" alt="Image with title"></p> | ||
| <p><label class="checkbox-label"><input class="checkbox-img" type="checkbox"><img src="https://rust-lang.org/logos/rust-logo-256x256.png" alt="Image “alt” & " "text" & <stuff> url <em>html</em> — hard break "><span class="img-wrapper"><img src="https://rust-lang.org/logos/rust-logo-256x256.png" alt="Image “alt” & " "text" & <stuff> url <em>html</em> — hard break "></span></label></p> | ||
| <p><label class="checkbox-label"><input class="checkbox-img" type="checkbox"><img src="https://rust-lang.org/logos/rust-logo-256x256.png" title="Some title" alt="Image with title"><span class="img-wrapper"><img src="https://rust-lang.org/logos/rust-logo-256x256.png" title="Some title" alt="Image with title"></span></label></p> |
Uh oh!
There was an error while loading. Please reload this page.