Skip to content

Implement delayed key event synthesis support for framework - #59358

Merged
fluttergithubbot merged 5 commits into
flutter:masterfrom
gspencergoog:framework_synchronous_keys
Jun 25, 2020
Merged

fluttergithubbot merged 5 commits into
flutter:masterfrom
gspencergoog:framework_synchronous_keys

Conversation

@gspencergoog

@gspencergoog gspencergoog commented Jun 12, 2020 •

Copy link
Copy Markdown
Contributor

Description

This implements the design in https://flutter.dev/go/handling-synchronous-keyboard-events for Android.

I started with Android, but this will be used for all platforms as we add them.

The related engine PR is: flutter-team-archive/engine#19024

Related Issues

Tests

  • Added a test to make sure that generated key events send back messages indicating that they handled the event.

Breaking Change

  • No, no existing tests failed, so this is not a breaking change.

@fluttergithubbot fluttergithubbot added the framework flutter/packages/flutter repository. See also f: labels. label Jun 12, 2020
@gspencergoog gspencergoog changed the title [WIP] Framework synchronous keys [WIP] Implement synchronous key event support for Android Jun 12, 2020
@gspencergoog
gspencergoog requested a review from goderbauer June 13, 2020 00:38
@gspencergoog gspencergoog changed the title [WIP] Implement synchronous key event support for Android Implement synchronous key event support for Android Jun 13, 2020
@fluttergithubbot fluttergithubbot added the a: tests "flutter test", flutter_test, or one of our tests label Jun 13, 2020
@gspencergoog
gspencergoog marked this pull request as ready for review June 13, 2020 00:38
@gspencergoog
gspencergoog removed the request for review from goderbauer June 13, 2020 00:39
@pingbird

Copy link
Copy Markdown
Member

What would happen if multiple Flutter Views are on top of each other?

I assume this would lead to n^2 key events in rapid succession, but its not clear if there would be any notable side effects of that.

@gspencergoog
gspencergoog requested review from goderbauer and jason-simmons and removed request for jason-simmons June 17, 2020 20:51

@goderbauer goderbauer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, just some doc nits.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: maybe mentioned what "handled" (or returning true) means, e.g. that the event will stop propagating and nobody else will see them?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we assert here that if the keyEventHandler was provided it returned true/false (and not null)? Assuming that returning null from keyEventHandler is illegal.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If null is illegal, let's document that on RawKeyEventHandler as well.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

maybe link to [addListener] as well from here and say how that's different then receiving the key events from here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

@gspencergoog
gspencergoog force-pushed the framework_synchronous_keys branch 2 times, most recently from f725ee0 to 0719bce Compare June 22, 2020 23:04
@flutter-github-sync

Copy link
Copy Markdown

Google testing passed!

@gspencergoog gspencergoog changed the title Implement synchronous key event support for Android Implement delayed key event synthesis support for Android Jun 25, 2020
@gspencergoog
gspencergoog force-pushed the framework_synchronous_keys branch from 0719bce to 2571236 Compare June 25, 2020 01:05
@flutter-github-sync

Copy link
Copy Markdown

Started Google testing for this PR

@flutter-github-sync

Copy link
Copy Markdown

Google testing passed!

@gspencergoog gspencergoog changed the title Implement delayed key event synthesis support for Android Implement delayed key event synthesis support for framework Jun 25, 2020
gspencergoog added a commit to flutter-team-archive/engine that referenced this pull request Jul 17, 2020
)

This implements the design in flutter.dev/go/handling-synchronous-keyboard-events for Android.

I started with Android, but this will be used for all platforms as we add them.

The related framework PR is: flutter/flutter#59358 (which has already landed)
mingwandroid pushed a commit to mingwandroid/flutter that referenced this pull request Sep 6, 2020
@gspencergoog
gspencergoog deleted the framework_synchronous_keys branch November 2, 2020 19:47
zhongdaiqi pushed a commit to zhongdaiqi/flutter_flutter that referenced this pull request Oct 9, 2026
…tter-team-archive/engine#19024)

This implements the design in flutter.dev/go/handling-synchronous-keyboard-events for Android.

I started with Android, but this will be used for all platforms as we add them.

The related framework PR is: flutter/flutter#59358 (which has already landed)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: tests "flutter test", flutter_test, or one of our tests framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants