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/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! { <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.
 
-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! { <input type="checkbox" checked=true> };
-```
-
-It's still possible to use `elem.set_attribute("checked", "")` and `elem.remove_attribute("checked")` to configure the default checkedness.
-
-```rust
-let vnode = html! {<input type="checkbox">};
-
-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::<HtmlInputElement>().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! { <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.
 
-```rust
-html! { <input value="hello!"> };
-```
+`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.

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! { <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/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(())
         }

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! {<input checked={!checkedness}>};
         let end_input = html! {<input checked=checkedness>};
@@ -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! {<input checked=checkedness>};
         let end_input = html! {<input checked=checkedness>};
@@ -122,67 +122,41 @@ fn patch_always_sets_checked() {
         let input_elem = input_node.dyn_ref::<HtmlInputElement>().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! {<input checked=old_checkedness>};
-                let end_input = html! {<input checked=new_checkedness>};
-
-                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::<HtmlInputElement>().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, "<input>"), (true, "<input checked>")] {
+        let input = html! {<input checked=checkedness>};
+        let string = input.to_string();
+        assert_eq!(&string, html);
     }
 }

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

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 [email protected]:chinedufn/percy.git
+cd percy
+
+cargo install wasm-pack
+./examples/append-to-dom/start.sh
+```

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! { <div id="my_div" /> };
+
+    // 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::<HtmlElement>().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::<HtmlInputElement>().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! { <div id="my_div" data-custom="data" /> };
+    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::<HtmlInputElement>().unwrap();
+    assert!(child_input_elem_ref.has_attribute("checked"));
+    assert!(!child_input_elem_ref.checked());
+}

examples/append-to-dom/start.sh

@@ -0,0 +1,3 @@
+#!/bin/bash
+
+wasm-pack test --chrome --headless examples/append-to-dom