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.

Sign up for GitHub

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

Jump to bottom

Run the focus pass in post-processing for all events #604

Merged
merged 3 commits into from
Sep 23, 2024
Merged

Run the focus pass in post-processing for all events #604

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7a7d7c3
Run the focus pass in response to pointer and accessibility events
mwcampbell Sep 19, 2024
a27ddbe
Move the call into post_event_processing.
mwcampbell Sep 20, 2024
2213134
Run update_focus pass in two places.
PoignardAzur Sep 23, 2024
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
3 changes: 2 additions & 1 deletion masonry/src/render_root.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,6 @@ impl RenderRoot {
let mut dummy_state = WidgetState::synthetic(self.root.id(), self.get_kurbo_size());

let handled = root_on_text_event(self, &mut dummy_state, &event);
run_update_focus_pass(self, &mut dummy_state);

self.post_event_processing(&mut dummy_state);
self.get_root_widget().debug_validate(false);
Expand Down Expand Up @@ -484,6 +483,8 @@ impl RenderRoot {

// --- MARK: POST-EVENT ---
fn post_event_processing(&mut self, widget_state: &mut WidgetState) {
run_update_focus_pass(self, widget_state);
Copy link
Member

Choose a reason for hiding this comment

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

The conditions under which run_update_focus_pass needs to happen are a bit unclear, and I suspect that logic should live inside the pass itself.

I think it only does something if:

  1. Reparenting happens (impossible)
  2. The currently focused node is now disabled
  3. The next focused node is different to the current focused node.

I think this should move to after the update_focus_chain pass as a first pass, with a comment like:

// TODO: Only run this pass if needed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I put it at the top because root_on_text_event used to make that call before post_event_processing.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, indeed. This is why I've asked Olivier to weigh in.

I would approve optimistically if this were moved to be after update_focus_chain, presuming that testing still works.


// If children are changed during the handling of an event,
// we need to send RouteWidgetAdded now, so that they are ready for update/layout.
if widget_state.children_changed {
Expand Down