Skip to content

Commit

Permalink
refactor!: Rename name to label and use value for label content (…
Browse files Browse the repository at this point in the history 
…#475)
  • Loading branch information
@mwcampbell
mwcampbell authored Oct 29, 2024
1 parent 7d8910e commit e0053a5
Show file tree
Hide file tree
Showing 14 changed files with 102 additions and 102 deletions.
12 changes: 8 additions & 4 deletions common/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -784,7 +784,7 @@ enum PropertyId {
PopupFor,

// String
Name,
Label,
Description,
Value,
AccessKey,
Expand Down Expand Up @@ -1537,7 +1537,11 @@ node_id_property_methods! {
}

string_property_methods! {
(Name, name, set_name, clear_name),
/// The label of a control that can have a label. If the label is specified
/// via the [`Node::labelled_by`] relation, this doesn't need to be set.
/// Note that the text content of a node with the [`Role::Label`] role
/// should be provided via [`Node::value`], not this property.
(Label, label, set_label, clear_label),
(Description, description, set_description, clear_description),
(Value, value, set_value, clear_value),
/// A single character, usually part of this node's name, that can be pressed,
Expand Down Expand Up @@ -1982,7 +1986,7 @@ impl<'de> Visitor<'de> for PropertiesVisitor {
PopupFor
},
String {
Name,
Label,
Description,
Value,
AccessKey,
Expand Down Expand Up @@ -2129,7 +2133,7 @@ impl JsonSchema for Properties {
PopupFor
},
Box<str> {
Name,
Label,
Description,
Value,
AccessKey,
Expand Down
14 changes: 7 additions & 7 deletions consumer/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ mod tests {
};
let label_0_0_ignored = {
let mut node = Node::new(Role::Label);
node.set_name("label_0_0_ignored");
node.set_value("label_0_0_ignored");
node
};
let paragraph_1_ignored = {
Expand All @@ -83,7 +83,7 @@ mod tests {
};
let button_1_0_hidden = {
let mut node = Node::new(Role::Button);
node.set_name("button_1_0_hidden");
node.set_label("button_1_0_hidden");
node.set_hidden();
node.set_children(vec![CONTAINER_1_0_0_HIDDEN_ID]);
node
Expand All @@ -101,12 +101,12 @@ mod tests {
x1: 90.0,
y1: 30.0,
});
node.set_name("label_1_1");
node.set_value("label_1_1");
node
};
let button_1_2_hidden = {
let mut node = Node::new(Role::Button);
node.set_name("button_1_2_hidden");
node.set_label("button_1_2_hidden");
node.set_hidden();
node.set_children(vec![CONTAINER_1_2_0_HIDDEN_ID]);
node
Expand All @@ -123,7 +123,7 @@ mod tests {
};
let label_2_0 = {
let mut node = Node::new(Role::Label);
node.set_name("label_2_0");
node.set_label("label_2_0");
node
};
let paragraph_3_ignored = {
Expand All @@ -145,12 +145,12 @@ mod tests {
};
let label_3_1_0 = {
let mut node = Node::new(Role::Label);
node.set_name("label_3_1_0");
node.set_value("label_3_1_0");
node
};
let button_3_2 = {
let mut node = Node::new(Role::Button);
node.set_name("button_3_2");
node.set_label("button_3_2");
node
};
let empty_container_3_3_ignored = Node::new(Role::GenericContainer);
Expand Down
70 changes: 40 additions & 30 deletions consumer/src/node.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -492,15 +492,25 @@ impl<'a> Node<'a> {
}
}

pub fn name(&self) -> Option<String> {
if let Some(name) = &self.data().name() {
Some(name.to_string())
pub fn label_comes_from_value(&self) -> bool {
self.role() == Role::Label
}

pub fn label(&self) -> Option<String> {
if let Some(label) = &self.data().label() {
Some(label.to_string())
} else {
let names = self
let labels = self
.labelled_by()
.filter_map(|node| node.name())
.filter_map(|node| {
if node.label_comes_from_value() {
node.value()
} else {
node.label()
}
})
.collect::<Vec<String>>();
(!names.is_empty()).then(move || names.join(" "))
(!labels.is_empty()).then(move || labels.join(" "))
}
}

Expand Down Expand Up @@ -916,7 +926,7 @@ mod tests {
}

#[test]
fn no_name_or_labelled_by() {
fn no_label_or_labelled_by() {
let update = TreeUpdate {
nodes: vec![
(NodeId(0), {
Expand All @@ -930,11 +940,11 @@ mod tests {
focus: NodeId(0),
};
let tree = crate::Tree::new(update, false);
assert_eq!(None, tree.state().node_by_id(NodeId(1)).unwrap().name());
assert_eq!(None, tree.state().node_by_id(NodeId(1)).unwrap().label());
}

#[test]
fn name_from_labelled_by() {
fn label_from_labelled_by() {
// The following mock UI probably isn't very localization-friendly,
// but it's good for this test.
const LABEL_1: &str = "Check email every";
Expand All @@ -954,7 +964,7 @@ mod tests {
}),
(NodeId(2), {
let mut node = Node::new(Role::Label);
node.set_name(LABEL_1);
node.set_value(LABEL_1);
node
}),
(NodeId(3), {
Expand All @@ -964,7 +974,7 @@ mod tests {
}),
(NodeId(4), {
let mut node = Node::new(Role::Label);
node.set_name(LABEL_2);
node.set_value(LABEL_2);
node
}),
],
Expand All @@ -974,16 +984,16 @@ mod tests {
let tree = crate::Tree::new(update, false);
assert_eq!(
Some([LABEL_1, LABEL_2].join(" ")),
tree.state().node_by_id(NodeId(1)).unwrap().name()
tree.state().node_by_id(NodeId(1)).unwrap().label()
);
assert_eq!(
Some(LABEL_2.into()),
tree.state().node_by_id(NodeId(3)).unwrap().name()
tree.state().node_by_id(NodeId(3)).unwrap().label()
);
}

#[test]
fn name_from_descendant_label() {
fn label_from_descendant_label() {
const ROOT_ID: NodeId = NodeId(0);
const DEFAULT_BUTTON_ID: NodeId = NodeId(1);
const DEFAULT_BUTTON_LABEL_ID: NodeId = NodeId(2);
Expand Down Expand Up @@ -1034,7 +1044,7 @@ mod tests {
}),
(DEFAULT_BUTTON_LABEL_ID, {
let mut node = Node::new(Role::Image);
node.set_name(DEFAULT_BUTTON_LABEL);
node.set_label(DEFAULT_BUTTON_LABEL);
node
}),
(LINK_ID, {
Expand All @@ -1049,7 +1059,7 @@ mod tests {
}),
(LINK_LABEL_ID, {
let mut node = Node::new(Role::Label);
node.set_name(LINK_LABEL);
node.set_value(LINK_LABEL);
node
}),
(CHECKBOX_ID, {
Expand All @@ -1059,7 +1069,7 @@ mod tests {
}),
(CHECKBOX_LABEL_ID, {
let mut node = Node::new(Role::Label);
node.set_name(CHECKBOX_LABEL);
node.set_value(CHECKBOX_LABEL);
node
}),
(RADIO_BUTTON_ID, {
Expand All @@ -1069,7 +1079,7 @@ mod tests {
}),
(RADIO_BUTTON_LABEL_ID, {
let mut node = Node::new(Role::Label);
node.set_name(RADIO_BUTTON_LABEL);
node.set_value(RADIO_BUTTON_LABEL);
node
}),
(MENU_BUTTON_ID, {
Expand All @@ -1079,7 +1089,7 @@ mod tests {
}),
(MENU_BUTTON_LABEL_ID, {
let mut node = Node::new(Role::Label);
node.set_name(MENU_BUTTON_LABEL);
node.set_value(MENU_BUTTON_LABEL);
node
}),
(MENU_ID, {
Expand All @@ -1094,7 +1104,7 @@ mod tests {
}),
(MENU_ITEM_LABEL_ID, {
let mut node = Node::new(Role::Label);
node.set_name(MENU_ITEM_LABEL);
node.set_value(MENU_ITEM_LABEL);
node
}),
(MENU_ITEM_CHECKBOX_ID, {
Expand All @@ -1104,7 +1114,7 @@ mod tests {
}),
(MENU_ITEM_CHECKBOX_LABEL_ID, {
let mut node = Node::new(Role::Label);
node.set_name(MENU_ITEM_CHECKBOX_LABEL);
node.set_value(MENU_ITEM_CHECKBOX_LABEL);
node
}),
(MENU_ITEM_RADIO_ID, {
Expand All @@ -1114,7 +1124,7 @@ mod tests {
}),
(MENU_ITEM_RADIO_LABEL_ID, {
let mut node = Node::new(Role::Label);
node.set_name(MENU_ITEM_RADIO_LABEL);
node.set_value(MENU_ITEM_RADIO_LABEL);
node
}),
],
Expand All @@ -1124,38 +1134,38 @@ mod tests {
let tree = crate::Tree::new(update, false);
assert_eq!(
Some(DEFAULT_BUTTON_LABEL.into()),
tree.state().node_by_id(DEFAULT_BUTTON_ID).unwrap().name()
tree.state().node_by_id(DEFAULT_BUTTON_ID).unwrap().label()
);
assert_eq!(
Some(LINK_LABEL.into()),
tree.state().node_by_id(LINK_ID).unwrap().name()
tree.state().node_by_id(LINK_ID).unwrap().label()
);
assert_eq!(
Some(CHECKBOX_LABEL.into()),
tree.state().node_by_id(CHECKBOX_ID).unwrap().name()
tree.state().node_by_id(CHECKBOX_ID).unwrap().label()
);
assert_eq!(
Some(RADIO_BUTTON_LABEL.into()),
tree.state().node_by_id(RADIO_BUTTON_ID).unwrap().name()
tree.state().node_by_id(RADIO_BUTTON_ID).unwrap().label()
);
assert_eq!(
Some(MENU_BUTTON_LABEL.into()),
tree.state().node_by_id(MENU_BUTTON_ID).unwrap().name()
tree.state().node_by_id(MENU_BUTTON_ID).unwrap().label()
);
assert_eq!(
Some(MENU_ITEM_LABEL.into()),
tree.state().node_by_id(MENU_ITEM_ID).unwrap().name()
tree.state().node_by_id(MENU_ITEM_ID).unwrap().label()
);
assert_eq!(
Some(MENU_ITEM_CHECKBOX_LABEL.into()),
tree.state()
.node_by_id(MENU_ITEM_CHECKBOX_ID)
.unwrap()
.name()
.label()
);
assert_eq!(
Some(MENU_ITEM_RADIO_LABEL.into()),
tree.state().node_by_id(MENU_ITEM_RADIO_ID).unwrap().name()
tree.state().node_by_id(MENU_ITEM_RADIO_ID).unwrap().label()
);
}
}
14 changes: 7 additions & 7 deletions consumer/src/tree.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -647,7 +647,7 @@ mod tests {
}),
(NodeId(1), {
let mut node = child_node.clone();
node.set_name("foo");
node.set_label("foo");
node
}),
],
Expand All @@ -657,12 +657,12 @@ mod tests {
let mut tree = super::Tree::new(first_update, false);
assert_eq!(
Some("foo".into()),
tree.state().node_by_id(NodeId(1)).unwrap().name()
tree.state().node_by_id(NodeId(1)).unwrap().label()
);
let second_update = TreeUpdate {
nodes: vec![(NodeId(1), {
let mut node = child_node;
node.set_name("bar");
node.set_label("bar");
node
})],
tree: None,
Expand All @@ -680,8 +680,8 @@ mod tests {
}
fn node_updated(&mut self, old_node: &crate::Node, new_node: &crate::Node) {
if new_node.id() == NodeId(1)
&& old_node.name() == Some("foo".into())
&& new_node.name() == Some("bar".into())
&& old_node.label() == Some("foo".into())
&& new_node.label() == Some("bar".into())
{
self.got_updated_child_node = true;
return;
Expand All @@ -706,7 +706,7 @@ mod tests {
assert!(handler.got_updated_child_node);
assert_eq!(
Some("bar".into()),
tree.state().node_by_id(NodeId(1)).unwrap().name()
tree.state().node_by_id(NodeId(1)).unwrap().label()
);
}

Expand All @@ -725,7 +725,7 @@ mod tests {
}),
(NodeId(1), {
let mut node = Node::new(Role::Button);
node.set_name("foo");
node.set_label("foo");
node
}),
],
Expand Down
6 changes: 5 additions & 1 deletion platforms/atspi-common/src/node.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,11 @@ pub(crate) struct NodeWrapper<'a>(pub(crate) &'a Node<'a>);

impl<'a> NodeWrapper<'a> {
pub(crate) fn name(&self) -> Option<String> {
self.0.name()
if self.0.label_comes_from_value() {
self.0.value()
} else {
self.0.label()
}
}

pub(crate) fn description(&self) -> Option<String> {
Expand Down
Loading

0 comments on commit e0053a5

Please sign in to comment.