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

feat: Only focus focusable nodes #884

Merged
merged 6 commits into from
Sep 19, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 0 additions & 1 deletion crates/components/src/button.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,6 @@ pub fn Button(
height: "{height}",
padding: "{padding}",
margin: "{margin}",
a11y_focusable: "true",
overflow: "clip",
a11y_role:"button",
color: "{font_theme.color}",
Expand Down
1 change: 0 additions & 1 deletion crates/components/src/input.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,6 @@ pub fn Input(
main_align: "center",
cursor_reference,
a11y_id,
a11y_focusable: "true",
a11y_role: "textInput",
a11y_auto_focus: "{auto_focus}",
onkeydown,
Expand Down
1 change: 0 additions & 1 deletion crates/components/src/menu.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,6 @@ pub fn MenuItem(
width: "fill-min",
padding: "6",
margin: "2",
a11y_focusable: "true",
a11y_role:"button",
color: "{font_theme.color}",
corner_radius: "{corner_radius}",
Expand Down
1 change: 0 additions & 1 deletion crates/components/src/radio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ pub fn Radio(
corner_radius: "99",
rect {
a11y_id: focus.attribute(),
a11y_focusable: "true",
width: "18",
height: "18",
border: "2 solid {fill}",
Expand Down
2 changes: 0 additions & 2 deletions crates/components/src/tabs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,6 @@ pub fn Tab(
a11y_id,
width: "{width}",
height: "{height}",
a11y_focusable: "true",
overflow: "clip",
a11y_role:"tab",
color: "{font_theme.color}",
Expand Down Expand Up @@ -254,7 +253,6 @@ pub fn BottomTab(children: Element, theme: Option<BottomTabThemeWith>) -> Elemen
a11y_id,
width: "{width}",
height: "{height}",
a11y_focusable: "true",
overflow: "clip",
a11y_role:"tab",
color: "{font_theme.color}",
Expand Down
7 changes: 5 additions & 2 deletions crates/core/src/accessibility/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,10 @@ impl AccessibilityTree {
let accessibility_id = node_ref.get_accessibility_id();

if let Some(accessibility_id) = accessibility_id {
nodes.push((accessibility_id, node_ref.id()))
let accessibility_state = node_ref.get::<AccessibilityNodeState>().unwrap();
if accessibility_state.a11y_focusable.is_enabled() {
nodes.push((accessibility_id, node_ref.id()))
}
}

if let Some(tag) = node_ref.node_type().tag() {
Expand Down Expand Up @@ -347,7 +350,7 @@ impl AccessibilityTree {
// Set focusable action
// This will cause assistive technology to offer the user an option
// to focus the current element if it supports it.
if node_accessibility.a11y_focusable {
if node_accessibility.a11y_focusable.is_enabled() {
builder.add_action(Action::Focus);
}

Expand Down
9 changes: 4 additions & 5 deletions crates/elements/src/definitions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -235,10 +235,10 @@ builder_constructors! {

a11y_auto_focus: String,
a11y_name: String,
a11y_focusable: String,
a11y_role:String,
a11y_id: AccessibilityId,
a11y_alt: String,
a11y_focusable: String,
canvas_reference: String,
layer: String,
offset_y: String,
Expand Down Expand Up @@ -308,10 +308,10 @@ builder_constructors! {
layer: String,
a11y_auto_focus: String,
a11y_name: String,
a11y_focusable: String,
a11y_role:String,
a11y_id: AccessibilityId,
a11y_alt: String,
a11y_focusable: String,
};
/// `paragraph` element let's you build texts with different styles.
///
Expand Down Expand Up @@ -387,10 +387,10 @@ builder_constructors! {
cursor_id: String,
a11y_auto_focus: String,
a11y_name: String,
a11y_focusable: String,
a11y_role:String,
a11y_id: AccessibilityId,
a11y_alt: String,
a11y_focusable: String,
highlights: String,
highlight_color: String,
highlight_mode: String,
Expand Down Expand Up @@ -459,7 +459,6 @@ builder_constructors! {
image_reference: String,
a11y_auto_focus: String,
a11y_name: String,
a11y_focusable: String,
a11y_role:String,
a11y_id: AccessibilityId,
a11y_alt: String,
Expand Down Expand Up @@ -499,10 +498,10 @@ builder_constructors! {
svg_content: String,
a11y_auto_focus: String,
a11y_name: String,
a11y_focusable: String,
a11y_role:String,
a11y_id: AccessibilityId,
a11y_alt: String,
a11y_focusable: String,
};
}

Expand Down
6 changes: 6 additions & 0 deletions crates/hooks/src/use_init_native_platform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,9 @@ mod test {
pub async fn uncontrolled_focus_accessibility() {
#[allow(non_snake_case)]
fn OtherChild() -> Element {
let mut focus = use_focus();
rsx!(rect {
a11y_id: focus.attribute(),
a11y_role: "genericContainer",
width: "100%",
height: "50%",
Expand Down Expand Up @@ -216,12 +218,16 @@ mod test {
#[tokio::test]
pub async fn auto_focus_accessibility() {
fn use_focus_app() -> Element {
let mut focus_1 = use_focus();
let mut focus_2 = use_focus();
rsx!(
rect {
a11y_id: focus_1.attribute(),
a11y_role: "genericContainer",
a11y_auto_focus: "true",
}
rect {
a11y_id: focus_2.attribute(),
a11y_role: "genericContainer",
a11y_auto_focus: "true",
}
Expand Down
2 changes: 1 addition & 1 deletion crates/native-core/src/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,10 +119,10 @@ impl FromStr for AttributeName {
"content" => Ok(AttributeName::Content),
"a11y_auto_focus" => Ok(AttributeName::A11YAutoFocus),
"a11y_name" => Ok(AttributeName::A11YName),
"a11y_focusable" => Ok(AttributeName::A11YFocusable),
"a11y_role" => Ok(AttributeName::A11YRole),
"a11y_id" => Ok(AttributeName::A11YId),
"a11y_alt" => Ok(AttributeName::A11YAlt),
"a11y_focusable" => Ok(AttributeName::A11YFocusable),
"canvas_reference" => Ok(AttributeName::CanvasReference),
"layer" => Ok(AttributeName::Layer),
"offset_y" => Ok(AttributeName::OffsetY),
Expand Down
20 changes: 13 additions & 7 deletions crates/state/src/accessibility.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ use freya_native_core_macro::partial_derive_state;

use crate::{
CustomAttributeValues,
Focusable,
Parse,
ParseAttribute,
ParseError,
};
Expand All @@ -42,8 +44,8 @@ pub struct AccessibilityNodeState {
pub a11y_role: Option<Role>,
pub a11y_alt: Option<String>,
pub a11y_name: Option<String>,
pub a11y_focusable: bool,
pub a11y_auto_focus: bool,
pub a11y_focusable: Focusable,
}

impl ParseAttribute for AccessibilityNodeState {
Expand All @@ -57,6 +59,11 @@ impl ParseAttribute for AccessibilityNodeState {
attr.value
{
self.a11y_id = Some(*id);

// Enable focus on nodes that pass a custom a11y id
if self.a11y_focusable.is_unknown() {
self.a11y_focusable = Focusable::Enabled;
}
}
}
AttributeName::A11YRole => {
Expand All @@ -77,14 +84,14 @@ impl ParseAttribute for AccessibilityNodeState {
self.a11y_name = Some(attr.to_owned())
}
}
AttributeName::A11YFocusable => {
AttributeName::A11YAutoFocus => {
if let OwnedAttributeValue::Text(attr) = attr.value {
self.a11y_focusable = attr.parse().unwrap_or_default()
self.a11y_auto_focus = attr.parse().unwrap_or_default()
}
}
AttributeName::A11YAutoFocus => {
AttributeName::A11YFocusable => {
if let OwnedAttributeValue::Text(attr) = attr.value {
self.a11y_auto_focus = attr.parse().unwrap_or_default()
self.a11y_focusable = Focusable::parse(attr)?;
}
}
_ => {}
Expand All @@ -108,7 +115,6 @@ impl State<CustomAttributeValues> for AccessibilityNodeState {
AttributeName::A11YRole,
AttributeName::A11YAlt,
AttributeName::A11YName,
AttributeName::A11YFocusable,
AttributeName::A11YAutoFocus,
]));

Expand Down Expand Up @@ -171,7 +177,7 @@ impl State<CustomAttributeValues> for AccessibilityNodeState {
#[cfg(debug_assertions)]
tracing::info!("Assigned {id:?} to {:?}", node_view.node_id());

self.a11y_id = Some(id)
self.a11y_id = Some(id);
}

let was_just_created = !had_id && self.a11y_id.is_some();
Expand Down
32 changes: 32 additions & 0 deletions crates/state/src/values/focusable.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
use crate::{
Parse,
ParseError,
};

#[derive(Clone, Debug, PartialEq, Default, Eq)]
pub enum Focusable {
#[default]
Unknown,
Disabled,
Enabled,
}

impl Focusable {
pub fn is_unknown(&self) -> bool {
matches!(self, Self::Unknown)
}

pub fn is_enabled(&self) -> bool {
matches!(self, Self::Enabled)
}
}

impl Parse for Focusable {
fn parse(value: &str) -> Result<Self, ParseError> {
Ok(match value {
"true" => Self::Enabled,
"false" => Self::Disabled,
_ => Self::Unknown,
})
}
}
2 changes: 2 additions & 0 deletions crates/state/src/values/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ mod corner_radius;
mod cursor;
mod decoration;
mod fill;
mod focusable;
mod font;
mod gaps;
mod gradient;
Expand All @@ -21,6 +22,7 @@ pub use color::*;
pub use corner_radius::*;
pub use cursor::*;
pub use fill::*;
pub use focusable::*;
pub use font::*;
pub use gradient::*;
pub use highlight::*;
Expand Down
23 changes: 23 additions & 0 deletions crates/state/tests/parse_focusable.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
use freya_engine::prelude::*;
use freya_node_state::{
Focusable,
Parse,
};

#[test]
fn parse_focusable_enabled() {
let enabled = Focusable::parse("true");
assert_eq!(enabled, Ok(Focusable::Enabled));
}

#[test]
fn parse_focusable_disabled() {
let disabled = Focusable::parse("false");
assert_eq!(disabled, Ok(Focusable::Disabled));
}

#[test]
fn parse_focusable_fallback() {
let fallback = Focusable::parse("hello!!");
assert_eq!(fallback, Ok(Focusable::Unknown));
}