From 5ab4fdd94df5fe2155dea2df039a3b504262fa61 Mon Sep 17 00:00:00 2001 From: Shaun Beautement Date: Mon, 23 Sep 2024 01:37:11 +0200 Subject: [PATCH 1/9] change the html-macro's checked attribute to directly set checkedness, rather than default checkedness --- book/src/SUMMARY.md | 1 + .../html-macro/special-attributes/README.md | 55 +++++ crates/percy-dom/src/diff.rs | 2 + crates/percy-dom/src/patch.rs | 8 + crates/percy-dom/src/patch/apply_patches.rs | 20 +- crates/percy-dom/tests/checked_attribute.rs | 188 ++++++++++++++++++ crates/percy-router/src/route.rs | 6 +- 7 files changed, 276 insertions(+), 4 deletions(-) create mode 100644 book/src/html-macro/special-attributes/README.md create mode 100644 crates/percy-dom/tests/checked_attribute.rs 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..78a04019 --- /dev/null +++ b/book/src/html-macro/special-attributes/README.md @@ -0,0 +1,55 @@ +# Special Attributes + +Some attributes do not merely set or remove the corresponding HTML attribute of the same name. + +## `checked` Attribute + +Specifying `checked` causes `percy` to render the checkbox with the specified checkedness, INSTEAD of setting the default checkedness. + +The `checked` HTML attribute specifies the [default checkedness of an input element](https://html.spec.whatwg.org/multipage/input.html#attr-input-checked). It does not determine the checkedness of the checkbox directly. + +From the link above: + +> The checked content attribute is a boolean attribute that gives the default checkedness of the input element. When the checked content attribute is added, if the control does not have dirty checkedness, the user agent must set the checkedness of the element to true; when the checked content attribute is removed, if the control does not have dirty checkedness, the user agent must set the checkedness of the element to false. + +A developer is likely to use `html!`'s `checked` attribute and expect the value they specify to be the value that is rendered. Setting the `checked` HTML attribute alone does not achieve this. + +To avoid this, `html!`'s `checked` specifies the rendered checkedness directly, using `set_checked` behind the scenes. + +```rust +html! { }; +``` + +It's still possible to use `elem.set_attribute("checked", "")` and `elem.remove_attribute("checked")` to configure the default checkedness. + +```rust +let vnode = html! {}; + +let mut events = VirtualEvents::new(); +let (input_node, enode) = vnode.create_dom_node(&mut events); +events.set_root(enode); + +// Sets the default checkedness to true by setting the `checked` attribute. +let input_elem = input_node.dyn_ref::().unwrap(); +input_elem.set_attribute("checked", "").unwrap(); +``` + +## `value` Attribute + +Specifying `value` causes `percy` to render the the input's value as specified, as well as setting the default value. + +Similar to `checked`, the `value` HTML attribute [specifies the default value of an input element](https://html.spec.whatwg.org/multipage/input.html#attr-input-value). It does not determine the value of the element directly. + +From the link above: + +> The value content attribute gives the default value of the input element. When the value content attribute is added, set, or removed, if the control's dirty value flag is false, the user agent must set the value of the element to the value of the value content attribute, if there is one, or the empty string otherwise, and then run the current value sanitization algorithm, if one is defined. + +A developer is likely to use `html!`'s `value` attribute and expect the value they specify to be the value that is rendered. Setting the `value` HTML attribute alone does not achieve this. + +To avoid this, `html!`'s `value` specifies the rendered value directly, using `set_value` behind the scenes. + +```rust +html! { }; +``` + +Note that **unlike the `checked` attribute**, `percy` applies the specified value by setting the `value` attribute as well as using `set_value`. diff --git a/crates/percy-dom/src/diff.rs b/crates/percy-dom/src/diff.rs index 7e74fc04..dffc47ac 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 => { 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..48b0829e 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 determines default checkedness, + // but `percy` interprets `checked` as determining current checkedness instead. + // 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_attribute.rs b/crates/percy-dom/tests/checked_attribute.rs new file mode 100644 index 00000000..7c3b6a74 --- /dev/null +++ b/crates/percy-dom/tests/checked_attribute.rs @@ -0,0 +1,188 @@ +//! Verify that we treat `checked` where specified in the `html!` macro as +//! setting the checkedness, not as setting the `checked` HTML attribute (which +//! only determines the default checkedness). Developers, unless already aware, +//! are likely to assume that `checked` specifies the state of the checkbox +//! directly. `percy` 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: +//! +//! wasm-pack test --chrome --headless crates/percy-dom --test checked_attribute + +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 checkedness of the checkbox, not +/// the default checkedness. This is more intuitive: changing the value of `checked` changes +/// the the checkbox's checkedness directly, rather than only when the dirty flag isn't set. +/// +/// Using `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 a DOM node with the checkbox having checkedness !C, and patch it to have +/// checkedness B. A and B may be the same or different, true or false. +/// (This should cause the dirty flag to be set IF `set_checked` is used.) +/// - Assert that the DOM element has checkedness of B. +/// +/// - 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_uses_set_checked_function() { + 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` attribute +/// 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_uses_set_checked_function`] 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` +/// attribute 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() { + 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); + + 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 add or remove the `checked` attribute +/// due to specifying `checked` in `percy`'s `html!` macro. +/// +/// ## Why? +/// +/// Note: the rationale given in [`patch_uses_set_checked_function`] is prerequisite reading. +/// +/// We do not want to override the default checkedness of the checkbox when the +/// user of the `html!` macro specifies the checkedness using `checked`. This means +/// that the `checked` HTML attribute should not be changed by specifying `checked`. +/// +/// For example: +/// - Developer sets default checkedness using `elem.set_attribute("checked", "")` on checkbox. +/// - Developer specifies `checked=false` to toggle it off for now. +/// - Developer stops specifying `checked`. +/// - The form that the `elem` is a part of gets reset, changing the checkbox to its default state. +/// - Developer expects it to return to the default checkedness they specified. +/// +/// ## Test approach +/// +/// - Create a a DOM node with the checkbox having some checkedness. +/// - Add or remove the checked attribute. +/// - Diff and patch with the virtual node with some checkedness (same or different). +/// - Assert that the presence of the checked attribute has not changed. +#[wasm_bindgen_test] +fn percy_checked_does_not_add_or_remove_checked_attribute() { + for old_checkedness in [false, true] { + for new_checkedness in [false, true] { + for old_attribute_presence in [false, true] { + let old_input = html! {}; + let end_input = html! {}; + + let mut events = VirtualEvents::new(); + let (input_node, enode) = old_input.create_dom_node(&mut events); + events.set_root(enode); + + let input_elem = input_node.dyn_ref::().unwrap(); + + if old_attribute_presence { + input_elem.set_attribute("checked", "").unwrap(); + } else { + input_elem.remove_attribute("checked").unwrap(); + } + + let patches = percy_dom::diff(&old_input, &end_input); + percy_dom::patch(input_node.clone(), &end_input, &mut events, &patches).unwrap(); + + assert_eq!( + input_elem.get_attribute("checked").is_some(), + old_attribute_presence + ); + } + } + } +} 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 { From eccdd56136b6b7d05014fe1a0186829a5dd49d2f Mon Sep 17 00:00:00 2001 From: Shaun Beautement Date: Mon, 23 Sep 2024 01:55:12 +0200 Subject: [PATCH 2/9] doc reference --- crates/percy-dom/tests/checked_attribute.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/percy-dom/tests/checked_attribute.rs b/crates/percy-dom/tests/checked_attribute.rs index 7c3b6a74..7546a8fe 100644 --- a/crates/percy-dom/tests/checked_attribute.rs +++ b/crates/percy-dom/tests/checked_attribute.rs @@ -5,7 +5,7 @@ //! directly. `percy` ensures that this is true. //! //! See the tests for more details. -//! Start with `patch_uses_set_checked_function`. +//! Start with [`patch_uses_set_checked_function`]. //! //! To run all tests in this file: //! From 702a4942a069ecac3ba85f9cc353cbbd53c71783 Mon Sep 17 00:00:00 2001 From: Shaun Beautement Date: Thu, 26 Sep 2024 09:16:23 +0200 Subject: [PATCH 3/9] modify checked attribute as well, add test, add example, change docs --- Cargo.toml | 1 + .../html-macro/special-attributes/README.md | 58 +++++--------- crates/percy-dom/src/diff.rs | 31 ++++++++ crates/percy-dom/src/patch/apply_patches.rs | 23 ++++-- crates/percy-dom/tests/checked_attribute.rs | 76 ++++++------------- examples/append-to-dom/Cargo.toml | 19 +++++ examples/append-to-dom/README.md | 24 ++++++ examples/append-to-dom/src/lib.rs | 50 ++++++++++++ examples/append-to-dom/start.sh | 3 + 9 files changed, 188 insertions(+), 97 deletions(-) create mode 100644 examples/append-to-dom/Cargo.toml create mode 100644 examples/append-to-dom/README.md create mode 100644 examples/append-to-dom/src/lib.rs create mode 100755 examples/append-to-dom/start.sh diff --git a/Cargo.toml b/Cargo.toml index a491c0de..0d02bace 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,6 +23,7 @@ members = [ "crates/percy-router-macro", "crates/percy-router-macro-test", "crates/virtual-node", + "examples/append-to-dom", "examples/component-preview", "examples/isomorphic/app", "examples/isomorphic/client", diff --git a/book/src/html-macro/special-attributes/README.md b/book/src/html-macro/special-attributes/README.md index 78a04019..b5a3b092 100644 --- a/book/src/html-macro/special-attributes/README.md +++ b/book/src/html-macro/special-attributes/README.md @@ -4,52 +4,32 @@ Some attributes do not merely set or remove the corresponding HTML attribute of ## `checked` Attribute -Specifying `checked` causes `percy` to render the checkbox with the specified checkedness, INSTEAD of setting the default checkedness. +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. -The `checked` HTML attribute specifies the [default checkedness of an input element](https://html.spec.whatwg.org/multipage/input.html#attr-input-checked). It does not determine the checkedness of the checkbox directly. +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. -From the link above: +`percy` updates both +- the `checked` attribute (default checkedness, reflected in HTML) and, +- the `checked` property (current checkedness, not reflected in HTML). -> The checked content attribute is a boolean attribute that gives the default checkedness of the input element. When the checked content attribute is added, if the control does not have dirty checkedness, the user agent must set the checkedness of the element to true; when the checked content attribute is removed, if the control does not have dirty checkedness, the user agent must set the checkedness of the element to false. - -A developer is likely to use `html!`'s `checked` attribute and expect the value they specify to be the value that is rendered. Setting the `checked` HTML attribute alone does not achieve this. - -To avoid this, `html!`'s `checked` specifies the rendered checkedness directly, using `set_checked` behind the scenes. - -```rust -html! { }; -``` - -It's still possible to use `elem.set_attribute("checked", "")` and `elem.remove_attribute("checked")` to configure the default checkedness. - -```rust -let vnode = html! {}; - -let mut events = VirtualEvents::new(); -let (input_node, enode) = vnode.create_dom_node(&mut events); -events.set_root(enode); - -// Sets the default checkedness to true by setting the `checked` attribute. -let input_elem = input_node.dyn_ref::().unwrap(); -input_elem.set_attribute("checked", "").unwrap(); -``` +This behavior is more desirable because `percy` developers are accustomed to declaratively controlling the DOM and rendered HTML. ## `value` Attribute -Specifying `value` causes `percy` to render the the input's value as specified, as well as setting the default value. - -Similar to `checked`, the `value` HTML attribute [specifies the default value of an input element](https://html.spec.whatwg.org/multipage/input.html#attr-input-value). It does not determine the value of the element directly. - -From the link above: - -> The value content attribute gives the default value of the input element. When the value content attribute is added, set, or removed, if the control's dirty value flag is false, the user agent must set the value of the element to the value of the value content attribute, if there is one, or the empty string otherwise, and then run the current value sanitization algorithm, if one is defined. +(Note, this is virtually identical to the above section on the `checked` attribute.) -A developer is likely to use `html!`'s `value` attribute and expect the value they specify to be the value that is rendered. Setting the `value` HTML attribute alone does not achieve this. +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. -To avoid this, `html!`'s `value` specifies the rendered value directly, using `set_value` behind the scenes. +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. -```rust -html! { }; -``` +`percy` updates both +- the `value` attribute (default value, reflected in HTML) and, +- the `value` property (current value, not reflected in HTML). -Note that **unlike the `checked` attribute**, `percy` applies the specified value by setting the `value` attribute as well as using `set_value`. +This behavior is more desirable because `percy` developers are accustomed to declaratively controlling the DOM and rendered HTML. diff --git a/crates/percy-dom/src/diff.rs b/crates/percy-dom/src/diff.rs index dffc47ac..8f5cc908 100644 --- a/crates/percy-dom/src/diff.rs +++ b/crates/percy-dom/src/diff.rs @@ -1248,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/apply_patches.rs b/crates/percy-dom/src/patch/apply_patches.rs index 48b0829e..7a66c582 100644 --- a/crates/percy-dom/src/patch/apply_patches.rs +++ b/crates/percy-dom/src/patch/apply_patches.rs @@ -241,16 +241,18 @@ fn apply_element_patch( } } AttributeValue::Bool(val_bool) => { - // Use `set_checked` instead of `{set,remove}_attribute` for the `checked` attribute. + if *val_bool { + node.set_attribute(attrib_name, "")?; + } else { + node.remove_attribute(attrib_name)?; + } + + // Use `set_checked` in addition to `{set,remove}_attribute` for the `checked` attribute. // The "checked" attribute determines default checkedness, - // but `percy` interprets `checked` as determining current checkedness instead. + // but `percy` interprets `checked` as determining current checkedness too. // 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)?; } } } @@ -397,7 +399,14 @@ fn apply_element_patch( Ok(()) } Patch::CheckedAttributeUnchanged(_node_idx, value) => { - maybe_set_checked_property(node, value.as_bool().unwrap()); + let value = value.as_bool().unwrap(); + maybe_set_checked_property(node, value); + + if value { + node.set_attribute("checked", "")?; + } else { + node.remove_attribute("checked")?; + } Ok(()) } diff --git a/crates/percy-dom/tests/checked_attribute.rs b/crates/percy-dom/tests/checked_attribute.rs index 7546a8fe..b3217253 100644 --- a/crates/percy-dom/tests/checked_attribute.rs +++ b/crates/percy-dom/tests/checked_attribute.rs @@ -34,11 +34,11 @@ wasm_bindgen_test_configure!(run_in_browser); /// 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 checkedness of the checkbox, not -/// the default checkedness. This is more intuitive: changing the value of `checked` changes -/// the the checkbox's checkedness directly, rather than only when the dirty flag isn't set. +/// We want `html!{ ... checked=val ... }` to specify the current checkedness of the checkbox +/// directly - avoiding the checkbox rendering in a different state to what the developer +/// specified in the virtual DOM. /// -/// Using `set_checked` sets the actual checkbox's checkedness. Futhermore, it enables the +/// 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 @@ -54,7 +54,7 @@ wasm_bindgen_test_configure!(run_in_browser); /// should affect the checkedness of the checkbox.) /// - Assert that the checkedness of the checkbox element is still B. #[wasm_bindgen_test] -fn patch_uses_set_checked_function() { +fn patch_sets_checked_property() { for checkedness in [false, true] { let start_input = html! {}; let end_input = html! {}; @@ -84,7 +84,7 @@ fn patch_uses_set_checked_function() { /// /// ## Why? /// -/// Note: the rationale given in [`patch_uses_set_checked_function`] is prerequisite reading. +/// 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 @@ -110,7 +110,7 @@ fn patch_uses_set_checked_function() { /// - 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() { +fn patch_always_sets_checked_property_and_attribute() { for checkedness in [false, true] { let start_input = html! {}; let end_input = html! {}; @@ -122,67 +122,41 @@ fn patch_always_sets_checked() { let input_elem = input_node.dyn_ref::().unwrap(); assert_eq!(input_elem.checked(), checkedness); - input_elem.set_checked(!checkedness); + input_elem.set_checked(!checkedness); // modify checked property + input_elem.set_default_checked(!checkedness); // modify checked attribute 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); + assert_eq!(input_elem.default_checked(), checkedness); } } -/// Verify that `percy_dom::patch` does not add or remove the `checked` attribute -/// due to specifying `checked` in `percy`'s `html!` macro. +/// Verify that `vnode.to_string()` includes the `checked` attribute +/// as a result of specifying `checked` in `percy`'s `html!` macro. /// /// ## Why? /// -/// Note: the rationale given in [`patch_uses_set_checked_function`] is prerequisite reading. +/// Note: the rationale given in [`patch_sets_checked_property`] is prerequisite reading. /// -/// We do not want to override the default checkedness of the checkbox when the -/// user of the `html!` macro specifies the checkedness using `checked`. This means -/// that the `checked` HTML attribute should not be changed by specifying `checked`. -/// -/// For example: -/// - Developer sets default checkedness using `elem.set_attribute("checked", "")` on checkbox. -/// - Developer specifies `checked=false` to toggle it off for now. -/// - Developer stops specifying `checked`. -/// - The form that the `elem` is a part of gets reset, changing the checkbox to its default state. -/// - Developer expects it to return to the default checkedness they specified. +/// While `html!`'s `checked` configures the `checked` property to declaratively specify +/// the rendered checkbox state, +/// When performing server-side rendering, only the HTML reaches the client, not the +/// (non-existent) DOM state. Therefore the HTML attribute's presence should correspond +/// to `html!`'s `checked` state as well. /// /// ## Test approach /// /// - Create a a DOM node with the checkbox having some checkedness. /// - Add or remove the checked attribute. -/// - Diff and patch with the virtual node with some checkedness (same or different). -/// - Assert that the presence of the checked attribute has not changed. +/// - Diff and patch with the virtual node with some new checkedness (same or different). +/// - Assert that the presence of the checked attribute has changed to match the new checkedness. #[wasm_bindgen_test] -fn percy_checked_does_not_add_or_remove_checked_attribute() { - for old_checkedness in [false, true] { - for new_checkedness in [false, true] { - for old_attribute_presence in [false, true] { - let old_input = html! {}; - let end_input = html! {}; - - let mut events = VirtualEvents::new(); - let (input_node, enode) = old_input.create_dom_node(&mut events); - events.set_root(enode); - - let input_elem = input_node.dyn_ref::().unwrap(); - - if old_attribute_presence { - input_elem.set_attribute("checked", "").unwrap(); - } else { - input_elem.remove_attribute("checked").unwrap(); - } - - let patches = percy_dom::diff(&old_input, &end_input); - percy_dom::patch(input_node.clone(), &end_input, &mut events, &patches).unwrap(); - - assert_eq!( - input_elem.get_attribute("checked").is_some(), - old_attribute_presence - ); - } - } +fn to_string_contains_checked_attribute() { + for (checkedness, html) in [(false, ""), (true, "")] { + let input = html! {}; + let string = input.to_string(); + assert_eq!(&string, html); } } diff --git a/examples/append-to-dom/Cargo.toml b/examples/append-to-dom/Cargo.toml new file mode 100644 index 00000000..a663c47b --- /dev/null +++ b/examples/append-to-dom/Cargo.toml @@ -0,0 +1,19 @@ +[package] +name = "append-to-dom" +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/append-to-dom/README.md b/examples/append-to-dom/README.md new file mode 100644 index 00000000..0addccfc --- /dev/null +++ b/examples/append-to-dom/README.md @@ -0,0 +1,24 @@ +# append-to-dom + +An example of manually managing and adding an HTML element into the `percy-dom` DOM tree. + +Such elements are outside `percy-dom`'s virtual DOM model and will not be affected by it (unless, for example, the parent element is removed). + +In this example, a checkbox that maintains a configured default checkedness (i.e. does not change the `checked` HTML attribute) despite changes to `HTMLInputElement.checked`. + +Note: This is a poor choice for server-side rendering and not useful for most client-side rendered `percy` applications. + +_Technical note: `percy` does not currently facilitate configuring a checkbox's default checkedness independently, instead setting both the `checked` attribute and property as it's seen to be the best default for typical users of `percy`, doing client-side or server-side rendering._ + + +--- + +## Running this example + +``` +git clone git@github.com:chinedufn/percy.git +cd percy + +cargo install wasm-pack +./examples/append-to-dom/start.sh +``` diff --git a/examples/append-to-dom/src/lib.rs b/examples/append-to-dom/src/lib.rs new file mode 100644 index 00000000..eb9276ce --- /dev/null +++ b/examples/append-to-dom/src/lib.rs @@ -0,0 +1,50 @@ +use percy_dom::{VirtualNode, JsCast, event::VirtualEvents, html}; +use web_sys::{HtmlElement, HtmlInputElement}; +use wasm_bindgen_test::*; + +wasm_bindgen_test_configure!(run_in_browser); + +#[wasm_bindgen_test] +pub fn main() { + // Create a div element that `percy` creates and updates. + let div = html! {
}; + + // Append to DOM from `VirtualNode`. + let mut events = VirtualEvents::new(); + let (div_node, enode) = div.create_dom_node(&mut events); + events.set_root(enode); + // Grab the div DOM element. + let div_elem = div_node.dyn_ref::().unwrap(); + + // Create a child checkbox node and append it to the DIV element. + let document = web_sys::window().unwrap().document().unwrap(); + let default_checked_elem = document.create_element("input").unwrap(); + default_checked_elem.set_attribute("type", "checkbox").unwrap(); + default_checked_elem.set_attribute("checked", "").unwrap(); + // Add our `percy`-agnostic checkbox as a child of a `percy`-controlled DOM element. + let child_elem = div_elem.append_child(&default_checked_elem).unwrap(); + + // Create a access the child element and confirm it has the `checked` attribute. + let child_checkbox_elem_ref = child_elem.dyn_ref::().unwrap(); + assert!(child_checkbox_elem_ref.has_attribute("checked")); + // Modify the checkedness of the child element. + // We will assert that it wasn't changed after a `percy-dom` update to ensure that `percy` isn't + // modifying our appended element in any way. + // Note that `percy` sets the `checked` attribute and property, whereas we're attempting to + // maintain a checkbox with an independent `checked` attribute (which means that the default + // checkedness is maintained: this might be useful for a form control that can be reset by the + // `HTMLFormElement.reset()` method, for example - however this would be unusual for a `percy` app.) + child_checkbox_elem_ref.set_checked(false); + + // Update the DOM according to the virtual node changes within the scope of `percy`. + let updated_div = html! {
}; + let patches = percy_dom::diff(&div, &updated_div); + percy_dom::patch(div_node.clone(), &updated_div, &mut events, &patches).unwrap(); + + // Get a reference to the child of the div element and confirm that is maintains the + // manually-specified attribute and property. + let child_elem = div_elem.children().get_with_index(0).unwrap(); + let child_input_elem_ref = child_elem.dyn_ref::().unwrap(); + assert!(child_input_elem_ref.has_attribute("checked")); + assert!(!child_input_elem_ref.checked()); +} diff --git a/examples/append-to-dom/start.sh b/examples/append-to-dom/start.sh new file mode 100755 index 00000000..dc0edf39 --- /dev/null +++ b/examples/append-to-dom/start.sh @@ -0,0 +1,3 @@ +#!/bin/bash + +wasm-pack test --chrome --headless examples/append-to-dom From 9739e7784acc9c648550c2769ddfffc3ba5a7bca Mon Sep 17 00:00:00 2001 From: Shaun Beautement Date: Sat, 28 Sep 2024 22:39:47 +0200 Subject: [PATCH 4/9] improve example and docs --- Cargo.toml | 2 +- .../html-macro/special-attributes/README.md | 2 - crates/percy-dom/tests/checked_attribute.rs | 49 ++--- examples/append-to-dom/README.md | 24 --- examples/append-to-dom/src/lib.rs | 50 ------ examples/append-to-dom/start.sh | 3 - .../Cargo.toml | 2 +- .../README.md | 38 ++++ .../src/lib.rs | 167 ++++++++++++++++++ .../start.sh | 3 + examples/isomorphic/app/src/lib.rs | 4 +- examples/isomorphic/app/src/store.rs | 8 +- 12 files changed, 226 insertions(+), 126 deletions(-) delete mode 100644 examples/append-to-dom/README.md delete mode 100644 examples/append-to-dom/src/lib.rs delete mode 100755 examples/append-to-dom/start.sh rename examples/{append-to-dom => input-element-default-checkedness}/Cargo.toml (86%) create mode 100644 examples/input-element-default-checkedness/README.md create mode 100644 examples/input-element-default-checkedness/src/lib.rs create mode 100755 examples/input-element-default-checkedness/start.sh diff --git a/Cargo.toml b/Cargo.toml index 0d02bace..d620f304 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,8 +23,8 @@ members = [ "crates/percy-router-macro", "crates/percy-router-macro-test", "crates/virtual-node", - "examples/append-to-dom", "examples/component-preview", + "examples/input-element-default-checkedness", "examples/isomorphic/app", "examples/isomorphic/client", "examples/isomorphic/server", diff --git a/book/src/html-macro/special-attributes/README.md b/book/src/html-macro/special-attributes/README.md index b5a3b092..cd629439 100644 --- a/book/src/html-macro/special-attributes/README.md +++ b/book/src/html-macro/special-attributes/README.md @@ -19,8 +19,6 @@ This behavior is more desirable because `percy` developers are accustomed to dec ## `value` Attribute -(Note, this is virtually identical to the above section on the `checked` attribute.) - 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. diff --git a/crates/percy-dom/tests/checked_attribute.rs b/crates/percy-dom/tests/checked_attribute.rs index b3217253..1d23287a 100644 --- a/crates/percy-dom/tests/checked_attribute.rs +++ b/crates/percy-dom/tests/checked_attribute.rs @@ -1,11 +1,11 @@ -//! Verify that we treat `checked` where specified in the `html!` macro as -//! setting the checkedness, not as setting the `checked` HTML attribute (which -//! only determines the default checkedness). Developers, unless already aware, -//! are likely to assume that `checked` specifies the state of the checkbox -//! directly. `percy` ensures that this is true. +//! `percy-dom` treats the `checked` virtual node attribute specially. +//! `percy-dom` sets the `checked` element property (actual checkedness), as well as +//! the `checked` HTML attribute (default checkedness). //! -//! See the tests for more details. -//! Start with [`patch_uses_set_checked_function`]. +//! 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: //! @@ -43,10 +43,9 @@ wasm_bindgen_test_configure!(run_in_browser); /// /// ## Test approach /// -/// - Create a a DOM node with the checkbox having checkedness !C, and patch it to have -/// checkedness B. A and B may be the same or different, true or false. +/// - 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 DOM element has checkedness of B. +/// - 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 @@ -98,7 +97,7 @@ fn patch_sets_checked_property() { /// - 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` +/// If `percy_dom::diff` always specifies that `percy_dom::patch` should set the `checked` /// attribute 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. @@ -132,31 +131,3 @@ fn patch_always_sets_checked_property_and_attribute() { assert_eq!(input_elem.default_checked(), checkedness); } } - -/// Verify that `vnode.to_string()` includes the `checked` attribute -/// as a result of specifying `checked` in `percy`'s `html!` macro. -/// -/// ## Why? -/// -/// Note: the rationale given in [`patch_sets_checked_property`] is prerequisite reading. -/// -/// While `html!`'s `checked` configures the `checked` property to declaratively specify -/// the rendered checkbox state, -/// When performing server-side rendering, only the HTML reaches the client, not the -/// (non-existent) DOM state. Therefore the HTML attribute's presence should correspond -/// to `html!`'s `checked` state as well. -/// -/// ## Test approach -/// -/// - Create a a DOM node with the checkbox having some checkedness. -/// - Add or remove the checked attribute. -/// - Diff and patch with the virtual node with some new checkedness (same or different). -/// - Assert that the presence of the checked attribute has changed to match the new checkedness. -#[wasm_bindgen_test] -fn to_string_contains_checked_attribute() { - for (checkedness, html) in [(false, ""), (true, "")] { - let input = html! {}; - let string = input.to_string(); - assert_eq!(&string, html); - } -} diff --git a/examples/append-to-dom/README.md b/examples/append-to-dom/README.md deleted file mode 100644 index 0addccfc..00000000 --- a/examples/append-to-dom/README.md +++ /dev/null @@ -1,24 +0,0 @@ -# append-to-dom - -An example of manually managing and adding an HTML element into the `percy-dom` DOM tree. - -Such elements are outside `percy-dom`'s virtual DOM model and will not be affected by it (unless, for example, the parent element is removed). - -In this example, a checkbox that maintains a configured default checkedness (i.e. does not change the `checked` HTML attribute) despite changes to `HTMLInputElement.checked`. - -Note: This is a poor choice for server-side rendering and not useful for most client-side rendered `percy` applications. - -_Technical note: `percy` does not currently facilitate configuring a checkbox's default checkedness independently, instead setting both the `checked` attribute and property as it's seen to be the best default for typical users of `percy`, doing client-side or server-side rendering._ - - ---- - -## Running this example - -``` -git clone git@github.com:chinedufn/percy.git -cd percy - -cargo install wasm-pack -./examples/append-to-dom/start.sh -``` diff --git a/examples/append-to-dom/src/lib.rs b/examples/append-to-dom/src/lib.rs deleted file mode 100644 index eb9276ce..00000000 --- a/examples/append-to-dom/src/lib.rs +++ /dev/null @@ -1,50 +0,0 @@ -use percy_dom::{VirtualNode, JsCast, event::VirtualEvents, html}; -use web_sys::{HtmlElement, HtmlInputElement}; -use wasm_bindgen_test::*; - -wasm_bindgen_test_configure!(run_in_browser); - -#[wasm_bindgen_test] -pub fn main() { - // Create a div element that `percy` creates and updates. - let div = html! {
}; - - // Append to DOM from `VirtualNode`. - let mut events = VirtualEvents::new(); - let (div_node, enode) = div.create_dom_node(&mut events); - events.set_root(enode); - // Grab the div DOM element. - let div_elem = div_node.dyn_ref::().unwrap(); - - // Create a child checkbox node and append it to the DIV element. - let document = web_sys::window().unwrap().document().unwrap(); - let default_checked_elem = document.create_element("input").unwrap(); - default_checked_elem.set_attribute("type", "checkbox").unwrap(); - default_checked_elem.set_attribute("checked", "").unwrap(); - // Add our `percy`-agnostic checkbox as a child of a `percy`-controlled DOM element. - let child_elem = div_elem.append_child(&default_checked_elem).unwrap(); - - // Create a access the child element and confirm it has the `checked` attribute. - let child_checkbox_elem_ref = child_elem.dyn_ref::().unwrap(); - assert!(child_checkbox_elem_ref.has_attribute("checked")); - // Modify the checkedness of the child element. - // We will assert that it wasn't changed after a `percy-dom` update to ensure that `percy` isn't - // modifying our appended element in any way. - // Note that `percy` sets the `checked` attribute and property, whereas we're attempting to - // maintain a checkbox with an independent `checked` attribute (which means that the default - // checkedness is maintained: this might be useful for a form control that can be reset by the - // `HTMLFormElement.reset()` method, for example - however this would be unusual for a `percy` app.) - child_checkbox_elem_ref.set_checked(false); - - // Update the DOM according to the virtual node changes within the scope of `percy`. - let updated_div = html! {
}; - let patches = percy_dom::diff(&div, &updated_div); - percy_dom::patch(div_node.clone(), &updated_div, &mut events, &patches).unwrap(); - - // Get a reference to the child of the div element and confirm that is maintains the - // manually-specified attribute and property. - let child_elem = div_elem.children().get_with_index(0).unwrap(); - let child_input_elem_ref = child_elem.dyn_ref::().unwrap(); - assert!(child_input_elem_ref.has_attribute("checked")); - assert!(!child_input_elem_ref.checked()); -} diff --git a/examples/append-to-dom/start.sh b/examples/append-to-dom/start.sh deleted file mode 100755 index dc0edf39..00000000 --- a/examples/append-to-dom/start.sh +++ /dev/null @@ -1,3 +0,0 @@ -#!/bin/bash - -wasm-pack test --chrome --headless examples/append-to-dom diff --git a/examples/append-to-dom/Cargo.toml b/examples/input-element-default-checkedness/Cargo.toml similarity index 86% rename from examples/append-to-dom/Cargo.toml rename to examples/input-element-default-checkedness/Cargo.toml index a663c47b..c27ef2e8 100644 --- a/examples/append-to-dom/Cargo.toml +++ b/examples/input-element-default-checkedness/Cargo.toml @@ -1,5 +1,5 @@ [package] -name = "append-to-dom" +name = "input-element-default-checkedness" version = "0.1.0" edition = "2021" publish = [] diff --git a/examples/input-element-default-checkedness/README.md b/examples/input-element-default-checkedness/README.md new file mode 100644 index 00000000..2f6ed26f --- /dev/null +++ b/examples/input-element-default-checkedness/README.md @@ -0,0 +1,38 @@ +# input-element-default-checkedness + +This example demonstrates an input element that maintains a **explicitly configured default checkedness** (i.e. does not change the `checked` HTML attribute) despite changes to `HTMLInputElement.checked`. + +`percy` does not support explicitly setting the default checkedness ([why?](#why-does-percy-override-default-checkedness)), so this also serves as an example of **appending independently-managed DOM elements to 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 removed). + +--- + +### When Should I Configure Default Checkedness Independently of Checkedness? + +This is irrelevant to server-side rendering, where there is no server-side DOM and only the HTML attributes are sent. + +This is not generally useful for client side rendering, as the default only matters if you want to configure what happens when checkedness isn't specified and `HTMLFormElement.reset()` is called or similarly, a reset element is pressed (which changes the element's checkedness to the default checkedness). +- Most `percy` applications are expected to have a client-side application state, with GUI reflecting that app state. This is in contrast to UI-driven apps (which often scale poorly as complexity increases). In this case, the checkedness of an input is always specified according to app state. +- Calling external functions that independently affect DOM elements such as `HTMLFormElement.reset()` may cause the DOM state may desync from the virtual DOM and app state, and hence is typically best avoided. + +--- + +### Why Does `percy` Override Default Checkedness? + +- Server-side rendering sends HTML and HTML attributes only, no DOM properties. Therefore `percy`'s `virtual-node` needs to set the `checked` attribute (default checkedness) for server side rendering, using `VirtualNode`'s `Display` implementation i.e. `virtual_node.to_string()` +- `percy-dom` is intended for client-side rendering, but it overrides the default checkedness to match `VirtualNode`'s `Display` implementation. + +--- + +## 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/input-element-default-checkedness/start.sh +GECKODRIVER=geckodriver ./examples/input-element-default-checkedness/start.sh +SAFARIDRIVER=safaridriver ./examples/input-element-default-checkedness/start.sh +``` + +You may need to install the appropriate driver and (optionally) add it to your `PATH`. diff --git a/examples/input-element-default-checkedness/src/lib.rs b/examples/input-element-default-checkedness/src/lib.rs new file mode 100644 index 00000000..5d2004fd --- /dev/null +++ b/examples/input-element-default-checkedness/src/lib.rs @@ -0,0 +1,167 @@ +use percy_dom::{event::VirtualEvents, html, IterableNodes, JsCast, VElement, VirtualNode}; +use wasm_bindgen_test::*; +use web_sys::{HtmlInputElement, Node}; + +wasm_bindgen_test_configure!(run_in_browser); + +fn create_my_default_checked_checkbox() -> HtmlInputElement { + let document = web_sys::window().unwrap().document().unwrap(); + + let checkbox = document + .create_element("input") + .unwrap() + .dyn_into::() + .unwrap(); + checkbox.set_id("my_default_checked_checkbox"); + checkbox.set_type("checkbox"); + checkbox.set_default_checked(true); + checkbox.set_checked(true); + + checkbox +} + +fn setup_percy_dom_with_appended_child_checkbox( +) -> (VirtualNode, Node, VirtualEvents, HtmlInputElement) { + // This will be a checkbox that `percy-dom` controls. + let vdom_percy_checkbox = html! { + + }; + + let my_default_checked_checkbox = create_my_default_checked_checkbox(); + let my_default_checked_checkbox_append = my_default_checked_checkbox.clone(); + + // Manually create a container VirtualNode that will append my-default-checked-checkbox + // onto its corresponding DOM element upon creation. + let mut vdom_checkbox_holder = VElement::new("div"); + vdom_checkbox_holder + .attrs + .insert("id".into(), "checkbox_holder".into()); + vdom_checkbox_holder.children.push(vdom_percy_checkbox); + vdom_checkbox_holder + .special_attributes + .set_on_create_element("key", move |e| { + e.append_child(&my_default_checked_checkbox_append).unwrap(); + }); + + // Parent the container to some other node for the sake of example. + let vdom_root_node = html! { +
{ VirtualNode::Element(vdom_checkbox_holder) }
+ }; + + // 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_default_checked_checkbox, + ) +} + +fn uncheck_both_checkboxes( + vdom_root: &VirtualNode, + dom_root: Node, + events: &mut VirtualEvents, + my_default_checked_checkbox: HtmlInputElement, +) { + // Update the default-checked checkbox we maintain to be unchecked + my_default_checked_checkbox.set_checked(false); + + // Update the percy checkbox to unchecked + 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(); +} + +#[wasm_bindgen_test] +fn conduct_checked_modification_test() { + // Setup both checkboxes and create the DOM node tree. + let (vdom_root, dom_root, mut events, my_checkbox) = + setup_percy_dom_with_appended_child_checkbox(); + + // This allows us to use `document.get_element_by_id("...")` which is convenient for testing purposes. + // This is NOT necessary. + web_sys::window() + .unwrap() + .document() + .unwrap() + .body() + .unwrap() + .append_child(&dom_root) + .unwrap(); + + ensure_percy_and_default_checked_checkboxes_are_checked(); + + uncheck_both_checkboxes(&vdom_root, dom_root, &mut events, my_checkbox); + + // Both checkboxes should now be unchecked. + ensure_checkboxes_are_unchecked(); + + // `percy` should have overridden the default checkedness to match the checkedness. + ensure_percy_checkbox_is_no_longer_default_checked(); + + // My default-checked checkbox should still be checked by default, + // as we didn't override default checkedness ourselves. + ensure_my_default_checked_checkbox_is_still_default_checked(); +} + +fn ensure_percy_and_default_checked_checkboxes_are_checked() { + let document = web_sys::window().unwrap().document().unwrap(); + + let percy_checkbox = document.get_element_by_id("percy_checkbox").unwrap(); + let percy_checkbox_ref = percy_checkbox.dyn_ref::().unwrap(); + assert!(percy_checkbox_ref.checked()); + + let default_checked_checkbox = document + .get_element_by_id("my_default_checked_checkbox") + .unwrap(); + let default_checked_checkbox_ref = default_checked_checkbox + .dyn_ref::() + .unwrap(); + assert!(default_checked_checkbox_ref.checked()); +} + +fn ensure_checkboxes_are_unchecked() { + let document = web_sys::window().unwrap().document().unwrap(); + + let percy_checkbox = document.get_element_by_id("percy_checkbox").unwrap(); + let percy_checkbox_ref = percy_checkbox.dyn_ref::().unwrap(); + assert!(!percy_checkbox_ref.checked()); + + let default_checked_checkbox = document + .get_element_by_id("my_default_checked_checkbox") + .unwrap(); + let default_checked_checkbox_ref = default_checked_checkbox + .dyn_ref::() + .unwrap(); + assert!(!default_checked_checkbox_ref.checked()); +} + +fn ensure_percy_checkbox_is_no_longer_default_checked() { + let document = web_sys::window().unwrap().document().unwrap(); + + let percy_checkbox = document.get_element_by_id("percy_checkbox").unwrap(); + let percy_checkbox_ref = percy_checkbox.dyn_ref::().unwrap(); + assert!(!percy_checkbox_ref.default_checked()); +} + +fn ensure_my_default_checked_checkbox_is_still_default_checked() { + let document = web_sys::window().unwrap().document().unwrap(); + + let default_checked_checkbox = document + .get_element_by_id("my_default_checked_checkbox") + .unwrap(); + let default_checked_checkbox_ref = default_checked_checkbox + .dyn_ref::() + .unwrap(); + assert!(default_checked_checkbox_ref.default_checked()); +} diff --git a/examples/input-element-default-checkedness/start.sh b/examples/input-element-default-checkedness/start.sh new file mode 100755 index 00000000..1495ef13 --- /dev/null +++ b/examples/input-element-default-checkedness/start.sh @@ -0,0 +1,3 @@ +#!/bin/bash + +cargo test -p input-element-default-checkedness --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); } From a6c9e2a7528525c2ced358fcf2074a541d6d8dac Mon Sep 17 00:00:00 2001 From: Shaun Beautement Date: Sat, 28 Sep 2024 23:06:46 +0200 Subject: [PATCH 5/9] improve example test name and docs --- examples/input-element-default-checkedness/src/lib.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/examples/input-element-default-checkedness/src/lib.rs b/examples/input-element-default-checkedness/src/lib.rs index 5d2004fd..d74cac00 100644 --- a/examples/input-element-default-checkedness/src/lib.rs +++ b/examples/input-element-default-checkedness/src/lib.rs @@ -82,8 +82,12 @@ fn uncheck_both_checkboxes( percy_dom::patch(dom_root.clone(), &new_vdom_root, events, &patches).unwrap(); } +/// Verify that the `percy-dom`-controlled checkbox, `percy_checkbox`, and the +/// `my_default_checked_checkbox` behave exactly as we expect when updating the checkedness: +/// - Both `percy_checkbox`'s checkedness and default checkedness should change. +/// - Only `my_default_checked_checkbox`'s checkedness should change, not default checkedness. #[wasm_bindgen_test] -fn conduct_checked_modification_test() { +fn checkbox_checkedness_update_test() { // Setup both checkboxes and create the DOM node tree. let (vdom_root, dom_root, mut events, my_checkbox) = setup_percy_dom_with_appended_child_checkbox(); From 8e08e79e89e6b0479da4804b6e0031898ebb8791 Mon Sep 17 00:00:00 2001 From: Shaun Beautement Date: Sat, 28 Sep 2024 23:25:27 +0200 Subject: [PATCH 6/9] simplify example --- .../src/lib.rs | 36 ++++++++----------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/examples/input-element-default-checkedness/src/lib.rs b/examples/input-element-default-checkedness/src/lib.rs index d74cac00..a548317b 100644 --- a/examples/input-element-default-checkedness/src/lib.rs +++ b/examples/input-element-default-checkedness/src/lib.rs @@ -1,6 +1,6 @@ -use percy_dom::{event::VirtualEvents, html, IterableNodes, JsCast, VElement, VirtualNode}; +use percy_dom::{event::VirtualEvents, html, JsCast, VirtualNode}; use wasm_bindgen_test::*; -use web_sys::{HtmlInputElement, Node}; +use web_sys::{Element, HtmlInputElement, Node}; wasm_bindgen_test_configure!(run_in_browser); @@ -22,30 +22,22 @@ fn create_my_default_checked_checkbox() -> HtmlInputElement { fn setup_percy_dom_with_appended_child_checkbox( ) -> (VirtualNode, Node, VirtualEvents, HtmlInputElement) { - // This will be a checkbox that `percy-dom` controls. - let vdom_percy_checkbox = html! { - - }; - let my_default_checked_checkbox = create_my_default_checked_checkbox(); let my_default_checked_checkbox_append = my_default_checked_checkbox.clone(); - // Manually create a container VirtualNode that will append my-default-checked-checkbox - // onto its corresponding DOM element upon creation. - let mut vdom_checkbox_holder = VElement::new("div"); - vdom_checkbox_holder - .attrs - .insert("id".into(), "checkbox_holder".into()); - vdom_checkbox_holder.children.push(vdom_percy_checkbox); - vdom_checkbox_holder - .special_attributes - .set_on_create_element("key", move |e| { - e.append_child(&my_default_checked_checkbox_append).unwrap(); - }); - - // Parent the container to some other node for the sake of example. + // This will be a checkbox that `percy-dom` controls. let vdom_root_node = html! { -
{ VirtualNode::Element(vdom_checkbox_holder) }
+
+
+ +
+
}; // Create the DOM nodes from the virtual DOM. From 6253741e72d2073f512269e77dfaa898565088d9 Mon Sep 17 00:00:00 2001 From: Shaun Beautement Date: Sun, 29 Sep 2024 14:32:41 +0200 Subject: [PATCH 7/9] revert setting default checkedness, remove example --- Cargo.toml | 1 - .../html-macro/special-attributes/README.md | 18 +- crates/percy-dom/src/patch/apply_patches.rs | 25 +-- ...ecked_attribute.rs => checked_property.rs} | 73 ++++++-- .../Cargo.toml | 19 -- .../README.md | 38 ---- .../src/lib.rs | 163 ------------------ .../start.sh | 3 - 8 files changed, 81 insertions(+), 259 deletions(-) rename crates/percy-dom/tests/{checked_attribute.rs => checked_property.rs} (65%) delete mode 100644 examples/input-element-default-checkedness/Cargo.toml delete mode 100644 examples/input-element-default-checkedness/README.md delete mode 100644 examples/input-element-default-checkedness/src/lib.rs delete mode 100755 examples/input-element-default-checkedness/start.sh diff --git a/Cargo.toml b/Cargo.toml index d620f304..a491c0de 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,7 +24,6 @@ members = [ "crates/percy-router-macro-test", "crates/virtual-node", "examples/component-preview", - "examples/input-element-default-checkedness", "examples/isomorphic/app", "examples/isomorphic/client", "examples/isomorphic/server", diff --git a/book/src/html-macro/special-attributes/README.md b/book/src/html-macro/special-attributes/README.md index cd629439..877b16e0 100644 --- a/book/src/html-macro/special-attributes/README.md +++ b/book/src/html-macro/special-attributes/README.md @@ -1,8 +1,8 @@ -# Special Attributes +# Special Virtual DOM Attributes -Some attributes do not merely set or remove the corresponding HTML attribute of the same name. +Some virtual DOM attributes do not merely set or remove the corresponding HTML attribute of the same name. -## `checked` Attribute +## `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. @@ -11,13 +11,15 @@ By contrast: specifying `html! { }` causes `percy` to a - 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` updates both -- the `checked` attribute (default checkedness, reflected in HTML) and, -- the `checked` property (current checkedness, not reflected in HTML). +`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. -## `value` Attribute +`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. @@ -30,4 +32,4 @@ By contrast: specifying `html! { }` causes `percy` to alwa - the `value` attribute (default value, reflected in HTML) and, - the `value` property (current value, not reflected in HTML). -This behavior is more desirable because `percy` developers are accustomed to declaratively controlling the DOM and rendered 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/src/patch/apply_patches.rs b/crates/percy-dom/src/patch/apply_patches.rs index 7a66c582..026b1fc3 100644 --- a/crates/percy-dom/src/patch/apply_patches.rs +++ b/crates/percy-dom/src/patch/apply_patches.rs @@ -241,18 +241,16 @@ fn apply_element_patch( } } AttributeValue::Bool(val_bool) => { - if *val_bool { - node.set_attribute(attrib_name, "")?; - } else { - node.remove_attribute(attrib_name)?; - } - - // Use `set_checked` in addition to `{set,remove}_attribute` for the `checked` attribute. - // The "checked" attribute determines default checkedness, - // but `percy` interprets `checked` as determining current checkedness too. + // 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)?; } } } @@ -399,14 +397,7 @@ fn apply_element_patch( Ok(()) } Patch::CheckedAttributeUnchanged(_node_idx, value) => { - let value = value.as_bool().unwrap(); - maybe_set_checked_property(node, value); - - if value { - node.set_attribute("checked", "")?; - } else { - node.remove_attribute("checked")?; - } + maybe_set_checked_property(node, value.as_bool().unwrap()); Ok(()) } diff --git a/crates/percy-dom/tests/checked_attribute.rs b/crates/percy-dom/tests/checked_property.rs similarity index 65% rename from crates/percy-dom/tests/checked_attribute.rs rename to crates/percy-dom/tests/checked_property.rs index 1d23287a..2389e1fe 100644 --- a/crates/percy-dom/tests/checked_attribute.rs +++ b/crates/percy-dom/tests/checked_property.rs @@ -1,6 +1,6 @@ //! `percy-dom` treats the `checked` virtual node attribute specially. -//! `percy-dom` sets the `checked` element property (actual checkedness), as well as -//! the `checked` HTML attribute (default checkedness). +//! `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. @@ -9,7 +9,9 @@ //! //! To run all tests in this file: //! -//! wasm-pack test --chrome --headless crates/percy-dom --test checked_attribute +//! ```sh +//! wasm-pack test --chrome --headless crates/percy-dom --test checked_property +//! ``` use wasm_bindgen_test::*; @@ -26,7 +28,7 @@ wasm_bindgen_test_configure!(run_in_browser); /// /// ## Why? /// -/// The `checked` HTML attribute, only determines the default checkedness. +/// 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 @@ -35,7 +37,7 @@ wasm_bindgen_test_configure!(run_in_browser); /// 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 rendering in a different state to what the developer +/// 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 @@ -77,7 +79,7 @@ fn patch_sets_checked_property() { } } -/// Verify that `percy_dom::patch` uses `set_checked` to set the `checked` attribute +/// 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. /// @@ -98,7 +100,7 @@ fn patch_sets_checked_property() { /// stays unchecked. /// /// If `percy_dom::diff` always specifies that `percy_dom::patch` should set the `checked` -/// attribute if its specified, then the above cannot happen. The element's checked state +/// 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. /// @@ -109,7 +111,7 @@ fn patch_sets_checked_property() { /// - 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_and_attribute() { +fn patch_always_sets_checked_property() { for checkedness in [false, true] { let start_input = html! {}; let end_input = html! {}; @@ -122,12 +124,63 @@ fn patch_always_sets_checked_property_and_attribute() { assert_eq!(input_elem.checked(), checkedness); input_elem.set_checked(!checkedness); // modify checked property - input_elem.set_default_checked(!checkedness); // modify checked attribute 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); - assert_eq!(input_elem.default_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/examples/input-element-default-checkedness/Cargo.toml b/examples/input-element-default-checkedness/Cargo.toml deleted file mode 100644 index c27ef2e8..00000000 --- a/examples/input-element-default-checkedness/Cargo.toml +++ /dev/null @@ -1,19 +0,0 @@ -[package] -name = "input-element-default-checkedness" -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/input-element-default-checkedness/README.md b/examples/input-element-default-checkedness/README.md deleted file mode 100644 index 2f6ed26f..00000000 --- a/examples/input-element-default-checkedness/README.md +++ /dev/null @@ -1,38 +0,0 @@ -# input-element-default-checkedness - -This example demonstrates an input element that maintains a **explicitly configured default checkedness** (i.e. does not change the `checked` HTML attribute) despite changes to `HTMLInputElement.checked`. - -`percy` does not support explicitly setting the default checkedness ([why?](#why-does-percy-override-default-checkedness)), so this also serves as an example of **appending independently-managed DOM elements to 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 removed). - ---- - -### When Should I Configure Default Checkedness Independently of Checkedness? - -This is irrelevant to server-side rendering, where there is no server-side DOM and only the HTML attributes are sent. - -This is not generally useful for client side rendering, as the default only matters if you want to configure what happens when checkedness isn't specified and `HTMLFormElement.reset()` is called or similarly, a reset element is pressed (which changes the element's checkedness to the default checkedness). -- Most `percy` applications are expected to have a client-side application state, with GUI reflecting that app state. This is in contrast to UI-driven apps (which often scale poorly as complexity increases). In this case, the checkedness of an input is always specified according to app state. -- Calling external functions that independently affect DOM elements such as `HTMLFormElement.reset()` may cause the DOM state may desync from the virtual DOM and app state, and hence is typically best avoided. - ---- - -### Why Does `percy` Override Default Checkedness? - -- Server-side rendering sends HTML and HTML attributes only, no DOM properties. Therefore `percy`'s `virtual-node` needs to set the `checked` attribute (default checkedness) for server side rendering, using `VirtualNode`'s `Display` implementation i.e. `virtual_node.to_string()` -- `percy-dom` is intended for client-side rendering, but it overrides the default checkedness to match `VirtualNode`'s `Display` implementation. - ---- - -## 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/input-element-default-checkedness/start.sh -GECKODRIVER=geckodriver ./examples/input-element-default-checkedness/start.sh -SAFARIDRIVER=safaridriver ./examples/input-element-default-checkedness/start.sh -``` - -You may need to install the appropriate driver and (optionally) add it to your `PATH`. diff --git a/examples/input-element-default-checkedness/src/lib.rs b/examples/input-element-default-checkedness/src/lib.rs deleted file mode 100644 index a548317b..00000000 --- a/examples/input-element-default-checkedness/src/lib.rs +++ /dev/null @@ -1,163 +0,0 @@ -use percy_dom::{event::VirtualEvents, html, JsCast, VirtualNode}; -use wasm_bindgen_test::*; -use web_sys::{Element, HtmlInputElement, Node}; - -wasm_bindgen_test_configure!(run_in_browser); - -fn create_my_default_checked_checkbox() -> HtmlInputElement { - let document = web_sys::window().unwrap().document().unwrap(); - - let checkbox = document - .create_element("input") - .unwrap() - .dyn_into::() - .unwrap(); - checkbox.set_id("my_default_checked_checkbox"); - checkbox.set_type("checkbox"); - checkbox.set_default_checked(true); - checkbox.set_checked(true); - - checkbox -} - -fn setup_percy_dom_with_appended_child_checkbox( -) -> (VirtualNode, Node, VirtualEvents, HtmlInputElement) { - let my_default_checked_checkbox = create_my_default_checked_checkbox(); - let my_default_checked_checkbox_append = my_default_checked_checkbox.clone(); - - // This will be a checkbox that `percy-dom` controls. - 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_default_checked_checkbox, - ) -} - -fn uncheck_both_checkboxes( - vdom_root: &VirtualNode, - dom_root: Node, - events: &mut VirtualEvents, - my_default_checked_checkbox: HtmlInputElement, -) { - // Update the default-checked checkbox we maintain to be unchecked - my_default_checked_checkbox.set_checked(false); - - // Update the percy checkbox to unchecked - 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(); -} - -/// Verify that the `percy-dom`-controlled checkbox, `percy_checkbox`, and the -/// `my_default_checked_checkbox` behave exactly as we expect when updating the checkedness: -/// - Both `percy_checkbox`'s checkedness and default checkedness should change. -/// - Only `my_default_checked_checkbox`'s checkedness should change, not default checkedness. -#[wasm_bindgen_test] -fn checkbox_checkedness_update_test() { - // Setup both checkboxes and create the DOM node tree. - let (vdom_root, dom_root, mut events, my_checkbox) = - setup_percy_dom_with_appended_child_checkbox(); - - // This allows us to use `document.get_element_by_id("...")` which is convenient for testing purposes. - // This is NOT necessary. - web_sys::window() - .unwrap() - .document() - .unwrap() - .body() - .unwrap() - .append_child(&dom_root) - .unwrap(); - - ensure_percy_and_default_checked_checkboxes_are_checked(); - - uncheck_both_checkboxes(&vdom_root, dom_root, &mut events, my_checkbox); - - // Both checkboxes should now be unchecked. - ensure_checkboxes_are_unchecked(); - - // `percy` should have overridden the default checkedness to match the checkedness. - ensure_percy_checkbox_is_no_longer_default_checked(); - - // My default-checked checkbox should still be checked by default, - // as we didn't override default checkedness ourselves. - ensure_my_default_checked_checkbox_is_still_default_checked(); -} - -fn ensure_percy_and_default_checked_checkboxes_are_checked() { - let document = web_sys::window().unwrap().document().unwrap(); - - let percy_checkbox = document.get_element_by_id("percy_checkbox").unwrap(); - let percy_checkbox_ref = percy_checkbox.dyn_ref::().unwrap(); - assert!(percy_checkbox_ref.checked()); - - let default_checked_checkbox = document - .get_element_by_id("my_default_checked_checkbox") - .unwrap(); - let default_checked_checkbox_ref = default_checked_checkbox - .dyn_ref::() - .unwrap(); - assert!(default_checked_checkbox_ref.checked()); -} - -fn ensure_checkboxes_are_unchecked() { - let document = web_sys::window().unwrap().document().unwrap(); - - let percy_checkbox = document.get_element_by_id("percy_checkbox").unwrap(); - let percy_checkbox_ref = percy_checkbox.dyn_ref::().unwrap(); - assert!(!percy_checkbox_ref.checked()); - - let default_checked_checkbox = document - .get_element_by_id("my_default_checked_checkbox") - .unwrap(); - let default_checked_checkbox_ref = default_checked_checkbox - .dyn_ref::() - .unwrap(); - assert!(!default_checked_checkbox_ref.checked()); -} - -fn ensure_percy_checkbox_is_no_longer_default_checked() { - let document = web_sys::window().unwrap().document().unwrap(); - - let percy_checkbox = document.get_element_by_id("percy_checkbox").unwrap(); - let percy_checkbox_ref = percy_checkbox.dyn_ref::().unwrap(); - assert!(!percy_checkbox_ref.default_checked()); -} - -fn ensure_my_default_checked_checkbox_is_still_default_checked() { - let document = web_sys::window().unwrap().document().unwrap(); - - let default_checked_checkbox = document - .get_element_by_id("my_default_checked_checkbox") - .unwrap(); - let default_checked_checkbox_ref = default_checked_checkbox - .dyn_ref::() - .unwrap(); - assert!(default_checked_checkbox_ref.default_checked()); -} diff --git a/examples/input-element-default-checkedness/start.sh b/examples/input-element-default-checkedness/start.sh deleted file mode 100755 index 1495ef13..00000000 --- a/examples/input-element-default-checkedness/start.sh +++ /dev/null @@ -1,3 +0,0 @@ -#!/bin/bash - -cargo test -p input-element-default-checkedness --target wasm32-unknown-unknown From 8e11bbe4415956f0bafc77e1cc1aae686e8e11ca Mon Sep 17 00:00:00 2001 From: Shaun Beautement Date: Sun, 29 Sep 2024 15:33:14 +0200 Subject: [PATCH 8/9] add embed-non-percy-node example, version bump `percy-dom` to 0.10.0 --- Cargo.toml | 1 + crates/percy-dom/Cargo.toml | 2 +- examples/embed-non-percy-node/Cargo.toml | 16 +++++ examples/embed-non-percy-node/README.md | 19 ++++++ examples/embed-non-percy-node/src/lib.rs | 82 ++++++++++++++++++++++++ examples/embed-non-percy-node/start.sh | 3 + 6 files changed, 122 insertions(+), 1 deletion(-) create mode 100644 examples/embed-non-percy-node/Cargo.toml create mode 100644 examples/embed-non-percy-node/README.md create mode 100644 examples/embed-non-percy-node/src/lib.rs create mode 100755 examples/embed-non-percy-node/start.sh 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/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/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..6026a2b5 --- /dev/null +++ b/examples/embed-non-percy-node/src/lib.rs @@ -0,0 +1,82 @@ +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 foreign 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 foreign elements are preserved across `percy_dom::patch` calls. +#[wasm_bindgen_test] +fn checkbox_checkedness_update_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_div_element = create_my_special_paragraph_element(); + let my_div_element_append = my_div_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_div_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 From 7099525df9ca78cb9ad9baf1b7f51bb578151046 Mon Sep 17 00:00:00 2001 From: Shaun Beautement Date: Sun, 29 Sep 2024 15:37:53 +0200 Subject: [PATCH 9/9] fix naming in example --- examples/embed-non-percy-node/src/lib.rs | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/examples/embed-non-percy-node/src/lib.rs b/examples/embed-non-percy-node/src/lib.rs index 6026a2b5..936d7f25 100644 --- a/examples/embed-non-percy-node/src/lib.rs +++ b/examples/embed-non-percy-node/src/lib.rs @@ -7,14 +7,14 @@ wasm_bindgen_test_configure!(run_in_browser); /// Verify that the /// - `percy-dom`-controlled element, `root`, -/// - and the foreign DOM element, `my_special_paragraph` +/// - 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 foreign elements are preserved across `percy_dom::patch` calls. +/// - The state of the embedded elements are preserved across `percy_dom::patch` calls. #[wasm_bindgen_test] -fn checkbox_checkedness_update_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(); @@ -51,15 +51,15 @@ fn create_my_special_paragraph_element() -> Element { } fn setup_percy_dom_with_embedded_element() -> (VirtualNode, Node, VirtualEvents, Element) { - let my_div_element = create_my_special_paragraph_element(); - let my_div_element_append = my_div_element.clone(); + 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! {
}; @@ -68,7 +68,12 @@ fn setup_percy_dom_with_embedded_element() -> (VirtualNode, Node, VirtualEvents, 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_div_element) + ( + vdom_root_node, + root_node, + events, + my_special_paragraph_element, + ) } fn percy_diff_and_patch_root_node(