Skip to content

ui-egui: yield canvas focus on initial dialog frame and preserve IME after shortcuts - #6626

Open
RilakKuma2 wants to merge 1 commit into
storytold:mainfrom
RilakKuma2:fix/macos-korean-ime
Open

RilakKuma2 wants to merge 1 commit into
storytold:mainfrom
RilakKuma2:fix/macos-korean-ime

Conversation

@RilakKuma2

@RilakKuma2 RilakKuma2 commented Nov 14, 2026 •

Copy link
Copy Markdown

Fixes two focus/input handoff issues affecting IME typing when opening dialogs or panes:

  1. swallow_shortcut_text (dialogs/find.rs) cleared its swallow guard on plain Event::Key presses, but not on Event::Ime(ImeEvent::Preedit | ImeEvent::Commit), allowing the first IME event after a shortcut to remain guarded.
  2. On the opening frame of a focus-taking modeless dialog (Dialog::Find { focus: true, .. }) or when an earlier pane widget already took focus, prepare_layout and update_canvas_focus (canvas.rs) could still consume keyboard/IME events or re-claim focus for the document canvas before the dialog drew.

Paired with macOS Korean IME fixes in storytold/winit (storytold/winit#38).

Summary by CodeRabbit

  • Bug Fixes
    • Improved keyboard focus handling when dialogs are open, including the Find and Go To tabs.
    • Prevented canvas keyboard shortcuts from firing when a dialog has focus.
    • Improved handling of text input method events in the Find dialog.

@coderabbitai

coderabbitai Bot commented Nov 14, 2026 •

Copy link
Copy Markdown

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

Dialog focus state now affects canvas focus and keyboard-event handling. Find-dialog handling clears pending focus on the Go To tab. Shortcut-text swallowing now handles IME preedit and commit events.

Changes: Dialog Keyboard Focus

Layer / File(s) Summary
Dialog focus arbitration and canvas routing
crates/ui-egui/src/dialogs.rs, crates/ui-egui/src/app.rs, crates/ui-egui/src/canvas.rs, crates/ui-egui/src/dialogs/dispatch.rs
Dialog focus classification determines canvas focus requests, focus-lock, and keyboard-event dispatch. The Go To tab clears the pending focus flag.
IME shortcut-text handling
crates/ui-egui/src/dialogs/find.rs
IME preedit and commit events clear the shortcut-text swallowing state and return false.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: ratkor, dylanmtaylor, aiongg

Merge Risk: Minimal · up to bd1f9

This change routes IME and keyboard input correctly between the document canvas and focus-taking dialogs. No actionable merge-blocking risk was found; the only remaining edge case is a brief, narrow focus glitch.

Pre-merge checks: Passed 4

Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title names two changes: canvas focus is yielded on the initial dialog frame, and IME input is preserved after shortcuts. The changeset matches both. Canvas focus is suppressed for dialogs that ta…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title names two changes: canvas focus is yielded on the initial dialog frame, and IME input is preserved after shortcuts. The changeset matches both. Canvas focus is suppressed for dialogs that take initial focus (dialogs.rs, app.rs, canvas.rs). swallow_shortcut_text in find.rs now returns false for IME preedit and commit events. The title covers the main changes, though it omits the dispatch.rs focus clearing for the Go To tab.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between 60415e0 and bd1f946.

Files selected for processing: 5

  • crates/ui-egui/src/app.rs
  • crates/ui-egui/src/canvas.rs
  • crates/ui-egui/src/dialogs.rs
  • crates/ui-egui/src/dialogs/dispatch.rs
  • crates/ui-egui/src/dialogs/find.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant