From 8f85c19d3b84723cb37797e887c5fa0a5a9a04e2 Mon Sep 17 00:00:00 2001 From: liquidblock Date: Thu, 16 Apr 2020 18:21:00 +0200 Subject: [PATCH 01/14] Prevent setting empty class string in DOM --- src/virtual_dom/vtag.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/virtual_dom/vtag.rs b/src/virtual_dom/vtag.rs index 5d6ca89ca33..9fd7b28ed0e 100644 --- a/src/virtual_dom/vtag.rs +++ b/src/virtual_dom/vtag.rs @@ -213,7 +213,7 @@ impl VTag { if ancestor .as_ref() .map(|ancestor| self.classes.ne(&ancestor.classes)) - .unwrap_or(true) + .unwrap_or_else(|| !self.classes.is_empty()) { Some(self.classes.to_string()) } else { From 41bfbc3218ce17310d7b9fa6ac99cb0049a34911 Mon Sep 17 00:00:00 2001 From: liquidblock Date: Thu, 16 Apr 2020 18:46:29 +0200 Subject: [PATCH 02/14] Use the className property for setting classes in DOM --- src/virtual_dom/vtag.rs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/virtual_dom/vtag.rs b/src/virtual_dom/vtag.rs index 9fd7b28ed0e..096cef24dab 100644 --- a/src/virtual_dom/vtag.rs +++ b/src/virtual_dom/vtag.rs @@ -131,14 +131,14 @@ impl VTag { } /// Adds a single class to this virtual node. Actually it will set by - /// [Element.setAttribute](https://developer.mozilla.org/en-US/docs/Web/API/Element/setAttribute) + /// [Element.className](https://developer.mozilla.org/en-US/docs/Web/API/Element/className) /// call later. pub fn add_class(&mut self, class: &str) { self.classes.push(class); } /// Adds multiple classes to this virtual node. Actually it will set by - /// [Element.setAttribute](https://developer.mozilla.org/en-US/docs/Web/API/Element/setAttribute) + /// [Element.className](https://developer.mozilla.org/en-US/docs/Web/API/Element/className) /// call later. pub fn add_classes(&mut self, classes: Vec<&str>) { for class in classes { @@ -147,7 +147,7 @@ impl VTag { } /// Add classes to this virtual node. Actually it will set by - /// [Element.setAttribute](https://developer.mozilla.org/en-US/docs/Web/API/Element/setAttribute) + /// [Element.className](https://developer.mozilla.org/en-US/docs/Web/API/Element/className) /// call later. pub fn set_classes(&mut self, classes: impl Into) { self.classes = classes.into(); @@ -296,9 +296,7 @@ impl VTag { // Update parameters let class_str = self.diff_classes(ancestor); if let Some(class_str) = class_str { - element - .set_attribute("class", &class_str) - .expect("could not set class"); + set_class_name(element, &class_str); } let changes = self.diff_attributes(ancestor); @@ -520,6 +518,14 @@ fn set_checked(input: &InputElement, value: bool) { }; } +/// Set `className` value for the `Element`. +fn set_class_name(input: &Element, value: &str) { + cfg_match! { + feature = "std_web" => js!( @(no_return) @{input}.className = @{value}; ), + feature = "web_sys" => input.set_class_name(value), + }; +} + impl PartialEq for VTag { fn eq(&self, other: &VTag) -> bool { self.tag == other.tag From 4bba674bb95bd63694f287afa20df328e423e980 Mon Sep 17 00:00:00 2001 From: liquidblock Date: Thu, 16 Apr 2020 19:04:45 +0200 Subject: [PATCH 03/14] Update documentation of VTag::diff_classes --- src/virtual_dom/vtag.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/virtual_dom/vtag.rs b/src/virtual_dom/vtag.rs index 096cef24dab..25a04dfac4e 100644 --- a/src/virtual_dom/vtag.rs +++ b/src/virtual_dom/vtag.rs @@ -205,8 +205,9 @@ impl VTag { } } - /// If there is no ancestor or the classes, or the order, differs from the ancestor: - /// - Returns the classes of self separated by spaces. + /// Returns the classes of self separated by spaces if and only if + /// - the classes, or the order, differs from the ancestor + /// - or there is no ancestor and the classes are not empty /// /// Otherwise None is returned. fn diff_classes<'a>(&'a self, ancestor: &'a Option>) -> Option { From c5e555d408a8daa41dfdc35e7f1969c357015e6f Mon Sep 17 00:00:00 2001 From: liquidblock Date: Fri, 17 Apr 2020 21:15:07 +0200 Subject: [PATCH 04/14] Add test cases for checking if the class_name is not set for empty class attributes --- src/virtual_dom/vtag.rs | 72 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/src/virtual_dom/vtag.rs b/src/virtual_dom/vtag.rs index 25a04dfac4e..99cb58075cd 100644 --- a/src/virtual_dom/vtag.rs +++ b/src/virtual_dom/vtag.rs @@ -1024,6 +1024,78 @@ mod tests { html! { }; } + #[test] + fn test_diff_classes_no_ancestor() { + // test with missing class property + let mut elem = html! {
}; + let expected = None; + let vtag = assert_vtag(&mut elem); + assert_eq!(vtag.diff_classes(&None), expected); + + // test with class + let mut elem = html! {
}; + let expected = Some(String::from("ferris the crab")); + let vtag = assert_vtag(&mut elem); + assert_eq!(vtag.diff_classes(&None), expected); + + // test with empty string + let mut elem = html! {
}; + // Some(String::from("")) is not expected, even though class="" + // this is done to prevent unnecessary updates to the DOM + // furthermore a missing class property or an empty string both result in an empty Classes set + let expected = None; + let vtag = assert_vtag(&mut elem); + assert_eq!(vtag.diff_classes(&None), expected); + } + + #[test] + fn it_does_not_set_empty_class_name() { + let parent = document().create_element("div").unwrap(); + + #[cfg(feature = "std_web")] + document().body().unwrap().append_child(&parent); + #[cfg(feature = "web_sys")] + document().body().unwrap().append_child(&parent).unwrap(); + + let mut elem = html! {
}; + elem.apply(&parent, None, None); + let vtag = assert_vtag(&mut elem); + // test if the className has not been set + assert!(!vtag.reference.as_ref().unwrap().has_attribute("class")); + } + + #[test] + fn it_does_not_set_missing_class_name() { + let parent = document().create_element("div").unwrap(); + + #[cfg(feature = "std_web")] + document().body().unwrap().append_child(&parent); + #[cfg(feature = "web_sys")] + document().body().unwrap().append_child(&parent).unwrap(); + + let mut elem = html! {
}; + elem.apply(&parent, None, None); + let vtag = assert_vtag(&mut elem); + // test if the className has not been set + assert!(!vtag.reference.as_ref().unwrap().has_attribute("class")); + } + + #[test] + fn it_sets_class_name() { + let parent = document().create_element("div").unwrap(); + + #[cfg(feature = "std_web")] + document().body().unwrap().append_child(&parent); + #[cfg(feature = "web_sys")] + document().body().unwrap().append_child(&parent).unwrap(); + + let mut elem = html! {
}; + elem.apply(&parent, None, None); + let vtag = assert_vtag(&mut elem); + // test if the className has been set + assert!(vtag.reference.as_ref().unwrap().has_attribute("class")); + } + #[test] fn swap_order_of_classes() { let parent = document().create_element("div").unwrap(); From 3524ab1b679040f94189f2965cc75425decd3ba4 Mon Sep 17 00:00:00 2001 From: liquidblock Date: Fri, 17 Apr 2020 22:00:04 +0200 Subject: [PATCH 05/14] Add class to SVG g element in the supports_svg test --- src/virtual_dom/vtag.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/virtual_dom/vtag.rs b/src/virtual_dom/vtag.rs index 99cb58075cd..08aaf626ef6 100644 --- a/src/virtual_dom/vtag.rs +++ b/src/virtual_dom/vtag.rs @@ -889,7 +889,7 @@ mod tests { let namespace = Some(namespace); let svg_el = document.create_element_ns(namespace, "svg").unwrap(); - let mut g_node = html! { }; + let mut g_node = html! { }; let path_node = html! { }; let mut svg_node = html! { {path_node} }; From da92aab21de49624e0fc4abdd51d68094fbdbcab Mon Sep 17 00:00:00 2001 From: liquidblock Date: Sat, 18 Apr 2020 01:35:44 +0200 Subject: [PATCH 06/14] Revert "Use the className property for setting classes in DOM" This reverts commit 41bfbc3218ce17310d7b9fa6ac99cb0049a34911. --- src/virtual_dom/vtag.rs | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/virtual_dom/vtag.rs b/src/virtual_dom/vtag.rs index 08aaf626ef6..3d3fc0a60e1 100644 --- a/src/virtual_dom/vtag.rs +++ b/src/virtual_dom/vtag.rs @@ -131,14 +131,14 @@ impl VTag { } /// Adds a single class to this virtual node. Actually it will set by - /// [Element.className](https://developer.mozilla.org/en-US/docs/Web/API/Element/className) + /// [Element.setAttribute](https://developer.mozilla.org/en-US/docs/Web/API/Element/setAttribute) /// call later. pub fn add_class(&mut self, class: &str) { self.classes.push(class); } /// Adds multiple classes to this virtual node. Actually it will set by - /// [Element.className](https://developer.mozilla.org/en-US/docs/Web/API/Element/className) + /// [Element.setAttribute](https://developer.mozilla.org/en-US/docs/Web/API/Element/setAttribute) /// call later. pub fn add_classes(&mut self, classes: Vec<&str>) { for class in classes { @@ -147,7 +147,7 @@ impl VTag { } /// Add classes to this virtual node. Actually it will set by - /// [Element.className](https://developer.mozilla.org/en-US/docs/Web/API/Element/className) + /// [Element.setAttribute](https://developer.mozilla.org/en-US/docs/Web/API/Element/setAttribute) /// call later. pub fn set_classes(&mut self, classes: impl Into) { self.classes = classes.into(); @@ -297,7 +297,9 @@ impl VTag { // Update parameters let class_str = self.diff_classes(ancestor); if let Some(class_str) = class_str { - set_class_name(element, &class_str); + element + .set_attribute("class", &class_str) + .expect("could not set class"); } let changes = self.diff_attributes(ancestor); @@ -519,14 +521,6 @@ fn set_checked(input: &InputElement, value: bool) { }; } -/// Set `className` value for the `Element`. -fn set_class_name(input: &Element, value: &str) { - cfg_match! { - feature = "std_web" => js!( @(no_return) @{input}.className = @{value}; ), - feature = "web_sys" => input.set_class_name(value), - }; -} - impl PartialEq for VTag { fn eq(&self, other: &VTag) -> bool { self.tag == other.tag From 9bf93fb49bc20768cb531fab88e3645b1ba3b6c2 Mon Sep 17 00:00:00 2001 From: liquidblock Date: Sun, 19 Apr 2020 20:06:06 +0200 Subject: [PATCH 07/14] Use patches for the class attribute and remove classes with remove_attribute --- src/virtual_dom/mod.rs | 27 ++++++++++ src/virtual_dom/vtag.rs | 116 +++++++++++++++++++++++++--------------- 2 files changed, 101 insertions(+), 42 deletions(-) diff --git a/src/virtual_dom/mod.rs b/src/virtual_dom/mod.rs index b20042d2fe6..b6e9ba99814 100644 --- a/src/virtual_dom/mod.rs +++ b/src/virtual_dom/mod.rs @@ -162,12 +162,39 @@ impl PartialEq for Classes { } /// Patch for DOM node modification. +#[derive(Debug, PartialEq)] enum Patch { Add(ID, T), Replace(ID, T), Remove(ID), } +impl Patch { + pub fn as_ref(&self) -> Patch<&ID, &T> { + match self { + Patch::Add(ref id, ref x) => Patch::Add(id, x), + Patch::Replace(ref id, ref x) => Patch::Replace(id, x), + Patch::Remove(ref id) => Patch::Remove(id), + } + } + + pub fn map U>(self, f: F) -> Patch { + match self { + Patch::Add(id, x) => Patch::Add(id, f(x)), + Patch::Replace(id, x) => Patch::Replace(id, f(x)), + Patch::Remove(id) => Patch::Remove(id), + } + } + + pub fn map_key K>(self, f: F) -> Patch { + match self { + Patch::Add(id, x) => Patch::Add(f(id), x), + Patch::Replace(id, x) => Patch::Replace(f(id), x), + Patch::Remove(id) => Patch::Remove(f(id)), + } + } +} + /// Reform of a node. enum Reform { /// Don't create a NEW reference (js Node). diff --git a/src/virtual_dom/vtag.rs b/src/virtual_dom/vtag.rs index 3d3fc0a60e1..34ebe1cf7c1 100644 --- a/src/virtual_dom/vtag.rs +++ b/src/virtual_dom/vtag.rs @@ -149,6 +149,9 @@ impl VTag { /// Add classes to this virtual node. Actually it will set by /// [Element.setAttribute](https://developer.mozilla.org/en-US/docs/Web/API/Element/setAttribute) /// call later. + /// Or if the classes are empty it will be removed by + /// [Element.removeAttribute](https://developer.mozilla.org/en-US/docs/Web/API/Element/removeAttribute) + /// call later. pub fn set_classes(&mut self, classes: impl Into) { self.classes = classes.into(); } @@ -205,20 +208,39 @@ impl VTag { } } - /// Returns the classes of self separated by spaces if and only if - /// - the classes, or the order, differs from the ancestor - /// - or there is no ancestor and the classes are not empty + /// Returns the classes of this virtual node if they are not empty, or `None` otherwise. + fn non_empty_classes(&self) -> Option<&Classes> { + Some(&self.classes).filter(|c| !c.is_empty()) + } + + /// Compute differences between the ancestor and determine patch changes for classes. /// - /// Otherwise None is returned. - fn diff_classes<'a>(&'a self, ancestor: &'a Option>) -> Option { - if ancestor - .as_ref() - .map(|ancestor| self.classes.ne(&ancestor.classes)) - .unwrap_or_else(|| !self.classes.is_empty()) - { - Some(self.classes.to_string()) - } else { - None + /// If the classes of this virtual node are empty, the following patches can occur: + /// - `Patch::Remove` if the ancestor is present and in containing at least one class. + /// - `None` otherwise. + /// + /// If the classes of this virtual node contain at least one class, the following patches can occur: + /// - `Patch::Add` if there is no ancestor or it contains no classes. + /// - `Patch::Replace` if there is an ancestor and it classes are not equal to this virtual nodes classes. + /// - `None` if there is an ancestor and it classes are equal to this virtual nodes classes. + /// + fn diff_classes<'a>(&'a self, ancestor: &'a Option>) -> Option> { + match ( + self.non_empty_classes(), + ancestor + .as_ref() + .and_then(|ancestor| ancestor.non_empty_classes()), + ) { + (Some(ref left), Some(ref right)) => { + if left != right { + Some(Patch::Replace((), left.to_string())) + } else { + None + } + } + (Some(left), None) => Some(Patch::Add((), left.to_string())), + (None, Some(ref _right)) => Some(Patch::Remove(())), + (None, None) => None, } } @@ -295,14 +317,15 @@ impl VTag { let element = self.reference.as_ref().expect("element expected"); // Update parameters - let class_str = self.diff_classes(ancestor); - if let Some(class_str) = class_str { - element - .set_attribute("class", &class_str) - .expect("could not set class"); - } + let owned_class_patch = self.diff_classes(ancestor); + let class_attribute: Option> = owned_class_patch + .as_ref() + .map(|patch| patch.as_ref().map_key(|_| "class").map(String::as_str)); + + let attributes = self.diff_attributes(ancestor); - let changes = self.diff_attributes(ancestor); + // apply attribute patches including an optional "class"-attribute patch + let changes = attributes.chain(class_attribute); for change in changes { match change { Patch::Add(key, value) | Patch::Replace(key, value) => { @@ -313,7 +336,7 @@ impl VTag { Patch::Remove(key) => { cfg_match! { feature = "std_web" => element.remove_attribute(&key), - feature = "web_sys" => element.remove_attribute(&key).expect("could not remove class"), + feature = "web_sys" => element.remove_attribute(&key).expect("could not remove attribute"), }; } } @@ -1018,28 +1041,37 @@ mod tests { html! {
}; } - #[test] - fn test_diff_classes_no_ancestor() { - // test with missing class property - let mut elem = html! {
}; - let expected = None; - let vtag = assert_vtag(&mut elem); - assert_eq!(vtag.diff_classes(&None), expected); - - // test with class - let mut elem = html! {
}; - let expected = Some(String::from("ferris the crab")); - let vtag = assert_vtag(&mut elem); - assert_eq!(vtag.diff_classes(&None), expected); + fn vtag_with_class(class: &str) -> Box { + let node = html! {
}; + if let VNode::VTag(vtag) = node { + return vtag; + } + panic!("should be vtag"); + } - // test with empty string - let mut elem = html! {
}; - // Some(String::from("")) is not expected, even though class="" - // this is done to prevent unnecessary updates to the DOM - // furthermore a missing class property or an empty string both result in an empty Classes set - let expected = None; - let vtag = assert_vtag(&mut elem); - assert_eq!(vtag.diff_classes(&None), expected); + #[test] + fn test_diff_classes() { + assert_eq!(vtag_with_class("").diff_classes(&None), None,); + assert_eq!( + vtag_with_class("ferris the crab").diff_classes(&None), + Some(Patch::Add((), String::from("ferris the crab"))), + ); + assert_eq!( + vtag_with_class("").diff_classes(&Some(vtag_with_class(""))), + None, + ); + assert_eq!( + vtag_with_class("ferris the crab").diff_classes(&Some(vtag_with_class(""))), + Some(Patch::Add((), String::from("ferris the crab"))), + ); + assert_eq!( + vtag_with_class("ferris the crab").diff_classes(&Some(vtag_with_class("hello world"))), + Some(Patch::Replace((), String::from("ferris the crab"))), + ); + assert_eq!( + vtag_with_class("").diff_classes(&Some(vtag_with_class("hello world"))), + Some(Patch::Remove(())), + ); } #[test] From cb6b3bbf174cd999bec4ecada2e34d756d412ced Mon Sep 17 00:00:00 2001 From: liquidblock Date: Sun, 26 Apr 2020 15:28:11 +0200 Subject: [PATCH 08/14] Set classes directly as class attribute --- yew-macro/src/html_tree/html_tag/mod.rs | 2 +- yew/src/virtual_dom/mod.rs | 26 ---- yew/src/virtual_dom/vtag.rs | 159 ++++++------------------ 3 files changed, 40 insertions(+), 147 deletions(-) diff --git a/yew-macro/src/html_tree/html_tag/mod.rs b/yew-macro/src/html_tree/html_tag/mod.rs index 1f45fc83f9e..4ee265cfe84 100644 --- a/yew-macro/src/html_tree/html_tag/mod.rs +++ b/yew-macro/src/html_tree/html_tag/mod.rs @@ -132,7 +132,7 @@ impl ToTokens for HtmlTag { }); let set_classes = classes.iter().map(|classes_form| match classes_form { ClassesForm::Tuple(classes) => quote! { - #vtag.add_classes(vec![#(&(#classes)),*]); + #vtag.set_classes(<::yew::virtual_dom::Classes as ::std::convert::From::>>::from(vec![#(&(#classes)),*])); }, ClassesForm::Single(classes) => quote! { #vtag.set_classes(#classes); diff --git a/yew/src/virtual_dom/mod.rs b/yew/src/virtual_dom/mod.rs index b6e9ba99814..b6ae3563149 100644 --- a/yew/src/virtual_dom/mod.rs +++ b/yew/src/virtual_dom/mod.rs @@ -169,32 +169,6 @@ enum Patch { Remove(ID), } -impl Patch { - pub fn as_ref(&self) -> Patch<&ID, &T> { - match self { - Patch::Add(ref id, ref x) => Patch::Add(id, x), - Patch::Replace(ref id, ref x) => Patch::Replace(id, x), - Patch::Remove(ref id) => Patch::Remove(id), - } - } - - pub fn map U>(self, f: F) -> Patch { - match self { - Patch::Add(id, x) => Patch::Add(id, f(x)), - Patch::Replace(id, x) => Patch::Replace(id, f(x)), - Patch::Remove(id) => Patch::Remove(id), - } - } - - pub fn map_key K>(self, f: F) -> Patch { - match self { - Patch::Add(id, x) => Patch::Add(f(id), x), - Patch::Replace(id, x) => Patch::Replace(f(id), x), - Patch::Remove(id) => Patch::Remove(f(id)), - } - } -} - /// Reform of a node. enum Reform { /// Don't create a NEW reference (js Node). diff --git a/yew/src/virtual_dom/vtag.rs b/yew/src/virtual_dom/vtag.rs index 34ebe1cf7c1..0c35ed18f10 100644 --- a/yew/src/virtual_dom/vtag.rs +++ b/yew/src/virtual_dom/vtag.rs @@ -50,8 +50,6 @@ pub struct VTag { pub attributes: Attributes, /// List of children nodes pub children: VList, - /// List of attached classes. - pub classes: Classes, /// Contains a value of an /// [InputElement](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input). pub value: Option, @@ -81,7 +79,6 @@ impl Clone for VTag { listeners: self.listeners.clone(), attributes: self.attributes.clone(), children: self.children.clone(), - classes: self.classes.clone(), value: self.value.clone(), kind: self.kind.clone(), checked: self.checked, @@ -98,7 +95,6 @@ impl VTag { VTag { tag: tag.into(), reference: None, - classes: Classes::new(), attributes: Attributes::new(), listeners: Vec::new(), captured: Vec::new(), @@ -130,19 +126,12 @@ impl VTag { } } - /// Adds a single class to this virtual node. Actually it will set by - /// [Element.setAttribute](https://developer.mozilla.org/en-US/docs/Web/API/Element/setAttribute) - /// call later. - pub fn add_class(&mut self, class: &str) { - self.classes.push(class); - } - - /// Adds multiple classes to this virtual node. Actually it will set by - /// [Element.setAttribute](https://developer.mozilla.org/en-US/docs/Web/API/Element/setAttribute) - /// call later. - pub fn add_classes(&mut self, classes: Vec<&str>) { - for class in classes { - self.classes.push(class); + /// List of attached classes. + pub fn classes(&self) -> &str { + if let Some(class_str) = self.attributes.get("class") { + class_str.as_ref() + } else { + "" } } @@ -153,7 +142,13 @@ impl VTag { /// [Element.removeAttribute](https://developer.mozilla.org/en-US/docs/Web/API/Element/removeAttribute) /// call later. pub fn set_classes(&mut self, classes: impl Into) { - self.classes = classes.into(); + let classes: Classes = classes.into(); + if classes.is_empty() { + self.attributes.remove("class"); + } else { + self.attributes + .insert("class".to_string(), classes.to_string()); + } } /// Sets `value` for an @@ -208,42 +203,6 @@ impl VTag { } } - /// Returns the classes of this virtual node if they are not empty, or `None` otherwise. - fn non_empty_classes(&self) -> Option<&Classes> { - Some(&self.classes).filter(|c| !c.is_empty()) - } - - /// Compute differences between the ancestor and determine patch changes for classes. - /// - /// If the classes of this virtual node are empty, the following patches can occur: - /// - `Patch::Remove` if the ancestor is present and in containing at least one class. - /// - `None` otherwise. - /// - /// If the classes of this virtual node contain at least one class, the following patches can occur: - /// - `Patch::Add` if there is no ancestor or it contains no classes. - /// - `Patch::Replace` if there is an ancestor and it classes are not equal to this virtual nodes classes. - /// - `None` if there is an ancestor and it classes are equal to this virtual nodes classes. - /// - fn diff_classes<'a>(&'a self, ancestor: &'a Option>) -> Option> { - match ( - self.non_empty_classes(), - ancestor - .as_ref() - .and_then(|ancestor| ancestor.non_empty_classes()), - ) { - (Some(ref left), Some(ref right)) => { - if left != right { - Some(Patch::Replace((), left.to_string())) - } else { - None - } - } - (Some(left), None) => Some(Patch::Add((), left.to_string())), - (None, Some(ref _right)) => Some(Patch::Remove(())), - (None, None) => None, - } - } - /// Similar to diff_classes except for attributes. /// /// This also handles patching of attributes when the keys are equal but @@ -317,15 +276,9 @@ impl VTag { let element = self.reference.as_ref().expect("element expected"); // Update parameters - let owned_class_patch = self.diff_classes(ancestor); - let class_attribute: Option> = owned_class_patch - .as_ref() - .map(|patch| patch.as_ref().map_key(|_| "class").map(String::as_str)); - - let attributes = self.diff_attributes(ancestor); + let changes = self.diff_attributes(ancestor); // apply attribute patches including an optional "class"-attribute patch - let changes = attributes.chain(class_attribute); for change in changes { match change { Patch::Add(key, value) | Patch::Replace(key, value) => { @@ -557,7 +510,6 @@ impl PartialEq for VTag { .map(|l| l.kind()) .eq(other.listeners.iter().map(|l| l.kind())) && self.attributes == other.attributes - && self.classes.eq(&other.classes) && self.children == other.children } } @@ -786,10 +738,10 @@ mod tests { }; if let VNode::VTag(vtag) = a { - println!("{:?}", vtag.classes); - assert!(vtag.classes.contains("class-1")); - assert!(vtag.classes.contains("class-2")); - assert!(!vtag.classes.contains("class-3")); + println!("{:?}", vtag.classes()); + assert!(vtag.classes().contains("class-1")); + assert!(vtag.classes().contains("class-2")); + assert!(!vtag.classes().contains("class-3")); } else { panic!("vtag expected"); } @@ -808,10 +760,10 @@ mod tests { assert_ne!(a, b); if let VNode::VTag(vtag) = a { - println!("{:?}", vtag.classes); - assert!(vtag.classes.contains("class-1")); - assert!(vtag.classes.contains("class-2")); - assert!(vtag.classes.contains("class-3")); + println!("{:?}", vtag.classes()); + assert!(vtag.classes().contains("class-1")); + assert!(vtag.classes().contains("class-2")); + assert!(vtag.classes().contains("class-3")); } else { panic!("vtag expected"); } @@ -826,10 +778,10 @@ mod tests { }; if let VNode::VTag(vtag) = a { - println!("{:?}", vtag.classes); - assert!(vtag.classes.contains("class-1")); - assert!(vtag.classes.contains("class-2")); - assert!(!vtag.classes.contains("class-3")); + println!("{:?}", vtag.classes()); + assert!(vtag.classes().contains("class-1")); + assert!(vtag.classes().contains("class-2")); + assert!(!vtag.classes().contains("class-3")); } else { panic!("vtag expected"); } @@ -843,10 +795,10 @@ mod tests { }; if let VNode::VTag(vtag) = a { - println!("{:?}", vtag.classes); - assert!(vtag.classes.contains("class-1")); - assert!(vtag.classes.contains("class-2")); - assert!(!vtag.classes.contains("class-3")); + println!("{:?}", vtag.classes()); + assert!(vtag.classes().contains("class-1")); + assert!(vtag.classes().contains("class-2")); + assert!(!vtag.classes().contains("class-3")); } else { panic!("vtag expected"); } @@ -861,19 +813,19 @@ mod tests { let c = html! {
}; if let VNode::VTag(vtag) = a { - assert!(vtag.classes.is_empty()); + assert!(vtag.classes().is_empty()); } else { panic!("vtag expected"); } if let VNode::VTag(vtag) = b { - assert!(vtag.classes.is_empty()); + assert!(vtag.classes().is_empty()); } else { panic!("vtag expected"); } if let VNode::VTag(vtag) = c { - assert!(vtag.classes.is_empty()); + assert!(vtag.classes().is_empty()); } else { panic!("vtag expected"); } @@ -932,8 +884,8 @@ mod tests { }; if let VNode::VTag(vtag) = a { - println!("{:?}", vtag.classes); - assert_eq!(vtag.classes.to_string(), "class-1 class-2 class-3"); + println!("{:?}", vtag.classes()); + assert_eq!(vtag.classes(), "class-1 class-2 class-3"); } } @@ -1041,39 +993,6 @@ mod tests { html! {
}; } - fn vtag_with_class(class: &str) -> Box { - let node = html! {
}; - if let VNode::VTag(vtag) = node { - return vtag; - } - panic!("should be vtag"); - } - - #[test] - fn test_diff_classes() { - assert_eq!(vtag_with_class("").diff_classes(&None), None,); - assert_eq!( - vtag_with_class("ferris the crab").diff_classes(&None), - Some(Patch::Add((), String::from("ferris the crab"))), - ); - assert_eq!( - vtag_with_class("").diff_classes(&Some(vtag_with_class(""))), - None, - ); - assert_eq!( - vtag_with_class("ferris the crab").diff_classes(&Some(vtag_with_class(""))), - Some(Patch::Add((), String::from("ferris the crab"))), - ); - assert_eq!( - vtag_with_class("ferris the crab").diff_classes(&Some(vtag_with_class("hello world"))), - Some(Patch::Replace((), String::from("ferris the crab"))), - ); - assert_eq!( - vtag_with_class("").diff_classes(&Some(vtag_with_class("hello world"))), - Some(Patch::Remove(())), - ); - } - #[test] fn it_does_not_set_empty_class_name() { let parent = document().create_element("div").unwrap(); @@ -1141,7 +1060,7 @@ mod tests { }; let expected = "class-1 class-2 class-3"; - assert_eq!(vtag.classes.to_string(), expected); + assert_eq!(vtag.classes(), expected); assert_eq!( vtag.reference .as_ref() @@ -1161,7 +1080,7 @@ mod tests { vtag.apply(&parent, None, Some(VNode::VTag(ancestor))); let expected = "class-3 class-2 class-1"; - assert_eq!(vtag.classes.to_string(), expected); + assert_eq!(vtag.classes(), expected); assert_eq!( vtag.reference .as_ref() @@ -1191,7 +1110,7 @@ mod tests { }; let expected = "class-1 class-3"; - assert_eq!(vtag.classes.to_string(), expected); + assert_eq!(vtag.classes(), expected); assert_eq!( vtag.reference .as_ref() @@ -1211,7 +1130,7 @@ mod tests { vtag.apply(&parent, None, Some(VNode::VTag(ancestor))); let expected = "class-1 class-2 class-3"; - assert_eq!(vtag.classes.to_string(), expected); + assert_eq!(vtag.classes(), expected); assert_eq!( vtag.reference .as_ref() From ef5617f315705cbc73c1295037c2d3138dcedfe5 Mon Sep 17 00:00:00 2001 From: liquidblock Date: Sun, 26 Apr 2020 15:28:27 +0200 Subject: [PATCH 09/14] Fix markdown example --- examples/crm/src/markdown.rs | 38 +++++++++++++++-------- yew-router/examples/guide/src/markdown.rs | 36 ++++++++++++++------- 2 files changed, 49 insertions(+), 25 deletions(-) diff --git a/examples/crm/src/markdown.rs b/examples/crm/src/markdown.rs index 2a239806f4b..0fc398cb6bc 100644 --- a/examples/crm/src/markdown.rs +++ b/examples/crm/src/markdown.rs @@ -1,9 +1,21 @@ /// Original author of this code is [Nathan Ringo](https://github.com/remexre) /// Source: https://github.com/acmumn/mentoring/blob/master/web-client/src/view/markdown.rs use pulldown_cmark::{Alignment, CodeBlockKind, Event, Options, Parser, Tag}; -use yew::virtual_dom::{VNode, VTag, VText}; +use yew::virtual_dom::{Classes, VNode, VTag, VText}; use yew::{html, Html}; +/// Adds a class to the VTag. +/// You can also provide multiple classes separated by ascii whitespaces. +/// +/// Note that this has a complexity of O(n), +/// where n is the number of classes already in VTag plus +/// the number of classes to be added. +fn add_class(vtag: &mut VTag, class: &str) { + let mut classes: Classes = vtag.classes().into(); + classes.push(class); + vtag.set_classes(classes); +} + /// Renders a string of Markdown to HTML with the default options (footnotes /// disabled, tables enabled). pub fn render_markdown(src: &str) -> Html { @@ -42,9 +54,9 @@ pub fn render_markdown(src: &str) -> Html { if let VNode::VTag(ref mut vtag) = c { match aligns[i] { Alignment::None => {} - Alignment::Left => vtag.add_class("text-left"), - Alignment::Center => vtag.add_class("text-center"), - Alignment::Right => vtag.add_class("text-right"), + Alignment::Left => add_class(vtag, "text-left"), + Alignment::Center => add_class(vtag, "text-center"), + Alignment::Right => add_class(vtag, "text-right"), } } } @@ -92,7 +104,7 @@ fn make_tag(t: Tag) -> VTag { } Tag::BlockQuote => { let mut el = VTag::new("blockquote"); - el.add_class("blockquote"); + el.set_classes("blockquote"); el } Tag::CodeBlock(code_block_kind) => { @@ -104,10 +116,10 @@ fn make_tag(t: Tag) -> VTag { // highlighting support by locating the language classes and applying dom transforms // on their contents. match lang.as_ref() { - "html" => el.add_class("html-language"), - "rust" => el.add_class("rust-language"), - "java" => el.add_class("java-language"), - "c" => el.add_class("c-language"), + "html" => el.set_classes("html-language"), + "rust" => el.set_classes("rust-language"), + "java" => el.set_classes("java-language"), + "c" => el.set_classes("c-language"), _ => {} // Add your own language highlighting support }; } @@ -124,7 +136,7 @@ fn make_tag(t: Tag) -> VTag { Tag::Item => VTag::new("li"), Tag::Table(_) => { let mut el = VTag::new("table"); - el.add_class("table"); + el.set_classes("table"); el } Tag::TableHead => VTag::new("th"), @@ -132,12 +144,12 @@ fn make_tag(t: Tag) -> VTag { Tag::TableCell => VTag::new("td"), Tag::Emphasis => { let mut el = VTag::new("span"); - el.add_class("font-italic"); + el.set_classes("font-italic"); el } Tag::Strong => { let mut el = VTag::new("span"); - el.add_class("font-weight-bold"); + el.set_classes("font-weight-bold"); el } Tag::Link(_link_type, ref href, ref title) => { @@ -161,7 +173,7 @@ fn make_tag(t: Tag) -> VTag { Tag::FootnoteDefinition(ref _footnote_id) => VTag::new("span"), // Footnotes are not rendered as anything special Tag::Strikethrough => { let mut el = VTag::new("span"); - el.add_class("text-decoration-strikethrough"); + el.set_classes("text-decoration-strikethrough"); el } } diff --git a/yew-router/examples/guide/src/markdown.rs b/yew-router/examples/guide/src/markdown.rs index b65c0fa6a00..de225ed0926 100644 --- a/yew-router/examples/guide/src/markdown.rs +++ b/yew-router/examples/guide/src/markdown.rs @@ -3,10 +3,22 @@ use pulldown_cmark::{Alignment, Event, Options, Parser, Tag}; use yew::{ html, - virtual_dom::{VNode, VTag, VText}, + virtual_dom::{Classes, VNode, VTag, VText}, Html, }; +/// Adds a class to the VTag. +/// You can also provide multiple classes separated by ascii whitespaces. +/// +/// Note that this has a complexity of O(n), +/// where n is the number of classes already in VTag plus +/// the number of classes to be added. +fn add_class(vtag: &mut VTag, class: &str) { + let mut classes: Classes = vtag.classes().into(); + classes.push(class); + vtag.set_classes(classes); +} + /// Renders a string of Markdown to HTML with the default options (footnotes /// disabled, tables enabled). pub fn render_markdown(src: &str) -> Html { @@ -44,9 +56,9 @@ pub fn render_markdown(src: &str) -> Html { if let VNode::VTag(ref mut vtag) = *c { match aligns[i] { Alignment::None => {} - Alignment::Left => vtag.add_class("text-left"), - Alignment::Center => vtag.add_class("text-center"), - Alignment::Right => vtag.add_class("text-right"), + Alignment::Left => add_class(vtag, "text-left"), + Alignment::Center => add_class(vtag, "text-center"), + Alignment::Right => add_class(vtag, "text-right"), } } } @@ -93,7 +105,7 @@ fn make_tag(t: Tag) -> VTag { Tag::Paragraph => VTag::new("p"), Tag::BlockQuote => { let mut el = VTag::new("blockquote"); - el.add_class("blockquote"); + el.set_classes("blockquote"); el } Tag::CodeBlock(lang) => { @@ -103,10 +115,10 @@ fn make_tag(t: Tag) -> VTag { // actually provide the highlighting support by locating the language // classes and applying dom transforms on their contents. match lang.as_ref() { - "html" => el.add_class("html-language"), - "rust" => el.add_class("rust-language"), - "java" => el.add_class("java-language"), - "c" => el.add_class("c-language"), + "html" => el.set_classes("html-language"), + "rust" => el.set_classes("rust-language"), + "java" => el.set_classes("java-language"), + "c" => el.set_classes("c-language"), _ => {} // Add your own language highlighting support }; el @@ -121,7 +133,7 @@ fn make_tag(t: Tag) -> VTag { Tag::Item => VTag::new("li"), Tag::Table(_) => { let mut el = VTag::new("table"); - el.add_class("table"); + el.set_classes("table"); el } Tag::TableHead => VTag::new("tr"), @@ -129,12 +141,12 @@ fn make_tag(t: Tag) -> VTag { Tag::TableCell => VTag::new("td"), Tag::Emphasis => { let mut el = VTag::new("span"); - el.add_class("font-italic"); + el.set_classes("font-italic"); el } Tag::Strong => { let mut el = VTag::new("span"); - el.add_class("font-weight-bold"); + el.set_classes("font-weight-bold"); el } Tag::Link(_lt, ref href, ref title) => { From c96ce88e99d3c61a8fc888c1f11f33fb2a801e8e Mon Sep 17 00:00:00 2001 From: liquidblock Date: Mon, 27 Apr 2020 02:17:27 +0200 Subject: [PATCH 10/14] Remove set_classes and classes from VTag Add possibility to use different types in the class tuple Add convertion to Classes from Option where T: AsRef --- examples/crm/src/markdown.rs | 27 ++-- yew-macro/src/html_tree/html_tag/mod.rs | 10 +- yew-router/examples/guide/src/markdown.rs | 25 +-- yew/src/virtual_dom/mod.rs | 30 ++-- yew/src/virtual_dom/vtag.rs | 177 +++++++++++++++------- 5 files changed, 176 insertions(+), 93 deletions(-) diff --git a/examples/crm/src/markdown.rs b/examples/crm/src/markdown.rs index 0fc398cb6bc..b809a4a4d9c 100644 --- a/examples/crm/src/markdown.rs +++ b/examples/crm/src/markdown.rs @@ -11,9 +11,14 @@ use yew::{html, Html}; /// where n is the number of classes already in VTag plus /// the number of classes to be added. fn add_class(vtag: &mut VTag, class: &str) { - let mut classes: Classes = vtag.classes().into(); + let mut classes: Classes = vtag + .attributes + .get("class") + .map(AsRef::as_ref) + .unwrap_or("") + .into(); classes.push(class); - vtag.set_classes(classes); + vtag.add_attribute("class", &classes); } /// Renders a string of Markdown to HTML with the default options (footnotes @@ -104,7 +109,7 @@ fn make_tag(t: Tag) -> VTag { } Tag::BlockQuote => { let mut el = VTag::new("blockquote"); - el.set_classes("blockquote"); + el.add_attribute("class", &"blockquote"); el } Tag::CodeBlock(code_block_kind) => { @@ -116,10 +121,10 @@ fn make_tag(t: Tag) -> VTag { // highlighting support by locating the language classes and applying dom transforms // on their contents. match lang.as_ref() { - "html" => el.set_classes("html-language"), - "rust" => el.set_classes("rust-language"), - "java" => el.set_classes("java-language"), - "c" => el.set_classes("c-language"), + "html" => el.add_attribute("class", &"html-language"), + "rust" => el.add_attribute("class", &"rust-language"), + "java" => el.add_attribute("class", &"java-language"), + "c" => el.add_attribute("class", &"c-language"), _ => {} // Add your own language highlighting support }; } @@ -136,7 +141,7 @@ fn make_tag(t: Tag) -> VTag { Tag::Item => VTag::new("li"), Tag::Table(_) => { let mut el = VTag::new("table"); - el.set_classes("table"); + el.add_attribute("class", &"table"); el } Tag::TableHead => VTag::new("th"), @@ -144,12 +149,12 @@ fn make_tag(t: Tag) -> VTag { Tag::TableCell => VTag::new("td"), Tag::Emphasis => { let mut el = VTag::new("span"); - el.set_classes("font-italic"); + el.add_attribute("class", &"font-italic"); el } Tag::Strong => { let mut el = VTag::new("span"); - el.set_classes("font-weight-bold"); + el.add_attribute("class", &"font-weight-bold"); el } Tag::Link(_link_type, ref href, ref title) => { @@ -173,7 +178,7 @@ fn make_tag(t: Tag) -> VTag { Tag::FootnoteDefinition(ref _footnote_id) => VTag::new("span"), // Footnotes are not rendered as anything special Tag::Strikethrough => { let mut el = VTag::new("span"); - el.set_classes("text-decoration-strikethrough"); + el.add_attribute("class", &"text-decoration-strikethrough"); el } } diff --git a/yew-macro/src/html_tree/html_tag/mod.rs b/yew-macro/src/html_tree/html_tag/mod.rs index 4ee265cfe84..2f0a50b6267 100644 --- a/yew-macro/src/html_tree/html_tag/mod.rs +++ b/yew-macro/src/html_tree/html_tag/mod.rs @@ -132,10 +132,16 @@ impl ToTokens for HtmlTag { }); let set_classes = classes.iter().map(|classes_form| match classes_form { ClassesForm::Tuple(classes) => quote! { - #vtag.set_classes(<::yew::virtual_dom::Classes as ::std::convert::From::>>::from(vec![#(&(#classes)),*])); + let mut __yew_classes = ::yew::virtual_dom::Classes::default()#(.extend(#classes))*; + if !__yew_classes.is_empty() { + #vtag.add_attribute("class", &__yew_classes); + } }, ClassesForm::Single(classes) => quote! { - #vtag.set_classes(#classes); + let mut __yew_classes = std::convert::Into::<::yew::virtual_dom::Classes>::into(#classes); + if !__yew_classes.is_empty() { + #vtag.add_attribute("class", &__yew_classes); + } }, }); let set_node_ref = node_ref.iter().map(|node_ref| { diff --git a/yew-router/examples/guide/src/markdown.rs b/yew-router/examples/guide/src/markdown.rs index de225ed0926..80fdfe12772 100644 --- a/yew-router/examples/guide/src/markdown.rs +++ b/yew-router/examples/guide/src/markdown.rs @@ -14,9 +14,14 @@ use yew::{ /// where n is the number of classes already in VTag plus /// the number of classes to be added. fn add_class(vtag: &mut VTag, class: &str) { - let mut classes: Classes = vtag.classes().into(); + let mut classes: Classes = vtag + .attributes + .get("class") + .map(AsRef::as_ref) + .unwrap_or("") + .into(); classes.push(class); - vtag.set_classes(classes); + vtag.add_attribute("class", &classes); } /// Renders a string of Markdown to HTML with the default options (footnotes @@ -105,7 +110,7 @@ fn make_tag(t: Tag) -> VTag { Tag::Paragraph => VTag::new("p"), Tag::BlockQuote => { let mut el = VTag::new("blockquote"); - el.set_classes("blockquote"); + el.add_attribute("class", &"blockquote"); el } Tag::CodeBlock(lang) => { @@ -115,10 +120,10 @@ fn make_tag(t: Tag) -> VTag { // actually provide the highlighting support by locating the language // classes and applying dom transforms on their contents. match lang.as_ref() { - "html" => el.set_classes("html-language"), - "rust" => el.set_classes("rust-language"), - "java" => el.set_classes("java-language"), - "c" => el.set_classes("c-language"), + "html" => el.add_attribute("class", &"html-language"), + "rust" => el.add_attribute("class", &"rust-language"), + "java" => el.add_attribute("class", &"java-language"), + "c" => el.add_attribute("class", &"c-language"), _ => {} // Add your own language highlighting support }; el @@ -133,7 +138,7 @@ fn make_tag(t: Tag) -> VTag { Tag::Item => VTag::new("li"), Tag::Table(_) => { let mut el = VTag::new("table"); - el.set_classes("table"); + el.add_attribute("class", &"table"); el } Tag::TableHead => VTag::new("tr"), @@ -141,12 +146,12 @@ fn make_tag(t: Tag) -> VTag { Tag::TableCell => VTag::new("td"), Tag::Emphasis => { let mut el = VTag::new("span"); - el.set_classes("font-italic"); + el.add_attribute("class", &"font-italic"); el } Tag::Strong => { let mut el = VTag::new("span"); - el.set_classes("font-weight-bold"); + el.add_attribute("class", &"font-weight-bold"); el } Tag::Link(_lt, ref href, ref title) => { diff --git a/yew/src/virtual_dom/mod.rs b/yew/src/virtual_dom/mod.rs index b6ae3563149..8a872ad35ae 100644 --- a/yew/src/virtual_dom/mod.rs +++ b/yew/src/virtual_dom/mod.rs @@ -122,23 +122,29 @@ impl From<&str> for Classes { impl From for Classes { fn from(t: String) -> Self { - let set = t - .split_whitespace() - .map(String::from) - .filter(|c| !c.is_empty()) - .collect(); - Self { set } + Classes::from(t.as_str()) } } impl From<&String> for Classes { fn from(t: &String) -> Self { - let set = t - .split_whitespace() - .map(String::from) - .filter(|c| !c.is_empty()) - .collect(); - Self { set } + Classes::from(t.as_str()) + } +} + +impl> From> for Classes { + fn from(t: Option) -> Self { + t.as_ref() + .map(|s| >::from(s.as_ref())) + .unwrap_or_default() + } +} + +impl> From<&Option> for Classes { + fn from(t: &Option) -> Self { + t.as_ref() + .map(|s| >::from(s.as_ref())) + .unwrap_or_default() } } diff --git a/yew/src/virtual_dom/vtag.rs b/yew/src/virtual_dom/vtag.rs index 0c35ed18f10..892abb55bdb 100644 --- a/yew/src/virtual_dom/vtag.rs +++ b/yew/src/virtual_dom/vtag.rs @@ -1,8 +1,6 @@ //! This module contains the implementation of a virtual element node `VTag`. -use super::{ - Attributes, Classes, Listener, Listeners, Patch, Reform, Transformer, VDiff, VList, VNode, -}; +use super::{Attributes, Listener, Listeners, Patch, Reform, Transformer, VDiff, VList, VNode}; use crate::html::NodeRef; use crate::utils::document; use cfg_if::cfg_if; @@ -126,31 +124,6 @@ impl VTag { } } - /// List of attached classes. - pub fn classes(&self) -> &str { - if let Some(class_str) = self.attributes.get("class") { - class_str.as_ref() - } else { - "" - } - } - - /// Add classes to this virtual node. Actually it will set by - /// [Element.setAttribute](https://developer.mozilla.org/en-US/docs/Web/API/Element/setAttribute) - /// call later. - /// Or if the classes are empty it will be removed by - /// [Element.removeAttribute](https://developer.mozilla.org/en-US/docs/Web/API/Element/removeAttribute) - /// call later. - pub fn set_classes(&mut self, classes: impl Into) { - let classes: Classes = classes.into(); - if classes.is_empty() { - self.attributes.remove("class"); - } else { - self.attributes - .insert("class".to_string(), classes.to_string()); - } - } - /// Sets `value` for an /// [InputElement](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input). pub fn set_value(&mut self, value: &T) { @@ -173,14 +146,14 @@ impl VTag { /// Adds attribute to a virtual node. Not every attribute works when /// it set as attribute. We use workarounds for: - /// `class`, `type/kind`, `value` and `checked`. + /// `type/kind`, `value` and `checked`. pub fn add_attribute(&mut self, name: &str, value: &T) { self.attributes.insert(name.to_owned(), value.to_string()); } /// Adds attributes to a virtual node. Not every attribute works when /// it set as attribute. We use workarounds for: - /// `class`, `type/kind`, `value` and `checked`. + /// `type/kind`, `value` and `checked`. pub fn add_attributes(&mut self, attrs: Vec<(String, String)>) { for (name, value) in attrs { self.attributes.insert(name, value); @@ -203,9 +176,7 @@ impl VTag { } } - /// Similar to diff_classes except for attributes. - /// - /// This also handles patching of attributes when the keys are equal but + /// This handles patching of attributes when the keys are equal but /// the values are different. fn diff_attributes<'a>( &'a self, @@ -738,10 +709,24 @@ mod tests { }; if let VNode::VTag(vtag) = a { - println!("{:?}", vtag.classes()); - assert!(vtag.classes().contains("class-1")); - assert!(vtag.classes().contains("class-2")); - assert!(!vtag.classes().contains("class-3")); + assert!(vtag + .attributes + .get("class") + .map(AsRef::as_ref) + .unwrap_or("") + .contains("class-1")); + assert!(vtag + .attributes + .get("class") + .map(AsRef::as_ref) + .unwrap_or("") + .contains("class-2")); + assert!(!vtag + .attributes + .get("class") + .map(AsRef::as_ref) + .unwrap_or("") + .contains("class-3")); } else { panic!("vtag expected"); } @@ -760,10 +745,24 @@ mod tests { assert_ne!(a, b); if let VNode::VTag(vtag) = a { - println!("{:?}", vtag.classes()); - assert!(vtag.classes().contains("class-1")); - assert!(vtag.classes().contains("class-2")); - assert!(vtag.classes().contains("class-3")); + assert!(vtag + .attributes + .get("class") + .map(AsRef::as_ref) + .unwrap_or("") + .contains("class-1")); + assert!(vtag + .attributes + .get("class") + .map(AsRef::as_ref) + .unwrap_or("") + .contains("class-2")); + assert!(vtag + .attributes + .get("class") + .map(AsRef::as_ref) + .unwrap_or("") + .contains("class-3")); } else { panic!("vtag expected"); } @@ -778,10 +777,24 @@ mod tests { }; if let VNode::VTag(vtag) = a { - println!("{:?}", vtag.classes()); - assert!(vtag.classes().contains("class-1")); - assert!(vtag.classes().contains("class-2")); - assert!(!vtag.classes().contains("class-3")); + assert!(vtag + .attributes + .get("class") + .map(AsRef::as_ref) + .unwrap_or("") + .contains("class-1")); + assert!(vtag + .attributes + .get("class") + .map(AsRef::as_ref) + .unwrap_or("") + .contains("class-2")); + assert!(!vtag + .attributes + .get("class") + .map(AsRef::as_ref) + .unwrap_or("") + .contains("class-3")); } else { panic!("vtag expected"); } @@ -795,10 +808,24 @@ mod tests { }; if let VNode::VTag(vtag) = a { - println!("{:?}", vtag.classes()); - assert!(vtag.classes().contains("class-1")); - assert!(vtag.classes().contains("class-2")); - assert!(!vtag.classes().contains("class-3")); + assert!(vtag + .attributes + .get("class") + .map(AsRef::as_ref) + .unwrap_or("") + .contains("class-1")); + assert!(vtag + .attributes + .get("class") + .map(AsRef::as_ref) + .unwrap_or("") + .contains("class-2")); + assert!(!vtag + .attributes + .get("class") + .map(AsRef::as_ref) + .unwrap_or("") + .contains("class-3")); } else { panic!("vtag expected"); } @@ -813,19 +840,19 @@ mod tests { let c = html! {
}; if let VNode::VTag(vtag) = a { - assert!(vtag.classes().is_empty()); + assert!(vtag.attributes.get("class").is_none()); } else { panic!("vtag expected"); } if let VNode::VTag(vtag) = b { - assert!(vtag.classes().is_empty()); + assert!(vtag.attributes.get("class").is_none()); } else { panic!("vtag expected"); } if let VNode::VTag(vtag) = c { - assert!(vtag.classes().is_empty()); + assert!(vtag.attributes.get("class").is_none()); } else { panic!("vtag expected"); } @@ -884,8 +911,10 @@ mod tests { }; if let VNode::VTag(vtag) = a { - println!("{:?}", vtag.classes()); - assert_eq!(vtag.classes(), "class-1 class-2 class-3"); + assert_eq!( + vtag.attributes.get("class").map(AsRef::as_ref), + Some("class-1 class-2 class-3") + ); } } @@ -1041,6 +1070,26 @@ mod tests { assert!(vtag.reference.as_ref().unwrap().has_attribute("class")); } + #[test] + fn tuple_different_types() { + // check if tuples containing different types are compiling + html! {
}; + html! {
}; + // check different string references + let str = "class"; + let string = str.to_string(); + let string_ref = &string; + html! {
}; + html! {
}; + html! {
}; + html! {
}; + html! {
}; + html! {
}; + html! {
}; + html! {
}; + html! {
}; + } + #[test] fn swap_order_of_classes() { let parent = document().create_element("div").unwrap(); @@ -1060,7 +1109,10 @@ mod tests { }; let expected = "class-1 class-2 class-3"; - assert_eq!(vtag.classes(), expected); + assert_eq!( + vtag.attributes.get("class").map(AsRef::as_ref), + Some(expected) + ); assert_eq!( vtag.reference .as_ref() @@ -1080,7 +1132,10 @@ mod tests { vtag.apply(&parent, None, Some(VNode::VTag(ancestor))); let expected = "class-3 class-2 class-1"; - assert_eq!(vtag.classes(), expected); + assert_eq!( + vtag.attributes.get("class").map(AsRef::as_ref), + Some(expected) + ); assert_eq!( vtag.reference .as_ref() @@ -1110,7 +1165,10 @@ mod tests { }; let expected = "class-1 class-3"; - assert_eq!(vtag.classes(), expected); + assert_eq!( + vtag.attributes.get("class").map(AsRef::as_ref), + Some(expected) + ); assert_eq!( vtag.reference .as_ref() @@ -1130,7 +1188,10 @@ mod tests { vtag.apply(&parent, None, Some(VNode::VTag(ancestor))); let expected = "class-1 class-2 class-3"; - assert_eq!(vtag.classes(), expected); + assert_eq!( + vtag.attributes.get("class").map(AsRef::as_ref), + Some(expected) + ); assert_eq!( vtag.reference .as_ref() From bfbaae74a3bbc5f438395c09e21e85cc0849cdb0 Mon Sep 17 00:00:00 2001 From: liquidblock Date: Tue, 28 Apr 2020 20:03:27 +0200 Subject: [PATCH 11/14] Remove mut from variable that does not need to be Add `::` to create an absolute path to std --- yew-macro/src/html_tree/html_tag/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/yew-macro/src/html_tree/html_tag/mod.rs b/yew-macro/src/html_tree/html_tag/mod.rs index 2f0a50b6267..834d56ce542 100644 --- a/yew-macro/src/html_tree/html_tag/mod.rs +++ b/yew-macro/src/html_tree/html_tag/mod.rs @@ -132,13 +132,13 @@ impl ToTokens for HtmlTag { }); let set_classes = classes.iter().map(|classes_form| match classes_form { ClassesForm::Tuple(classes) => quote! { - let mut __yew_classes = ::yew::virtual_dom::Classes::default()#(.extend(#classes))*; + let __yew_classes = ::yew::virtual_dom::Classes::default()#(.extend(#classes))*; if !__yew_classes.is_empty() { #vtag.add_attribute("class", &__yew_classes); } }, ClassesForm::Single(classes) => quote! { - let mut __yew_classes = std::convert::Into::<::yew::virtual_dom::Classes>::into(#classes); + let __yew_classes = ::std::convert::Into::<::yew::virtual_dom::Classes>::into(#classes); if !__yew_classes.is_empty() { #vtag.add_attribute("class", &__yew_classes); } From 34e83a50e07ab7f53ee0ac3a3e27a778b59935c5 Mon Sep 17 00:00:00 2001 From: liquidblock Date: Tue, 28 Apr 2020 20:41:26 +0200 Subject: [PATCH 12/14] Use helper methods for classes in tests --- yew/src/virtual_dom/vtag.rs | 172 ++++++++++++++---------------------- 1 file changed, 65 insertions(+), 107 deletions(-) diff --git a/yew/src/virtual_dom/vtag.rs b/yew/src/virtual_dom/vtag.rs index 892abb55bdb..a4cdabd7073 100644 --- a/yew/src/virtual_dom/vtag.rs +++ b/yew/src/virtual_dom/vtag.rs @@ -147,6 +147,8 @@ impl VTag { /// Adds attribute to a virtual node. Not every attribute works when /// it set as attribute. We use workarounds for: /// `type/kind`, `value` and `checked`. + /// + /// If this virtual node has this attribute present, the value is replaced. pub fn add_attribute(&mut self, name: &str, value: &T) { self.attributes.insert(name.to_owned(), value.to_string()); } @@ -702,6 +704,23 @@ mod tests { assert_eq!(a, c); } + /// Returns the class attribute as str reference, or "" if the attribute is not set. + fn get_class_str(vtag: &VTag) -> &str { + vtag.attributes + .get("class") + .map(AsRef::as_ref) + .unwrap_or("") + } + + /// Note: Compares to "" if the class attribute is not set. + fn assert_class(vnode: VNode, class: &str) { + if let VNode::VTag(ref vtag) = vnode { + assert_eq!(get_class_str(vtag), class); + } else { + panic!("expected VTag"); + } + } + #[test] fn supports_multiple_non_unique_classes_tuple() { let a = html! { @@ -709,24 +728,9 @@ mod tests { }; if let VNode::VTag(vtag) = a { - assert!(vtag - .attributes - .get("class") - .map(AsRef::as_ref) - .unwrap_or("") - .contains("class-1")); - assert!(vtag - .attributes - .get("class") - .map(AsRef::as_ref) - .unwrap_or("") - .contains("class-2")); - assert!(!vtag - .attributes - .get("class") - .map(AsRef::as_ref) - .unwrap_or("") - .contains("class-3")); + assert!(get_class_str(&vtag).contains("class-1")); + assert!(get_class_str(&vtag).contains("class-2")); + assert!(!get_class_str(&vtag).contains("class-3")); } else { panic!("vtag expected"); } @@ -745,24 +749,9 @@ mod tests { assert_ne!(a, b); if let VNode::VTag(vtag) = a { - assert!(vtag - .attributes - .get("class") - .map(AsRef::as_ref) - .unwrap_or("") - .contains("class-1")); - assert!(vtag - .attributes - .get("class") - .map(AsRef::as_ref) - .unwrap_or("") - .contains("class-2")); - assert!(vtag - .attributes - .get("class") - .map(AsRef::as_ref) - .unwrap_or("") - .contains("class-3")); + assert!(get_class_str(&vtag).contains("class-1")); + assert!(get_class_str(&vtag).contains("class-2")); + assert!(get_class_str(&vtag).contains("class-3")); } else { panic!("vtag expected"); } @@ -777,24 +766,9 @@ mod tests { }; if let VNode::VTag(vtag) = a { - assert!(vtag - .attributes - .get("class") - .map(AsRef::as_ref) - .unwrap_or("") - .contains("class-1")); - assert!(vtag - .attributes - .get("class") - .map(AsRef::as_ref) - .unwrap_or("") - .contains("class-2")); - assert!(!vtag - .attributes - .get("class") - .map(AsRef::as_ref) - .unwrap_or("") - .contains("class-3")); + assert!(get_class_str(&vtag).contains("class-1")); + assert!(get_class_str(&vtag).contains("class-2")); + assert!(!get_class_str(&vtag).contains("class-3")); } else { panic!("vtag expected"); } @@ -808,24 +782,9 @@ mod tests { }; if let VNode::VTag(vtag) = a { - assert!(vtag - .attributes - .get("class") - .map(AsRef::as_ref) - .unwrap_or("") - .contains("class-1")); - assert!(vtag - .attributes - .get("class") - .map(AsRef::as_ref) - .unwrap_or("") - .contains("class-2")); - assert!(!vtag - .attributes - .get("class") - .map(AsRef::as_ref) - .unwrap_or("") - .contains("class-3")); + assert!(get_class_str(&vtag).contains("class-1")); + assert!(get_class_str(&vtag).contains("class-2")); + assert!(!get_class_str(&vtag).contains("class-3")); } else { panic!("vtag expected"); } @@ -840,19 +799,19 @@ mod tests { let c = html! {
}; if let VNode::VTag(vtag) = a { - assert!(vtag.attributes.get("class").is_none()); + assert!(!vtag.attributes.contains_key("class")); } else { panic!("vtag expected"); } if let VNode::VTag(vtag) = b { - assert!(vtag.attributes.get("class").is_none()); + assert!(!vtag.attributes.contains_key("class")); } else { panic!("vtag expected"); } if let VNode::VTag(vtag) = c { - assert!(vtag.attributes.get("class").is_none()); + assert!(!vtag.attributes.contains_key("class")); } else { panic!("vtag expected"); } @@ -911,10 +870,7 @@ mod tests { }; if let VNode::VTag(vtag) = a { - assert_eq!( - vtag.attributes.get("class").map(AsRef::as_ref), - Some("class-1 class-2 class-3") - ); + assert_eq!(get_class_str(&vtag), "class-1 class-2 class-3"); } } @@ -1073,21 +1029,35 @@ mod tests { #[test] fn tuple_different_types() { // check if tuples containing different types are compiling - html! {
}; - html! {
}; + assert_class( + html! {
}, + "class-1 class-2 class-3 class-4", + ); + assert_class( + html! {
}, + "class-1 class-2 class-3 class-4", + ); // check different string references - let str = "class"; + let str = "some-class"; let string = str.to_string(); let string_ref = &string; - html! {
}; - html! {
}; - html! {
}; - html! {
}; - html! {
}; - html! {
}; - html! {
}; - html! {
}; - html! {
}; + assert_class(html! {

}, "some-class"); + assert_class(html! {

}, "some-class"); + assert_class(html! {

}, "some-class"); + assert_class(html! {

}, "some-class"); + assert_class(html! {

}, "some-class"); + assert_class(html! {

}, "some-class"); + assert_class(html! {

}, "some-class"); + assert_class(html! {

}, "some-class"); + assert_class(html! {

}, "some-class"); + // check with None + assert_class(html! {

::None /> }, ""); + assert_class(html! {

::None /> }, ""); + // check with variables + let some: Option = Some(false); + let none: Option = None; + assert_class(html! {

}, "false"); + assert_class(html! {

}, ""); } #[test] @@ -1109,10 +1079,7 @@ mod tests { }; let expected = "class-1 class-2 class-3"; - assert_eq!( - vtag.attributes.get("class").map(AsRef::as_ref), - Some(expected) - ); + assert_eq!(get_class_str(&vtag), expected); assert_eq!( vtag.reference .as_ref() @@ -1132,10 +1099,7 @@ mod tests { vtag.apply(&parent, None, Some(VNode::VTag(ancestor))); let expected = "class-3 class-2 class-1"; - assert_eq!( - vtag.attributes.get("class").map(AsRef::as_ref), - Some(expected) - ); + assert_eq!(get_class_str(&vtag), expected); assert_eq!( vtag.reference .as_ref() @@ -1165,10 +1129,7 @@ mod tests { }; let expected = "class-1 class-3"; - assert_eq!( - vtag.attributes.get("class").map(AsRef::as_ref), - Some(expected) - ); + assert_eq!(get_class_str(&vtag), expected); assert_eq!( vtag.reference .as_ref() @@ -1188,10 +1149,7 @@ mod tests { vtag.apply(&parent, None, Some(VNode::VTag(ancestor))); let expected = "class-1 class-2 class-3"; - assert_eq!( - vtag.attributes.get("class").map(AsRef::as_ref), - Some(expected) - ); + assert_eq!(get_class_str(&vtag), expected); assert_eq!( vtag.reference .as_ref() From 03fd474da84e13dea03f9af5cb47ccca22232b67 Mon Sep 17 00:00:00 2001 From: liquidblock Date: Tue, 28 Apr 2020 20:45:12 +0200 Subject: [PATCH 13/14] Add test case for Option<&'static str> variable --- yew/src/virtual_dom/vtag.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/yew/src/virtual_dom/vtag.rs b/yew/src/virtual_dom/vtag.rs index a4cdabd7073..854e4611b6e 100644 --- a/yew/src/virtual_dom/vtag.rs +++ b/yew/src/virtual_dom/vtag.rs @@ -1054,6 +1054,11 @@ mod tests { assert_class(html! {

::None /> }, ""); assert_class(html! {

::None /> }, ""); // check with variables + let some: Option<&'static str> = Some("some"); + let none: Option<&'static str> = None; + assert_class(html! {

}, "some"); + assert_class(html! {

}, ""); + // check with variables of different type let some: Option = Some(false); let none: Option = None; assert_class(html! {

}, "false"); From 391c122f3e8d7d97707becfb5f50cd4a7bd83ee0 Mon Sep 17 00:00:00 2001 From: liquidblock Date: Tue, 28 Apr 2020 20:46:22 +0200 Subject: [PATCH 14/14] Remove unnecessary map in test --- yew/src/virtual_dom/vtag.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/yew/src/virtual_dom/vtag.rs b/yew/src/virtual_dom/vtag.rs index 854e4611b6e..ed4aa90ec67 100644 --- a/yew/src/virtual_dom/vtag.rs +++ b/yew/src/virtual_dom/vtag.rs @@ -1056,8 +1056,8 @@ mod tests { // check with variables let some: Option<&'static str> = Some("some"); let none: Option<&'static str> = None; - assert_class(html! {

}, "some"); - assert_class(html! {

}, ""); + assert_class(html! {

}, "some"); + assert_class(html! {

}, ""); // check with variables of different type let some: Option = Some(false); let none: Option = None;