From 09d9ad555d1be8e129dd08bbbb78a0b9c06c335a Mon Sep 17 00:00:00 2001 From: Olivier FAURE Date: Mon, 16 Sep 2024 17:31:37 +0200 Subject: [PATCH] Make `Textbox` focusable and trigger redraw in response to `request_layout` (#537) Fixes #301. --- masonry/examples/two_textboxes.rs | 43 ++++++++++++++++++++++++++++ masonry/src/passes/accessibility.rs | 28 +++--------------- masonry/src/passes/event.rs | 4 +-- masonry/src/render_root.rs | 6 +++- masonry/src/text/selection.rs | 9 ++++++ masonry/src/widget/label.rs | 1 - masonry/src/widget/prose.rs | 3 -- masonry/src/widget/textbox.rs | 2 ++ masonry/src/widget/variable_label.rs | 6 +--- 9 files changed, 66 insertions(+), 36 deletions(-) create mode 100644 masonry/examples/two_textboxes.rs diff --git a/masonry/examples/two_textboxes.rs b/masonry/examples/two_textboxes.rs new file mode 100644 index 00000000..e8f8b75a --- /dev/null +++ b/masonry/examples/two_textboxes.rs @@ -0,0 +1,43 @@ +// Copyright 2024 the Xilem Authors +// SPDX-License-Identifier: Apache-2.0 + +//! This is a very small example to demonstrate tab focus. +//! It will probably be removed in the future. + +// On Windows platform, don't show a console when opening the app. +#![windows_subsystem = "windows"] + +use masonry::app_driver::{AppDriver, DriverCtx}; +use masonry::dpi::LogicalSize; +use masonry::widget::{Flex, RootWidget, Textbox}; +use masonry::{Action, WidgetId}; +use winit::window::Window; + +const VERTICAL_WIDGET_SPACING: f64 = 20.0; + +struct Driver; + +impl AppDriver for Driver { + fn on_action(&mut self, _ctx: &mut DriverCtx<'_>, _widget_id: WidgetId, _action: Action) {} +} + +pub fn main() { + let main_widget = Flex::column() + .with_child(Textbox::new("")) + .with_child(Textbox::new("")) + .with_spacer(VERTICAL_WIDGET_SPACING); + + let window_size = LogicalSize::new(400.0, 400.0); + let window_attributes = Window::default_attributes() + .with_title("Two textboxes") + .with_resizable(true) + .with_min_inner_size(window_size); + + masonry::event_loop_runner::run( + masonry::event_loop_runner::EventLoop::with_user_event(), + window_attributes, + RootWidget::new(main_widget), + Driver, + ) + .unwrap(); +} diff --git a/masonry/src/passes/accessibility.rs b/masonry/src/passes/accessibility.rs index e80505d3..7b407651 100644 --- a/masonry/src/passes/accessibility.rs +++ b/masonry/src/passes/accessibility.rs @@ -2,7 +2,6 @@ // SPDX-License-Identifier: Apache-2.0 use crate::render_root::RenderRoot; -use crate::tree_arena::ArenaMutChildren; use accesskit::{NodeBuilder, NodeId, TreeUpdate}; use tracing::debug; use tracing::info_span; @@ -35,12 +34,7 @@ fn build_accessibility_tree( widget.item.short_type_name(), id.to_raw(), ); - let current_node = build_access_node( - widget.item, - state.item, - state.children.reborrow_mut(), - scale_factor, - ); + let current_node = build_access_node(widget.item, state.item, scale_factor); let mut ctx = AccessCtx { global_state, @@ -74,8 +68,8 @@ fn build_accessibility_tree( widget.reborrow_mut(), state.children, |widget, mut state| { - // TODO - We skip updating stashed items. - // This may have knock-on effects we'd need to document. + // TODO - We don't skip updating stashed items because doing so + // is error-prone. We may want to revisit that decision. if state.item.is_stashed { return; } @@ -92,29 +86,15 @@ fn build_accessibility_tree( ); } -fn build_access_node( - widget: &dyn Widget, - state: &WidgetState, - state_children: ArenaMutChildren<'_, WidgetState>, - scale_factor: f64, -) -> NodeBuilder { +fn build_access_node(widget: &dyn Widget, state: &WidgetState, scale_factor: f64) -> NodeBuilder { let mut node = NodeBuilder::new(widget.accessibility_role()); node.set_bounds(to_accesskit_rect(state.window_layout_rect(), scale_factor)); - // TODO - We skip listing stashed items. - // This may have knock-on effects we'd need to document. node.set_children( widget .children_ids() .iter() .copied() - .filter(|id| { - !state_children - .get_child(id.to_raw()) - .unwrap() - .item - .is_stashed - }) .map(|id| id.into()) .collect::>(), ); diff --git a/masonry/src/passes/event.rs b/masonry/src/passes/event.rs index ae0e2dd0..38d76dbd 100644 --- a/masonry/src/passes/event.rs +++ b/masonry/src/passes/event.rs @@ -154,9 +154,9 @@ pub(crate) fn root_on_text_event( // Handle Tab focus if let TextEvent::KeyboardKey(key, mods) = event { - if handled == Handled::No - && key.physical_key == PhysicalKey::Code(KeyCode::Tab) + if key.physical_key == PhysicalKey::Code(KeyCode::Tab) && key.state == ElementState::Pressed + && handled == Handled::No { if !mods.shift_key() { root.state.next_focused_widget = root.widget_from_focus_chain(true); diff --git a/masonry/src/render_root.rs b/masonry/src/render_root.rs index 13faf7b2..cdee2fc0 100644 --- a/masonry/src/render_root.rs +++ b/masonry/src/render_root.rs @@ -527,7 +527,11 @@ impl RenderRoot { // We request a redraw if either the render tree or the accessibility // tree needs to be rebuilt. Usually both happen at the same time. // A redraw will trigger a rebuild of the accessibility tree. - if self.root_state().needs_paint || self.root_state().needs_accessibility { + // TODO - We assume that a relayout will trigger a repaint + if self.root_state().needs_paint + || self.root_state().needs_accessibility + || self.root_state().needs_layout + { self.state .signal_queue .push_back(RenderRootSignal::RequestRedraw); diff --git a/masonry/src/text/selection.rs b/masonry/src/text/selection.rs index 66864e7c..a449fe2e 100644 --- a/masonry/src/text/selection.rs +++ b/masonry/src/text/selection.rs @@ -204,6 +204,15 @@ impl TextWithSelection { } } + /// Call when this widget becomes focused + pub fn focus_gained(&mut self) { + if self.selection.is_none() { + // TODO - We need to have some "memory" of the text selected instead. + self.selection = Some(Selection::caret(self.text().len(), Affinity::Downstream)); + } + self.needs_selection_update = true; + } + /// Call when another widget becomes focused pub fn focus_lost(&mut self) { self.selection = None; diff --git a/masonry/src/widget/label.rs b/masonry/src/widget/label.rs index 211bdaca..f0c5d577 100644 --- a/masonry/src/widget/label.rs +++ b/masonry/src/widget/label.rs @@ -116,7 +116,6 @@ impl WidgetMut<'_, Label> { let ret = f(&mut self.widget.text_layout); if self.widget.text_layout.needs_rebuild() { self.ctx.request_layout(); - self.ctx.request_paint(); } ret } diff --git a/masonry/src/widget/prose.rs b/masonry/src/widget/prose.rs index 992f82bd..732a34bf 100644 --- a/masonry/src/widget/prose.rs +++ b/masonry/src/widget/prose.rs @@ -152,7 +152,6 @@ impl Widget for Prose { let made_change = self.text_layout.pointer_down(inner_origin, state, *button); if made_change { ctx.request_layout(); - ctx.request_paint(); ctx.request_focus(); ctx.capture_pointer(); } @@ -167,7 +166,6 @@ impl Widget for Prose { { // We might have changed text colours, so we need to re-request a layout ctx.request_layout(); - ctx.request_paint(); } } } @@ -188,7 +186,6 @@ impl Widget for Prose { ctx.set_handled(); // TODO: only some handlers need this repaint ctx.request_layout(); - ctx.request_paint(); } } diff --git a/masonry/src/widget/textbox.rs b/masonry/src/widget/textbox.rs index b0681d6a..65022b2c 100644 --- a/masonry/src/widget/textbox.rs +++ b/masonry/src/widget/textbox.rs @@ -235,7 +235,9 @@ impl Widget for Textbox { // TODO: Stop focusing on any links } StatusChange::FocusChanged(true) => { + self.editor.focus_gained(); // TODO: Focus on first link + ctx.request_layout(); } _ => {} } diff --git a/masonry/src/widget/variable_label.rs b/masonry/src/widget/variable_label.rs index c88696de..20a8569f 100644 --- a/masonry/src/widget/variable_label.rs +++ b/masonry/src/widget/variable_label.rs @@ -228,7 +228,6 @@ impl WidgetMut<'_, VariableLabel> { let ret = f(&mut self.widget.text_layout); if self.widget.text_layout.needs_rebuild() { self.ctx.request_layout(); - self.ctx.request_paint(); } ret } @@ -247,7 +246,6 @@ impl WidgetMut<'_, VariableLabel> { if !self.ctx.is_disabled() { self.widget.text_layout.invalidate(); self.ctx.request_layout(); - self.ctx.request_paint(); } } /// Set the font size for this text. @@ -269,13 +267,12 @@ impl WidgetMut<'_, VariableLabel> { /// How to handle overflowing lines. pub fn set_line_break_mode(&mut self, line_break_mode: LineBreaking) { self.widget.line_break_mode = line_break_mode; - self.ctx.request_paint(); + self.ctx.request_layout(); } /// Set the weight which this font will target. pub fn set_target_weight(&mut self, target: f32, over_millis: f32) { self.widget.weight.move_to(target, over_millis); self.ctx.request_layout(); - self.ctx.request_paint(); self.ctx.request_anim_frame(); } } @@ -343,7 +340,6 @@ impl Widget for VariableLabel { ctx.request_anim_frame(); } ctx.request_layout(); - ctx.request_paint(); } _ => {} }