From 508f6d9bf535dcd284792778d3ef4bf67c06c50c Mon Sep 17 00:00:00 2001 From: Nolan Darilek Date: Sat, 12 Jun 2021 07:54:01 -0500 Subject: [PATCH] Additional accessibility support (#412) * Expose getter for currently focused widget. * Remove a level of indirection, exposing the widget event on the top level. * Align widget descriptions more closely with common screen reader conventions. Note that this work isn't complete--I'll correct more cases as I add more widgets and become familiar with their structures. * Add support for click and double-click events. * Add `ValueChanged` events, with initial support for text. * Add support for reporting cursor selection changes. * Track enabled/disabled status. * Move `prev_text` off of the widget struct. * Get rid of `has_widget_info` and push events directly where it makes sense. * Fix typo. * s/text_value/current_text_value/ * Use a `RangeInclusive` for text selection. * Invert parameters. * Various fixes. * Only dispatch `SelectionChanged` if the selection actually changes. * Fix missing focus events. * If values for `current_text` and `prev_text` are unchanged, filter out the previous value. * No need to pass in `&mut prev_text` everywhere * Appease Clippy. * Mask password fields in generated events. Co-authored-by: Emil Ernerfeldt --- egui/src/data/output.rs | 117 ++++++++++++++++++++++++++-------- egui/src/memory.rs | 5 ++ egui/src/response.rs | 17 +++-- egui/src/widgets/text_edit.rs | 50 ++++++++++++++- 4 files changed, 154 insertions(+), 35 deletions(-) diff --git a/egui/src/data/output.rs b/egui/src/data/output.rs index ee30f07f..ff145b52 100644 --- a/egui/src/data/output.rs +++ b/egui/src/data/output.rs @@ -42,7 +42,11 @@ impl Output { // only describe last event: if let Some(event) = self.events.iter().rev().next() { match event { - OutputEvent::WidgetEvent(WidgetEvent::Focus, widget_info) => { + OutputEvent::Clicked(widget_info) + | OutputEvent::DoubleClicked(widget_info) + | OutputEvent::FocusGained(widget_info) + | OutputEvent::TextSelectionChanged(widget_info) + | OutputEvent::ValueChanged(widget_info) => { return widget_info.description(); } } @@ -206,60 +210,77 @@ impl Default for CursorIcon { /// In particular, these events may be useful for accessability, i.e. for screen readers. #[derive(Clone, PartialEq)] pub enum OutputEvent { + // A widget was clicked. + Clicked(WidgetInfo), + // A widget was double-clicked. + DoubleClicked(WidgetInfo), /// A widget gained keyboard focus (by tab key). - WidgetEvent(WidgetEvent, WidgetInfo), + FocusGained(WidgetInfo), + // Text selection was updated. + TextSelectionChanged(WidgetInfo), + // A widget's value changed. + ValueChanged(WidgetInfo), } impl std::fmt::Debug for OutputEvent { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - Self::WidgetEvent(we, wi) => write!(f, "{:?}: {:?}", we, wi), + Self::Clicked(wi) => write!(f, "Clicked({:?})", wi), + Self::DoubleClicked(wi) => write!(f, "DoubleClicked({:?})", wi), + Self::FocusGained(wi) => write!(f, "FocusGained({:?})", wi), + Self::TextSelectionChanged(wi) => write!(f, "TextSelectionChanged({:?})", wi), + Self::ValueChanged(wi) => write!(f, "ValueChanged({:?})", wi), } } } -#[derive(Clone, Copy, Debug, PartialEq)] -pub enum WidgetEvent { - /// Keyboard focused moved onto the widget. - Focus, - // /// Started hovering a new widget. - // Hover, // TODO: cursor hovered events -} - /// Describes a widget such as a [`crate::Button`] or a [`crate::TextEdit`]. #[derive(Clone, PartialEq)] pub struct WidgetInfo { /// The type of widget this is. pub typ: WidgetType, + // Whether the widget is enabled. + pub enabled: bool, /// The text on labels, buttons, checkboxes etc. pub label: Option, /// The contents of some editable text (for `TextEdit` fields). - pub edit_text: Option, + pub current_text_value: Option, + // The previous text value. + pub prev_text_value: Option, /// The current value of checkboxes and radio buttons. pub selected: Option, /// The current value of sliders etc. pub value: Option, + // Selected range of characters in [`Self::current_text_value`]. + pub text_selection: Option>, } impl std::fmt::Debug for WidgetInfo { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let Self { typ, + enabled, label, - edit_text, + current_text_value: text_value, + prev_text_value, selected, value, + text_selection, } = self; let mut s = f.debug_struct("WidgetInfo"); s.field("typ", typ); + s.field("enabled", enabled); if let Some(label) = label { s.field("label", label); } - if let Some(edit_text) = edit_text { - s.field("edit_text", edit_text); + if let Some(text_value) = text_value { + s.field("text_value", text_value); + } + if let Some(prev_text_value) = prev_text_value { + s.field("prev_text_value", prev_text_value); } if let Some(selected) = selected { s.field("selected", selected); @@ -267,6 +288,9 @@ impl std::fmt::Debug for WidgetInfo { if let Some(value) = value { s.field("value", value); } + if let Some(text_selection) = text_selection { + s.field("text_selection", text_selection); + } s.finish() } @@ -276,10 +300,13 @@ impl WidgetInfo { pub fn new(typ: WidgetType) -> Self { Self { typ, + enabled: true, label: None, - edit_text: None, + current_text_value: None, + prev_text_value: None, selected: None, value: None, + text_selection: None, } } @@ -319,9 +346,29 @@ impl WidgetInfo { } #[allow(clippy::needless_pass_by_value)] - pub fn text_edit(edit_text: impl ToString) -> Self { + pub fn text_edit(prev_text_value: impl ToString, text_value: impl ToString) -> Self { + let text_value = text_value.to_string(); + let prev_text_value = prev_text_value.to_string(); + let prev_text_value = if text_value == prev_text_value { + None + } else { + Some(prev_text_value) + }; Self { - edit_text: Some(edit_text.to_string()), + current_text_value: Some(text_value), + prev_text_value, + ..Self::new(WidgetType::TextEdit) + } + } + + #[allow(clippy::needless_pass_by_value)] + pub fn text_selection_changed( + text_selection: std::ops::RangeInclusive, + current_text_value: impl ToString, + ) -> Self { + Self { + text_selection: Some(text_selection), + current_text_value: Some(current_text_value.to_string()), ..Self::new(WidgetType::TextEdit) } } @@ -330,14 +377,17 @@ impl WidgetInfo { pub fn description(&self) -> String { let Self { typ, + enabled, label, - edit_text, + current_text_value: text_value, + prev_text_value: _, selected, value, + text_selection: _, } = self; // TODO: localization - let widget_name = match typ { + let widget_type = match typ { WidgetType::Hyperlink => "link", WidgetType::TextEdit => "text edit", WidgetType::Button => "button", @@ -353,25 +403,33 @@ impl WidgetInfo { WidgetType::Label | WidgetType::Other => "", }; - let mut description = widget_name.to_owned(); + let mut description = widget_type.to_owned(); if let Some(selected) = selected { if *typ == WidgetType::Checkbox { - description += " "; - description += if *selected { "checked" } else { "unchecked" }; + let state = if *selected { "checked" } else { "unchecked" }; + description = format!("{} {}", state, description); } else { description += if *selected { "selected" } else { "" }; }; } if let Some(label) = label { - description += " "; - description += label; + description = format!("{}: {}", label, description); } - if let Some(edit_text) = edit_text { - description += " "; - description += edit_text; + if typ == &WidgetType::TextEdit { + let text; + if let Some(text_value) = text_value { + if text_value.is_empty() { + text = "blank".into(); + } else { + text = text_value.to_string(); + } + } else { + text = "blank".into(); + } + description = format!("{}: {}", text, description); } if let Some(value) = value { @@ -379,6 +437,9 @@ impl WidgetInfo { description += &value.to_string(); } + if !enabled { + description += ": disabled"; + } description.trim().to_owned() } } diff --git a/egui/src/memory.rs b/egui/src/memory.rs index d9cb1535..fc6efabc 100644 --- a/egui/src/memory.rs +++ b/egui/src/memory.rs @@ -315,6 +315,11 @@ impl Memory { self.interaction.focus.id == Some(id) } + /// Which widget has keyboard focus? + pub fn focus(&self) -> Option { + self.interaction.focus.id + } + pub(crate) fn lock_focus(&mut self, id: Id, lock_focus: bool) { if self.had_focus_last_frame(id) && self.has_focus(id) { self.interaction.focus.is_focus_locked = lock_focus; diff --git a/egui/src/response.rs b/egui/src/response.rs index 71b2ddd5..0c6ebf6e 100644 --- a/egui/src/response.rs +++ b/egui/src/response.rs @@ -428,10 +428,19 @@ impl Response { /// /// Call after interacting and potential calls to [`Self::mark_changed`]. pub fn widget_info(&self, make_info: impl Fn() -> crate::WidgetInfo) { - if self.gained_focus() { - use crate::output::{OutputEvent, WidgetEvent}; - let widget_info = make_info(); - let event = OutputEvent::WidgetEvent(WidgetEvent::Focus, widget_info); + use crate::output::OutputEvent; + let event = if self.clicked() { + Some(OutputEvent::Clicked(make_info())) + } else if self.double_clicked() { + Some(OutputEvent::DoubleClicked(make_info())) + } else if self.gained_focus() { + Some(OutputEvent::FocusGained(make_info())) + } else if self.changed { + Some(OutputEvent::ValueChanged(make_info())) + } else { + None + }; + if let Some(event) = event { self.ctx.output().events.push(event); } } diff --git a/egui/src/widgets/text_edit.rs b/egui/src/widgets/text_edit.rs index 10470000..3e9cd2be 100644 --- a/egui/src/widgets/text_edit.rs +++ b/egui/src/widgets/text_edit.rs @@ -1,4 +1,4 @@ -use crate::{util::undoer::Undoer, *}; +use crate::{output::OutputEvent, util::undoer::Undoer, *}; use epaint::{text::cursor::*, *}; use std::ops::Range; @@ -393,6 +393,7 @@ impl<'t, S: TextBuffer> TextEdit<'t, S> { lock_focus, } = self; + let prev_text = text.clone(); let text_style = text_style .or(ui.style().override_text_style) .unwrap_or_else(|| ui.style().body_text_style); @@ -496,6 +497,8 @@ impl<'t, S: TextBuffer> TextEdit<'t, S> { ui.output().cursor_icon = CursorIcon::Text; } + let mut text_cursor = None; + let prev_text_cursor = state.cursorp; if ui.memory().has_focus(id) && enabled { ui.memory().lock_focus(id, lock_focus); @@ -554,7 +557,6 @@ impl<'t, S: TextBuffer> TextEdit<'t, S> { && text_to_insert != "\r" { let mut ccursor = delete_selected(text, &cursorp); - insert_text(&mut ccursor, text, text_to_insert); Some(CCursorPair::one(ccursor)) } else { @@ -667,6 +669,7 @@ impl<'t, S: TextBuffer> TextEdit<'t, S> { } } state.cursorp = Some(cursorp); + text_cursor = Some(cursorp); state .undoer @@ -709,7 +712,48 @@ impl<'t, S: TextBuffer> TextEdit<'t, S> { ui.memory().id_data.insert(id, state); - response.widget_info(|| WidgetInfo::text_edit(&*text)); + let selection_changed = if let (Some(text_cursor), Some(prev_text_cursor)) = + (text_cursor, prev_text_cursor) + { + text_cursor.primary.ccursor.index != prev_text_cursor.primary.ccursor.index + || text_cursor.secondary.ccursor.index != prev_text_cursor.secondary.ccursor.index + } else { + false + }; + + let masked = if self.password { + let prev_text_len = prev_text.to_string().len(); + let text_len = text.to_string().len(); + Some(("*".repeat(prev_text_len), "*".repeat(text_len))) + } else { + None + }; + + if response.changed { + if let Some((prev_text, text)) = masked { + response.widget_info(|| WidgetInfo::text_edit(&prev_text, &text)); + } else { + response.widget_info(|| WidgetInfo::text_edit(&prev_text, &text)); + } + } else if selection_changed { + let text_cursor = text_cursor.unwrap(); + let char_range = + text_cursor.primary.ccursor.index..=text_cursor.secondary.ccursor.index; + let info = if let Some((_, text)) = masked { + WidgetInfo::text_selection_changed(char_range, text) + } else { + WidgetInfo::text_selection_changed(char_range, &*text) + }; + response + .ctx + .output() + .events + .push(OutputEvent::TextSelectionChanged(info)); + } else if let Some((prev_text, text)) = masked { + response.widget_info(|| WidgetInfo::text_edit(&prev_text, &text)); + } else { + response.widget_info(|| WidgetInfo::text_edit(&prev_text, &text)); + } response } }