Skip to content
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 7 commits into from
Oct 6, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
157 changes: 157 additions & 0 deletions src/html5_parser/element_class.rs
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 count(&self) -> usize {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be better to stick with the Rust style here and use len as the method name? Especially because there is an is_empty method, which gets suggested by clippy when doing any comparison to zero, e.g. list.len() == 0.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, good suggestion. i'm still not entirely familiar with all of Rust's conventions yet. Will fix now

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated in 98e6130

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) {
Copy link
Collaborator

@emwalker emwalker Oct 5, 2023

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.

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_eq!(classes.is_empty(), true);
classes.add("one");
assert_eq!(classes.is_empty(), false);
}

#[test]
fn count_classes() {
let mut classes = ElementClass::new();
classes.add("one");
classes.add("two");
assert_eq!(classes.count(), 2);
}

#[test]
fn contains_nonexistant_class() {
let classes = ElementClass::new();
assert_eq!(classes.contains("nope"), false);
}

#[test]
fn contains_valid_class() {
let mut classes = ElementClass::new();
classes.add("yep");
assert_eq!(classes.contains("yep"), true);
}

#[test]
fn add_class() {
let mut classes = ElementClass::new();
classes.add("yep");
assert_eq!(classes.is_active("yep"), true);

classes.set_active("yep", false);
classes.add("yep"); // should be ignored
assert_eq!(classes.is_active("yep"), false);
}

#[test]
fn remove_class() {
let mut classes = ElementClass::new();
classes.add("yep");
classes.remove("yep");
assert_eq!(classes.contains("yep"), false);
}

#[test]
fn toggle_class() {
let mut classes = ElementClass::new();
classes.add("yep");
assert_eq!(classes.is_active("yep"), true);
classes.toggle("yep");
assert_eq!(classes.is_active("yep"), false);
classes.toggle("yep");
assert_eq!(classes.is_active("yep"), true);
}
}
2 changes: 2 additions & 0 deletions src/html5_parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,6 @@ pub mod dom;
pub mod error_logger;
pub mod input_stream;

pub mod element_class;

mod node_arena;
32 changes: 20 additions & 12 deletions src/html5_parser/node.rs
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;

Expand Down Expand Up @@ -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 {
Expand All @@ -117,6 +120,7 @@ impl Clone for Node {
name: self.name.clone(),
namespace: self.namespace.clone(),
data: self.data.clone(),
classes: self.classes.clone(),
}
}
}
Expand All @@ -131,6 +135,7 @@ impl Node {
data: NodeData::Document {},
name: "".to_string(),
namespace: None,
classes: None,
}
}

Expand All @@ -146,6 +151,7 @@ impl Node {
},
name: name.to_string(),
namespace: Some(namespace.into()),
classes: Some(ElementClass::new()),
}
}

Expand All @@ -160,6 +166,7 @@ impl Node {
},
name: "".to_string(),
namespace: None,
classes: None,
}
}

Expand All @@ -174,6 +181,7 @@ impl Node {
},
name: "".to_string(),
namespace: None,
classes: None,
}
}

Expand Down Expand Up @@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

The 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
Expand Down Expand Up @@ -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");
}

Expand All @@ -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]
Expand All @@ -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]
Expand All @@ -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");
}

Expand Down
Loading
Loading