Change html-macro's checked attribute to specify checkedness

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

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)

book/src/html-macro/special-attributes/README.md

@@ -0,0 +1,35 @@
+# Special Attributes
+
+Some attributes do not merely set or remove the corresponding HTML attribute of the same name.
+
+## `checked` Attribute
+
+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! { <input checked={bool} /> }` causes `percy` to always render the checkbox with the specified checkedness.
+- If the VDOM is updated from `html! { <input checked=true /> }` to `html { <input checked=false /> }`, the input element's checkedness will definitely change.
+- If the VDOM is updated from `html! { <input checked=true /> }` to `html { <input checked=true /> }`, 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).
+
+This behavior is more desirable because `percy` developers are accustomed to declaratively controlling the DOM and rendered HTML.
+
+## `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.
+
+By contrast: specifying `html! { <input value="..." /> }` causes `percy` to always render the input element with the specified value.
+- If the VDOM is updated from `html! { <input value="hello" /> }` to `html { <input value="goodbye" /> }`, the input element's value will definitely change.
+- If the VDOM is updated from `html! { <input value="hello" /> }` to `html { <input value="hello" /> }`, 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).
+
+This behavior is more desirable because `percy` developers are accustomed to declaratively controlling the DOM and rendered HTML.

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! { <input checked={checkedness} /> },
+                new: html! { <input checked={checkedness} /> },
+                expected: vec![Patch::CheckedAttributeUnchanged(0, &checkedness.into())],
+            }
+            .test();
+        }
+
+        for old_checkedness in [false, true] {
+            let new_checkedness = !old_checkedness;
+
+            DiffTestCase {
+                old: html! { <input checked=old_checkedness /> },
+                new: html! { <input checked=new_checkedness /> },
+                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]

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);

crates/percy-dom/src/patch/apply_patches.rs

@@ -246,6 +246,14 @@ fn apply_element_patch(
                         } 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.
+                        // See crates/percy-dom/tests/checked_attribute.rs for more info.
+                        if *attrib_name == "checked" {
+                            maybe_set_checked_property(node, *val_bool);
+                        }
                     }
                 }
             }
@@ -390,6 +398,18 @@ 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")?;
+            }
+
+            Ok(())
+        }
         Patch::SpecialAttribute(special) => match special {
             PatchSpecialAttribute::CallOnCreateElemOnExistingNode(_node_idx, new_node) => {
                 new_node
@@ -512,6 +532,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::<HtmlInputElement>() {
+        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

crates/percy-dom/tests/checked_attribute.rs

@@ -0,0 +1,162 @@
+//! 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 current checkedness of the checkbox
+/// directly - avoiding the checkbox rendering in a different state 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 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_sets_checked_property() {
+    for checkedness in [false, true] {
+        let start_input = html! {<input checked={!checkedness}>};
+        let end_input = html! {<input checked=checkedness>};
+
+        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::<HtmlInputElement>().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_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`
+/// 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_property_and_attribute() {
+    for checkedness in [false, true] {
+        let start_input = html! {<input checked=checkedness>};
+        let end_input = html! {<input checked=checkedness>};
+
+        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::<HtmlInputElement>().unwrap();
+        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 `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, "<input>"), (true, "<input checked>")] {
+        let input = html! {<input checked=checkedness>};
+        let string = input.to_string();
+        assert_eq!(&string, html);
+    }
+}

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 {

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",
+]