diff --git a/Cargo.toml b/Cargo.toml index a491c0de..cd84ab72 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,6 +24,7 @@ members = [ "crates/percy-router-macro-test", "crates/virtual-node", "examples/component-preview", + "examples/embed-non-percy-node", "examples/isomorphic/app", "examples/isomorphic/client", "examples/isomorphic/server", diff --git a/book/src/SUMMARY.md b/book/src/SUMMARY.md index 51e00553..5b080ba8 100644 --- a/book/src/SUMMARY.md +++ b/book/src/SUMMARY.md @@ -16,6 +16,7 @@ - [On Create Element](./html-macro/real-elements-and-nodes/on-create-elem/README.md) - [On Remove Element](./html-macro/real-elements-and-nodes/on-remove-elem/README.md) - [Boolean Attributes](./html-macro/boolean-attributes/README.md) + - [Special Attributes](./html-macro/special-attributes/README.md) - [Lists](./lists/README.md) - [Virtual DOM](./virtual-dom/README.md) - [Unit Testing your Views](./virtual-dom/unit-testing-views.md) diff --git a/book/src/html-macro/special-attributes/README.md b/book/src/html-macro/special-attributes/README.md new file mode 100644 index 00000000..877b16e0 --- /dev/null +++ b/book/src/html-macro/special-attributes/README.md @@ -0,0 +1,35 @@ +# Special Virtual DOM Attributes + +Some virtual DOM attributes do not merely set or remove the corresponding HTML attribute of the same name. + +## `checked` + +According to the [HTML spec](https://html.spec.whatwg.org/multipage/input.html#attr-input-checked), the `checked` HTML attribute only controls the default checkedness. +Changing the `checked` HTML attribute may not cause the checkbox's checkedness to change. + +By contrast: specifying `html! { }` causes `percy` to always render the checkbox with the specified checkedness. +- If the VDOM is updated from `html! { }` to `html { }`, the input element's checkedness will definitely change. +- If the VDOM is updated from `html! { }` to `html { }`, the input element's checkedness will be reverted to `true` even if the user interacted with the checkbox in between. + +`percy-dom` updates the `checked` property (current checkedness, not reflected in HTML). + +This behavior is more desirable because `percy` developers are accustomed to declaratively controlling the DOM and rendered HTML. + +`percy-dom` does not affect the presence of the `checked` attribute (default checkedness, reflected in HTML). + +This means that if you need to configure the `checked` attribute (this should almost never be the case if your GUI state is driven by the backend state) then `percy-dom` won't get in your way. + +## `value` + +According to the [HTML spec](https://html.spec.whatwg.org/multipage/input.html#attr-input-value), the `value` HTML attribute only controls the default value. +Changing the `value` HTML attribute may not cause the input element's value to change. + +By contrast: specifying `html! { }` causes `percy` to always render the input element with the specified value. +- If the VDOM is updated from `html! { }` to `html { }`, the input element's value will definitely change. +- If the VDOM is updated from `html! { }` to `html { }`, the input element's value will be reverted to `"hello"` even if the user interacted with the input element in between. + +`percy` updates both +- the `value` attribute (default value, reflected in HTML) and, +- the `value` property (current value, not reflected in HTML). + +Setting the `value` property is desirable because `percy` developers are accustomed to declaratively controlling the DOM and rendered HTML. diff --git a/crates/percy-dom/Cargo.toml b/crates/percy-dom/Cargo.toml index e497a2c0..d2ec7094 100644 --- a/crates/percy-dom/Cargo.toml +++ b/crates/percy-dom/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "percy-dom" -version = "0.9.9" +version = "0.10.0" authors = ["Chinedu Francis Nwafili "] description = "A standalone Virtual DOM creation, diffing and patching implementation" keywords = ["virtual", "dom", "wasm", "assembly", "webassembly"] diff --git a/crates/percy-dom/src/diff.rs b/crates/percy-dom/src/diff.rs index 7e74fc04..8f5cc908 100644 --- a/crates/percy-dom/src/diff.rs +++ b/crates/percy-dom/src/diff.rs @@ -282,6 +282,8 @@ fn find_attributes_to_add<'a>( attributes_to_add.insert(new_attr_name, new_attr_val); } else if new_attr_name == "value" { ctx.push_patch(Patch::ValueAttributeUnchanged(cur_node_idx, new_attr_val)); + } else if new_attr_name == "checked" { + ctx.push_patch(Patch::CheckedAttributeUnchanged(cur_node_idx, new_attr_val)); } } None => { @@ -1246,6 +1248,37 @@ mod tests { .test(); } + /// Verify that if an input (e.g. checkbox) has `checked` specified, we always push a + /// patch for setting the checked attribute and property so that the checkbox is + /// rendered in the specified state, regardless if any intervening input has occurred. + #[test] + fn always_pushes_patch_for_checked() { + for checkedness in [false, true] { + DiffTestCase { + old: html! { }, + new: html! { }, + expected: vec![Patch::CheckedAttributeUnchanged(0, &checkedness.into())], + } + .test(); + } + + for old_checkedness in [false, true] { + let new_checkedness = !old_checkedness; + + DiffTestCase { + old: html! { }, + new: html! { }, + expected: vec![Patch::AddAttributes( + 0, + vec![("checked", &new_checkedness.into())] + .into_iter() + .collect(), + )], + } + .test(); + } + } + /// Verify that we push an on create elem patch if the new node has the special attribute /// and the old node does not. #[test] diff --git a/crates/percy-dom/src/patch.rs b/crates/percy-dom/src/patch.rs index 51aee032..b70a3ba9 100644 --- a/crates/percy-dom/src/patch.rs +++ b/crates/percy-dom/src/patch.rs @@ -100,6 +100,10 @@ pub enum Patch<'a> { /// The value attribute of a textarea or input element has not changed, but we will still patch /// it anyway in case something was typed into the field. ValueAttributeUnchanged(BreadthFirstNodeIdx, &'a AttributeValue), + /// The checked attribute of a input element has not changed, but we will still patch + /// it anyway in case the checkbox was toggled by the user. Otherwise the checkbox + /// could have a different state to what is rendered. + CheckedAttributeUnchanged(BreadthFirstNodeIdx, &'a AttributeValue), /// Add attributes that the new node has that the old node does not AddAttributes(BreadthFirstNodeIdx, HashMap<&'a str, &'a AttributeValue>), /// Remove attributes that the old node had that the new node doesn't @@ -153,6 +157,7 @@ impl<'a> Patch<'a> { Patch::RemoveAttributes(node_idx, _) => *node_idx, Patch::ChangeText(node_idx, _) => *node_idx, Patch::ValueAttributeUnchanged(node_idx, _) => *node_idx, + Patch::CheckedAttributeUnchanged(node_idx, _) => *node_idx, Patch::SpecialAttribute(special) => match special { PatchSpecialAttribute::CallOnCreateElemOnExistingNode(node_idx, _) => *node_idx, PatchSpecialAttribute::SetDangerousInnerHtml(node_idx, _) => *node_idx, @@ -210,6 +215,9 @@ impl<'a> Patch<'a> { Patch::ValueAttributeUnchanged(node_idx, _) => { to_find.insert(*node_idx); } + Patch::CheckedAttributeUnchanged(node_idx, _) => { + to_find.insert(*node_idx); + } Patch::SpecialAttribute(special) => match special { PatchSpecialAttribute::CallOnCreateElemOnExistingNode(node_idx, _) => { to_find.insert(*node_idx); diff --git a/crates/percy-dom/src/patch/apply_patches.rs b/crates/percy-dom/src/patch/apply_patches.rs index 2c90a083..026b1fc3 100644 --- a/crates/percy-dom/src/patch/apply_patches.rs +++ b/crates/percy-dom/src/patch/apply_patches.rs @@ -241,7 +241,13 @@ fn apply_element_patch( } } AttributeValue::Bool(val_bool) => { - if *val_bool { + // Use `set_checked` instead of `{set,remove}_attribute` for the `checked` attribute. + // The "checked" attribute only determines default checkedness, + // but `percy-dom` takes `checked` to specify the actual checkedness. + // See crates/percy-dom/tests/checked_attribute.rs for more info. + if *attrib_name == "checked" { + maybe_set_checked_property(node, *val_bool); + } else if *val_bool { node.set_attribute(attrib_name, "")?; } else { node.remove_attribute(attrib_name)?; @@ -390,6 +396,11 @@ fn apply_element_patch( Ok(()) } + Patch::CheckedAttributeUnchanged(_node_idx, value) => { + maybe_set_checked_property(node, value.as_bool().unwrap()); + + Ok(()) + } Patch::SpecialAttribute(special) => match special { PatchSpecialAttribute::CallOnCreateElemOnExistingNode(_node_idx, new_node) => { new_node @@ -512,6 +523,13 @@ fn maybe_set_value_property(node: &Element, value: &str) { } } +// See crates/percy-dom/tests/checked_attribute.rs +fn maybe_set_checked_property(node: &Element, checked: bool) { + if let Some(input_node) = node.dyn_ref::() { + input_node.set_checked(checked); + } +} + // Looks for a property on the element. If it's there then this is a Percy element. // // TODO: We need to know not just if the node was created by Percy... but if it was created by diff --git a/crates/percy-dom/tests/checked_property.rs b/crates/percy-dom/tests/checked_property.rs new file mode 100644 index 00000000..2389e1fe --- /dev/null +++ b/crates/percy-dom/tests/checked_property.rs @@ -0,0 +1,186 @@ +//! `percy-dom` treats the `checked` virtual node attribute specially. +//! `percy-dom` sets the `checked` element property (actual checkedness), +//! not the `checked` HTML attribute (default checkedness). +//! +//! Developers, are likely to assume that `checked` specifies the state of the checkbox +//! directly. `percy-dom` ensures that this is true. +//! +//! See the tests for more details. Start with [`patch_uses_set_checked_function`]. +//! +//! To run all tests in this file: +//! +//! ```sh +//! wasm-pack test --chrome --headless crates/percy-dom --test checked_property +//! ``` + +use wasm_bindgen_test::*; + +use percy_dom::event::VirtualEvents; +use wasm_bindgen::JsCast; +use web_sys::*; + +use percy_dom::prelude::*; + +wasm_bindgen_test_configure!(run_in_browser); + +/// Verify that `percy_dom::patch` uses `set_checked` to set the checkedness +/// of an input element when specified. +/// +/// ## Why? +/// +/// The `checked` HTML attribute only determines the default checkedness. +/// The browser uses the default checkedness as the checkbox's +/// checkedness until the user clicks on the checkbox, setting the "dirty checked" browser +/// flag https://html.spec.whatwg.org/multipage/input.html#concept-input-checked-dirty +/// which results in the browser maintaining the checkbox's state INDEPENDENTLY from the +/// default checked state. i.e. Changing the default checkedness no longer affects the actual +/// checkedness after the user has pressed the input. +/// +/// We want `html!{ ... checked=val ... }` to specify the current checkedness of the checkbox +/// directly - avoiding the checkbox being toggled differently to what the developer +/// specified in the virtual DOM. +/// +/// Using web-sys's `set_checked` sets the actual checkbox's checkedness. Futhermore, it enables the +/// dirty-checked flag (NB: BUT ONLY WHEN THE CHECKBOX STATE IS CHANGED), which we can test for. +/// +/// ## Test approach +/// +/// - Create a virtual node with the checkbox having checkedness !C, and patch it to have checkedness C. +/// (This should cause the dirty flag to be set IF `set_checked` is used.) +/// - Assert that the corresponding DOM element has checkedness of C. +/// +/// - Now, remove the attribute if the checkbox is checked, or set the attribute if not. +/// (The checkbox should hold its state as the dirty flag is checked, therefore +/// changing the default checkedness through the `checked` attribute no longer +/// should affect the checkedness of the checkbox.) +/// - Assert that the checkedness of the checkbox element is still B. +#[wasm_bindgen_test] +fn patch_sets_checked_property() { + for checkedness in [false, true] { + let start_input = html! {}; + let end_input = html! {}; + + let mut events = VirtualEvents::new(); + let (input_node, enode) = start_input.create_dom_node(&mut events); + events.set_root(enode); + + let input_elem = input_node.dyn_ref::().unwrap(); + + let patches = percy_dom::diff(&start_input, &end_input); + percy_dom::patch(input_node.clone(), &end_input, &mut events, &patches).unwrap(); + assert_eq!(input_elem.checked(), checkedness); + + if checkedness { + input_elem.remove_attribute("checked").unwrap(); + } else { + input_elem.set_attribute("checked", "").unwrap(); + } + assert_eq!(input_elem.checked(), checkedness); + } +} + +/// Verify that `percy_dom::patch` uses `set_checked` to set the `checked` property +/// of an input element even if the the specified `checked` value does not change +/// between the `old` and `new` virtual nodes. +/// +/// ## Why? +/// +/// Note: the rationale given in [`patch_sets_checked_property`] is prerequisite reading. +/// +/// The user might interact with the checkbox in between the previous render and the next +/// one, changing the checkedness state in the browser, but `diff` would not realize this +/// assuming the rendered `checked` value does not change. +/// +/// For example: +/// - Developer renders `html! { ... checked=true ... }` +/// - User clicks on the checkbox, changing the browser's checkbox checkedness to false. +/// - Developer renders `html! { ... checked=true ... }` +/// - `diff` doesn't realize anything needs to change, so it doesn't issue any changes. +/// - Developer is still trying to render the checkbox as checked but the browser checkbox +/// stays unchecked. +/// +/// If `percy_dom::diff` always specifies that `percy_dom::patch` should set the `checked` +/// property if its specified, then the above cannot happen. The element's checked state +/// will be fixed when `percy_dom::patch` is called, keeping the developer-specified `checked` +/// value and the checkbox element's visual state in sync. +/// +/// ## Test approach +/// +/// - Create a a DOM node with the checkbox having checkedness C. +/// - Set it's checkedness to be !C. +/// - Diff and patch with the virtual node still specifying it's checkedness as C. +/// - Assert that the checkedness has been reset to C, even though the virtual node did not change. +#[wasm_bindgen_test] +fn patch_always_sets_checked_property() { + for checkedness in [false, true] { + let start_input = html! {}; + let end_input = html! {}; + + let mut events = VirtualEvents::new(); + let (input_node, enode) = start_input.create_dom_node(&mut events); + events.set_root(enode); + + let input_elem = input_node.dyn_ref::().unwrap(); + assert_eq!(input_elem.checked(), checkedness); + + input_elem.set_checked(!checkedness); // modify checked property + + let patches = percy_dom::diff(&start_input, &end_input); + percy_dom::patch(input_node.clone(), &end_input, &mut events, &patches).unwrap(); + + assert_eq!(input_elem.checked(), checkedness); + } +} + +/// Verify that `percy_dom::patch` does not set the default checkedness. +/// That is to say: it does NOT manipulate the HTML `checked` attribute. +/// +/// ## Why? +/// +/// Note: the rationale given in [`patch_sets_checked_property`] is prerequisite reading. +/// +/// `percy` is intended to serve the developer who is making a nontrivial app who's state +/// is driven by the backend of the application. The application state is _not_ driven by +/// the frontend. Therefore the state of checkboxes is specified explicitly idiomatically. +/// +/// `percy-dom` does not currently allow for a way to directly manipulate the default +/// checkedness of an input element. Default checkedness is not useful unless the state +/// of an input element is driven by the frontend. +/// +/// Users may want to break out of this mold however, and modify the default checkedness. +/// In this case, it's much simpler if `percy` doesn't change the default checkedness +/// unnecessarily. Equivalently, `percy-dom` does not manipulate the presence of the HTML +/// `checked` attribute. +/// +/// ## Test approach +/// +/// - Create an input element with checkedness C. +/// - Set the presence of the HTML attribute (default checkedness) to be D. +/// - Update the input element's checkedness to be E. +/// - Assert that the default checkedness is still D. +#[wasm_bindgen_test] +fn patch_does_not_set_default_checkedness() { + for prev_checkedness in [false, true] { + for next_checkedness in [false, true] { + for default_checkedness in [false, true] { + let start_input = html! {}; + let end_input = html! {}; + + let mut events = VirtualEvents::new(); + let (input_node, enode) = start_input.create_dom_node(&mut events); + events.set_root(enode); + + let input_elem = input_node.dyn_ref::().unwrap(); + assert_eq!(input_elem.checked(), prev_checkedness); + + // Modify presence of `checked`` attribute. + input_elem.set_default_checked(default_checkedness); + + let patches = percy_dom::diff(&start_input, &end_input); + percy_dom::patch(input_node.clone(), &end_input, &mut events, &patches).unwrap(); + + assert_eq!(input_elem.default_checked(), default_checkedness); + } + } + } +} diff --git a/crates/percy-router/src/route.rs b/crates/percy-router/src/route.rs index ed0a3e2f..2a982fb3 100644 --- a/crates/percy-router/src/route.rs +++ b/crates/percy-router/src/route.rs @@ -216,7 +216,7 @@ mod tests { ("/users/foo", false), ], } - .test(); + .test(); } /// Verify that a `/` route definition doesn't capture `/some-route`. @@ -230,7 +230,7 @@ mod tests { ("/foo", false), ], } - .test(); + .test(); } /// Verify that we correctly match when a static segment comes after a dynamic segment. @@ -247,7 +247,7 @@ mod tests { ("/5/bar", false), ], } - .test(); + .test(); } struct MatchRouteTestCase { diff --git a/examples/embed-non-percy-node/Cargo.toml b/examples/embed-non-percy-node/Cargo.toml new file mode 100644 index 00000000..2f6e8aa1 --- /dev/null +++ b/examples/embed-non-percy-node/Cargo.toml @@ -0,0 +1,16 @@ +[package] +name = "embed-non-percy-node" +version = "0.1.0" +edition = "2021" +publish = [] + +[lib] +crate-type = ["cdylib"] + +[dependencies] +percy-dom = { path = "../../crates/percy-dom" } +wasm-bindgen-test = "0.3" + +[dependencies.web-sys] +version = "0.3" +features = ["Document", "HtmlElement"] diff --git a/examples/embed-non-percy-node/README.md b/examples/embed-non-percy-node/README.md new file mode 100644 index 00000000..9e411d4b --- /dev/null +++ b/examples/embed-non-percy-node/README.md @@ -0,0 +1,19 @@ +# embed-non-percy-node + +This example demonstrates embedding non-`percy`-controlled DOM elements within the DOM elements controlled by `percy-dom`'s virtual DOM. + +Elements that are outside `percy-dom`'s virtual DOM model and will not be affected by it (unless, for example, the parent element is deleted). + +## Running this example + +```sh +git clone git@github.com:chinedufn/percy.git +cd percy + +# Use one of the following to run the example in a headless web browser +CHROMEDRIVER=chromedriver ./examples/embed-non-percy-node/start.sh +GECKODRIVER=geckodriver ./examples/embed-non-percy-node/start.sh +SAFARIDRIVER=safaridriver ./examples/embed-non-percy-node/start.sh +``` + +You may need to install the appropriate driver and (optionally) add it to your `PATH`. diff --git a/examples/embed-non-percy-node/src/lib.rs b/examples/embed-non-percy-node/src/lib.rs new file mode 100644 index 00000000..936d7f25 --- /dev/null +++ b/examples/embed-non-percy-node/src/lib.rs @@ -0,0 +1,87 @@ +use percy_dom::{event::VirtualEvents, prelude::*, JsCast}; +use web_sys::{Element, Node}; + +use wasm_bindgen_test::*; + +wasm_bindgen_test_configure!(run_in_browser); + +/// Verify that the +/// - `percy-dom`-controlled element, `root`, +/// - and the embedded DOM element, `my_special_paragraph` +/// +/// behave exactly as we expect when updating them... +/// +/// - Updating the `percy`-controlled element `root` does not affect the foreign elements. +/// - The state of the embedded elements are preserved across `percy_dom::patch` calls. +#[wasm_bindgen_test] +fn updating_percy_element_and_embedded_dom_element_test() { + let (vdom_root, dom_root, mut events, my_special_paragraph) = + setup_percy_dom_with_embedded_element(); + + // Modify the `my_special_paragraph` element + my_special_paragraph.set_text_content(Some("world")); + + // Adds the "data-example" attribute, set to "New data!", to the root node. + percy_diff_and_patch_root_node(&vdom_root, &dom_root, &mut events); + + // Assert that our modification of the DOM element was successful. + // Assert that `percy-dom` didn't overwrite our changes. + assert_eq!( + my_special_paragraph.text_content(), + Some("world".to_string()) + ); + + // Assert that `percy-dom`'s diff+patch succeeded. + let data_example = dom_root + .dyn_ref::() + .unwrap() + .get_attribute("data-example") + .unwrap(); + assert_eq!(&data_example, "New data!"); +} + +fn create_my_special_paragraph_element() -> Element { + let document = web_sys::window().unwrap().document().unwrap(); + + let element = document.create_element("p").unwrap(); + element.set_id("my_special_paragraph"); + element.set_text_content(Some("hello")); + + element +} + +fn setup_percy_dom_with_embedded_element() -> (VirtualNode, Node, VirtualEvents, Element) { + let my_special_paragraph_element = create_my_special_paragraph_element(); + let my_special_paragraph_element_append = my_special_paragraph_element.clone(); + + // The `div` element will be the child of the percy-controlled root element. + let vdom_root_node = html! { +
+ }; + + // Create the DOM nodes from the virtual DOM. + let mut events = VirtualEvents::new(); + let (root_node, event_node) = vdom_root_node.create_dom_node(&mut events); + events.set_root(event_node); + + ( + vdom_root_node, + root_node, + events, + my_special_paragraph_element, + ) +} + +fn percy_diff_and_patch_root_node( + vdom_root: &VirtualNode, + dom_root: &Node, + events: &mut VirtualEvents, +) { + let new_vdom_root = html! {
}; + let patches = percy_dom::diff(&vdom_root, &new_vdom_root); + percy_dom::patch(dom_root.clone(), &new_vdom_root, events, &patches).unwrap(); +} diff --git a/examples/embed-non-percy-node/start.sh b/examples/embed-non-percy-node/start.sh new file mode 100755 index 00000000..f35aba7f --- /dev/null +++ b/examples/embed-non-percy-node/start.sh @@ -0,0 +1,3 @@ +#!/bin/bash + +cargo test -p embed-non-percy-node --target wasm32-unknown-unknown diff --git a/examples/isomorphic/app/src/lib.rs b/examples/isomorphic/app/src/lib.rs index cd54b9df..67046fd0 100644 --- a/examples/isomorphic/app/src/lib.rs +++ b/examples/isomorphic/app/src/lib.rs @@ -91,7 +91,7 @@ fn download_contributors_json(store: Provided>>) { let store = Rc::clone(&store); let callback = Closure::wrap(Box::new(move |json: JsValue| { store.borrow_mut().msg(&Msg::SetContributorsJson(json)); - }) as Box); + }) as Box); download_json( "https://api.github.com/repos/chinedufn/percy/contributors", callback.as_ref().unchecked_ref(), @@ -100,7 +100,7 @@ fn download_contributors_json(store: Provided>>) { // TODO: Store and drop the callback instead of leaking memory. callback.forget(); } - }) as Box); + }) as Box); web_sys::window() .unwrap() diff --git a/examples/isomorphic/app/src/store.rs b/examples/isomorphic/app/src/store.rs index d48cc6c5..5f9754b2 100644 --- a/examples/isomorphic/app/src/store.rs +++ b/examples/isomorphic/app/src/store.rs @@ -7,9 +7,9 @@ use std::rc::Rc; pub struct Store { state: StateWrapper, - after_route: Option ()>>, + after_route: Option ()>>, router: Option>, - listeners: Vec ()>>, + listeners: Vec ()>>, } impl Store { @@ -49,11 +49,11 @@ impl Store { } } - pub fn subscribe(&mut self, callback: Box ()>) { + pub fn subscribe(&mut self, callback: Box ()>) { self.listeners.push(callback) } - pub fn set_after_route(&mut self, after_route: Box ()>) { + pub fn set_after_route(&mut self, after_route: Box ()>) { self.after_route = Some(after_route); }