fix: spacebar not working when editing text fields in grid view#8598
Open
yetval wants to merge 3 commits intoAppFlowy-IO:mainfrom
Open
fix: spacebar not working when editing text fields in grid view#8598yetval wants to merge 3 commits intoAppFlowy-IO:mainfrom
yetval wants to merge 3 commits intoAppFlowy-IO:mainfrom
Conversation
…text input; return skipRemainingHandlers on focusNode to stop propagation without suppressing WM_CHAR
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds a key event handler to text grid cells so spacebar presses are skipped for ancestor shortcut handling but still reach the text input system, fixing spacebar input on Windows when editing text fields in the database grid view. Sequence diagram for handling spacebar key events in grid text cellssequenceDiagram
actor User
participant GridTextCell as GridTextCell
participant FocusNode as FocusNode
participant Shortcuts as Shortcuts
participant PlatformWindow as PlatformWindow
participant TextInputSystem as TextInputSystem
User->>GridTextCell: press spacebar
GridTextCell->>FocusNode: onKeyEvent(KeyDownEvent space)
FocusNode-->>GridTextCell: KeyEventResult.skipRemainingHandlers
Note right of FocusNode: Ancestor Shortcuts does not receive the space key event
GridTextCell->>PlatformWindow: key down event (space)
PlatformWindow->>PlatformWindow: generate WM_CHAR for space
PlatformWindow->>TextInputSystem: WM_CHAR ' '
TextInputSystem-->>GridTextCell: insert space into text field
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
onKeyEventhandler only checks forKeyDownEvent, so holding the spacebar for key repeat may still be intercepted by ancestor shortcuts; consider also handlingKeyRepeatEventforLogicalKeyboardKey.spaceto keep the behavior consistent. - Since the root cause is specific to Windows (WM_CHAR suppression), consider guarding this logic with a platform check so that spacebar shortcuts on other platforms are not unintentionally masked if they rely on ancestor
Shortcuts.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `onKeyEvent` handler only checks for `KeyDownEvent`, so holding the spacebar for key repeat may still be intercepted by ancestor shortcuts; consider also handling `KeyRepeatEvent` for `LogicalKeyboardKey.space` to keep the behavior consistent.
- Since the root cause is specific to Windows (WM_CHAR suppression), consider guarding this logic with a platform check so that spacebar shortcuts on other platforms are not unintentionally masked if they rely on ancestor `Shortcuts`.
## Individual Comments
### Comment 1
<location path="frontend/appflowy_flutter/lib/plugins/database/widgets/cell/editable_cell_skeleton/text.dart" line_range="70-71" />
<code_context>
_textEditingController =
TextEditingController(text: cellBloc.state.content);
+ focusNode.onKeyEvent = (node, event) {
+ if (event is KeyDownEvent &&
+ event.logicalKey == LogicalKeyboardKey.space) {
+ return KeyEventResult.skipRemainingHandlers;
+ }
</code_context>
<issue_to_address>
**question (bug_risk):** Consider whether modifier combinations (e.g., Shift+Space) should also be intercepted, and if not, explicitly filter only unmodified space presses.
Since every LogicalKeyboardKey.space press is skipped, modifier chords like Shift+Space or Ctrl+Space will also be swallowed. If only the plain spacebar should be intercepted, guard against active modifiers (e.g., consult HardwareKeyboard.instance.logicalKeysPressed or the event’s modifier helpers) so shortcut bindings keep working.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
frontend/appflowy_flutter/lib/plugins/database/widgets/cell/editable_cell_skeleton/text.dart
Outdated
Show resolved
Hide resolved
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes the spacebar key not working when editing text fields in database grid view on Windows.
Root cause: When a key event is dispatched, Flutter's ancestor
Shortcutswidget maps the bare space key toActivateIntent. When that intent is handled, the engine receiveshandled: trueand suppresses theWM_CHARmessage — so the character never reaches the text input system.Fix: Set
onKeyEventon the text cell'sfocusNodeto returnKeyEventResult.skipRemainingHandlersfor the space key. This stops ancestor Shortcuts from seeing the event without marking it as "handled", so the platform still generatesWM_CHARand the space is inserted normally.Closes #8572
Checklist
Summary by Sourcery
Bug Fixes: