-
-
Notifications
You must be signed in to change notification settings - Fork 172
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Handle classes within Node #92
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
a4f0f4f
fixed some formatting, add starter tests
Kiyoshika d9e0866
finish tests in ElementClass
Kiyoshika 6156e93
add constructor by string for ElementClass, simple Node integration
Kiyoshika 888dc2a
finish ElementClass API and tests
Kiyoshika 98e6130
rename count() to len() in ElementClass to be more 'rusty'
Kiyoshika 92ef529
simplify boolean assertions with clippy
Kiyoshika 6cec23c
Merge branch 'main' into feat/handle-node-classes
Kiyoshika File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,157 @@ | ||
use std::collections::HashMap; | ||
|
||
#[derive(Debug, Clone, PartialEq)] | ||
pub struct ElementClass { | ||
/// a map of classes applied to an HTML element. | ||
/// key = name, value = is_active | ||
/// the is_active is used to toggle a class (JavaScript API) | ||
class_map: HashMap<String, bool>, | ||
} | ||
|
||
impl Default for ElementClass { | ||
fn default() -> Self { | ||
Self::new() | ||
} | ||
} | ||
|
||
impl ElementClass { | ||
/// Initialise a new (empty) ElementClass | ||
pub fn new() -> Self { | ||
ElementClass { | ||
class_map: HashMap::new(), | ||
} | ||
} | ||
|
||
/// Initialize a class from a class string | ||
/// with space-delimited class names | ||
pub fn from_string(class_string: &str) -> Self { | ||
let mut class_map_local = HashMap::new(); | ||
let classes = class_string.split_whitespace(); | ||
for class_name in classes { | ||
class_map_local.insert(class_name.to_owned(), true); | ||
} | ||
|
||
ElementClass { | ||
class_map: class_map_local, | ||
} | ||
} | ||
|
||
/// Count the number of classes (active or inactive) | ||
/// assigned to an element | ||
pub fn len(&self) -> usize { | ||
self.class_map.len() | ||
} | ||
|
||
/// Check if any classes are present | ||
pub fn is_empty(&self) -> bool { | ||
self.class_map.is_empty() | ||
} | ||
|
||
/// Check if class name exists | ||
pub fn contains(&self, name: &str) -> bool { | ||
self.class_map.contains_key(name) | ||
} | ||
|
||
/// Add a new class (if already exists, does nothing) | ||
pub fn add(&mut self, name: &str) { | ||
// by default, adding a new class will be active. | ||
// however, map.insert will update a key if it exists | ||
// and we don't want to overwrite an inactive class to make it active unintentionally | ||
// so we ignore this operation if the class already exists | ||
if !self.contains(name) { | ||
self.class_map.insert(name.to_owned(), true); | ||
} | ||
} | ||
|
||
/// Remove a class (does nothing if not exists) | ||
pub fn remove(&mut self, name: &str) { | ||
self.class_map.remove(name); | ||
} | ||
|
||
/// Toggle a class active/inactive. Does nothing if class doesn't exist | ||
pub fn toggle(&mut self, name: &str) { | ||
if let Some(is_active) = self.class_map.get_mut(name) { | ||
*is_active = !*is_active; | ||
} | ||
} | ||
|
||
/// Set explicitly if a class is active or not. Does nothing if class doesn't exist | ||
pub fn set_active(&mut self, name: &str, is_active: bool) { | ||
if let Some(is_active_item) = self.class_map.get_mut(name) { | ||
*is_active_item = is_active; | ||
} | ||
} | ||
|
||
/// Check if a class is active. Returns false if class doesn't exist | ||
pub fn is_active(&self, name: &str) -> bool { | ||
if let Some(is_active) = self.class_map.get(name) { | ||
return *is_active; | ||
} | ||
|
||
false | ||
} | ||
} | ||
|
||
#[cfg(test)] | ||
mod tests { | ||
use super::*; | ||
|
||
#[test] | ||
fn is_empty() { | ||
let mut classes = ElementClass::new(); | ||
assert!(classes.is_empty()); | ||
classes.add("one"); | ||
assert!(!classes.is_empty()); | ||
} | ||
|
||
#[test] | ||
fn count_classes() { | ||
let mut classes = ElementClass::new(); | ||
classes.add("one"); | ||
classes.add("two"); | ||
assert_eq!(classes.len(), 2); | ||
} | ||
|
||
#[test] | ||
fn contains_nonexistant_class() { | ||
let classes = ElementClass::new(); | ||
assert!(!classes.contains("nope")); | ||
} | ||
|
||
#[test] | ||
fn contains_valid_class() { | ||
let mut classes = ElementClass::new(); | ||
classes.add("yep"); | ||
assert!(classes.contains("yep")); | ||
} | ||
|
||
#[test] | ||
fn add_class() { | ||
let mut classes = ElementClass::new(); | ||
classes.add("yep"); | ||
assert!(classes.is_active("yep")); | ||
|
||
classes.set_active("yep", false); | ||
classes.add("yep"); // should be ignored | ||
assert!(!classes.is_active("yep")); | ||
} | ||
|
||
#[test] | ||
fn remove_class() { | ||
let mut classes = ElementClass::new(); | ||
classes.add("yep"); | ||
classes.remove("yep"); | ||
assert!(!classes.contains("yep")); | ||
} | ||
|
||
#[test] | ||
fn toggle_class() { | ||
let mut classes = ElementClass::new(); | ||
classes.add("yep"); | ||
assert!(classes.is_active("yep")); | ||
classes.toggle("yep"); | ||
assert!(!classes.is_active("yep")); | ||
classes.toggle("yep"); | ||
assert!(classes.is_active("yep")); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,4 +7,6 @@ pub mod dom; | |
pub mod error_logger; | ||
pub mod input_stream; | ||
|
||
pub mod element_class; | ||
|
||
mod node_arena; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
use crate::html5_parser::element_class::ElementClass; | ||
use derive_more::Display; | ||
use std::collections::HashMap; | ||
|
||
|
@@ -98,6 +99,8 @@ pub struct Node { | |
pub namespace: Option<String>, | ||
/// actual data of the node | ||
pub data: NodeData, | ||
/// CSS classes (only relevant for NodeType::Element, otherwise None) | ||
pub classes: Option<ElementClass>, | ||
} | ||
|
||
impl Node { | ||
|
@@ -117,6 +120,7 @@ impl Clone for Node { | |
name: self.name.clone(), | ||
namespace: self.namespace.clone(), | ||
data: self.data.clone(), | ||
classes: self.classes.clone(), | ||
} | ||
} | ||
} | ||
|
@@ -131,6 +135,7 @@ impl Node { | |
data: NodeData::Document {}, | ||
name: "".to_string(), | ||
namespace: None, | ||
classes: None, | ||
} | ||
} | ||
|
||
|
@@ -146,6 +151,7 @@ impl Node { | |
}, | ||
name: name.to_string(), | ||
namespace: Some(namespace.into()), | ||
classes: Some(ElementClass::new()), | ||
} | ||
} | ||
|
||
|
@@ -160,6 +166,7 @@ impl Node { | |
}, | ||
name: "".to_string(), | ||
namespace: None, | ||
classes: None, | ||
} | ||
} | ||
|
||
|
@@ -174,6 +181,7 @@ impl Node { | |
}, | ||
name: "".to_string(), | ||
namespace: None, | ||
classes: None, | ||
} | ||
} | ||
|
||
|
@@ -246,32 +254,32 @@ impl Node { | |
|
||
/// Get a constant reference to the attribute value | ||
/// (or None if attribute doesn't exist) | ||
pub fn get_attribute(&self, name: &str) -> Result<Option<&String>, String> { | ||
pub fn get_attribute(&self, name: &str) -> Option<&String> { | ||
if self.type_of() != NodeType::Element { | ||
return Err(ATTRIBUTE_NODETYPE_ERR_MSG.into()); | ||
return None; | ||
} | ||
|
||
let mut value: Option<&String> = None; | ||
if let NodeData::Element { attributes, .. } = &self.data { | ||
value = attributes.get(name); | ||
} | ||
|
||
Ok(value) | ||
value | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We'll have to keep our eye on whether there are error cases in CSS that need to be handled. But this change feels like a good next step. I'm thinking we can backfill error handling when we discover that it's needed. |
||
} | ||
|
||
/// Get a mutable reference to the attribute value | ||
/// (or None if the attribute doesn't exist) | ||
pub fn get_mut_attribute(&mut self, name: &str) -> Result<Option<&mut String>, String> { | ||
pub fn get_mut_attribute(&mut self, name: &str) -> Option<&mut String> { | ||
if self.type_of() != NodeType::Element { | ||
return Err(ATTRIBUTE_NODETYPE_ERR_MSG.into()); | ||
return None; | ||
} | ||
|
||
let mut value: Option<&mut String> = None; | ||
if let NodeData::Element { attributes, .. } = &mut self.data { | ||
value = attributes.get_mut(name); | ||
} | ||
|
||
Ok(value) | ||
value | ||
} | ||
|
||
/// Remove all attributes | ||
|
@@ -649,7 +657,7 @@ mod tests { | |
let mut node = Node::new_element("name", attr.clone(), HTML_NAMESPACE); | ||
|
||
assert!(node.insert_attribute("key", "value").is_ok()); | ||
let value = node.get_attribute("key").unwrap().unwrap(); | ||
let value = node.get_attribute("key").unwrap(); | ||
assert_eq!(value, "value"); | ||
} | ||
|
||
|
@@ -676,7 +684,7 @@ mod tests { | |
fn get_attribute_non_element() { | ||
let node = Node::new_document(); | ||
let result = node.get_attribute("name"); | ||
assert!(result.is_err()); | ||
assert!(result.is_none()); | ||
} | ||
|
||
#[test] | ||
|
@@ -686,15 +694,15 @@ mod tests { | |
|
||
let node = Node::new_element("name", attr.clone(), HTML_NAMESPACE); | ||
|
||
let value = node.get_attribute("key").unwrap().unwrap(); | ||
let value = node.get_attribute("key").unwrap(); | ||
assert_eq!(value, "value"); | ||
} | ||
|
||
#[test] | ||
fn get_mut_attribute_non_element() { | ||
let mut node = Node::new_document(); | ||
let result = node.get_mut_attribute("key"); | ||
assert!(result.is_err()); | ||
assert!(result.is_none()); | ||
} | ||
|
||
#[test] | ||
|
@@ -704,10 +712,10 @@ mod tests { | |
|
||
let mut node = Node::new_element("name", attr.clone(), HTML_NAMESPACE); | ||
|
||
let value = node.get_mut_attribute("key").unwrap().unwrap(); | ||
let value = node.get_mut_attribute("key").unwrap(); | ||
value.push_str(" appended"); | ||
|
||
let value = node.get_attribute("key").unwrap().unwrap(); | ||
let value = node.get_attribute("key").unwrap(); | ||
assert_eq!(value, "value appended"); | ||
} | ||
|
||
|
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably want to keep an eye on this kind of substring matching. Might be fine indefinitely, or might cause problems later on.I see we're using a
HashMap
.