Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

bleroux
Copy link
Contributor

@bleroux bleroux commented May 26, 2023

Description

This PR updates the Linux engine in order to answer to keyboard pressed state queries from the framework (as implemented in flutter/flutter#122885).

Related Issue

Linux engine implementation for flutter/flutter#87391

Tests

Adds 2 tests.

@bleroux bleroux force-pushed the linux_return_keyboard_pressed_state branch 2 times, most recently from 05ede6c to 90c2bce Compare May 26, 2023 14:35
@bleroux bleroux force-pushed the linux_return_keyboard_pressed_state branch from 90c2bce to 84397b1 Compare May 26, 2023 14:50
@bleroux bleroux requested a review from dkwingsmt May 26, 2023 18:57
* fl_key_embedder_responder_get_pressed_state:
* @responder: the #FlKeyEmbedderResponder self.
*
* Returns the keyboard pressed state.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Returns the keyboard pressed state.
* Returns the keyboard pressed state. The hash table contains one entry per pressed keys, mapping from the logical key to the physical key.

* fl_keyboard_manager_get_pressed_state:
* @manager: the #FlKeyboardManager self.
*
* Returns the keyboard pressed state.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Returns the keyboard pressed state.
* Returns the keyboard pressed state. The hash table contains one entry per pressed keys, mapping from the logical key to the physical key.

/**
* FlKeyboardPlugin:
*
* #FlKeyboardPlugin is a keyboard channel that implements the shell side
Copy link
Contributor

Choose a reason for hiding this comment

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

Since it's just a keyboard channel, should we call it FlKeyboardChannel? Anyway it's not very much a plugin and the name is a bit confusing.

Also I haven't thought it through but will it be simpler to merge this class with FlKeyboardManager, since it only contains one method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also I haven't thought it through but will it be simpler to merge this class with FlKeyboardManager, since it only contains one method?

Yes, it will be probably simpler, I will try this next week.

@bleroux bleroux force-pushed the linux_return_keyboard_pressed_state branch from 52a9356 to c6f9c0c Compare May 27, 2023 08:44
@bleroux bleroux requested a review from dkwingsmt May 29, 2023 14:15
@bleroux bleroux force-pushed the linux_return_keyboard_pressed_state branch 3 times, most recently from b71f2bb to bc09590 Compare May 30, 2023 05:51
@bleroux bleroux force-pushed the linux_return_keyboard_pressed_state branch from bc09590 to 81f420d Compare May 30, 2023 09:22
@cbracken
Copy link
Member

cbracken commented Jun 8, 2023

@dkwingsmt looks like this is ready for re-review.

Copy link
Contributor

@dkwingsmt dkwingsmt left a comment

Choose a reason for hiding this comment

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

LGTM

@bleroux bleroux added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 9, 2023
@auto-submit auto-submit bot merged commit f7adb9f into flutter:main Jun 9, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 9, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jun 9, 2023
…128573)

flutter/engine@cb93477...93afba9

2023-06-09 [email protected] Roll ANGLE from 8a62b4c44fc9 to e9493542672c (1 revision) (flutter/engine#42695)
2023-06-09 [email protected] [Linux] Return keyboard pressed state (flutter/engine#42346)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
auto-submit bot pushed a commit that referenced this pull request Jun 21, 2023
## Description

This PR updates the Android engine in order to answer to keyboard pressed state queries from the framework (as implemented in flutter/flutter#122885).

This is a rework of #41695 which was reverted in #42346.

This issue with #41695 was that the framework side did not get an answer when the channel was setup in the engine without registering a handler (on the engine side) to handle framework requests. The issue was reproducible when the engine initialization was managed by the app (see flutter/flutter#122441 (comment) for a repro).

This PR fixes this issue by changing `flutter/keyboard` lifecycle: the engine now creates the channel and registers a handler just after the channel creation.
In order to avoid regression, this PR also updates the channel implemenation (see `KeyboardChannel`) to return an empty `HashMap` when there is no handler registered.

## Related Issue

Android engine implementation for flutter/flutter#87391
(see #42346 for Linux implementation)
Fixes flutter/flutter#122441

## Tests

Adds 3 tests.
auto-submit bot pushed a commit that referenced this pull request Jul 26, 2023
## Description

This PR updates the macOS engine in order to answer to keyboard pressed state queries from the framework (as implemented in flutter/flutter#122885).

## Related Issue

macOS engine implementation for flutter/flutter#87391
Similar to:
- Linux: #42346
- Android: #42758

## Tests

Adds 2 tests.
@bleroux bleroux deleted the linux_return_keyboard_pressed_state branch August 4, 2023 07:34
auto-submit bot pushed a commit that referenced this pull request Aug 9, 2023
## Description

This PR updates the Windows engine in order to answer to keyboard pressed state queries from the framework (as implemented in flutter/flutter#122885).

## Related Issue

Windows engine implementation for flutter/flutter#87391.

Similar to:
- Linux: #42346
- Android: #42758
- macOS: #42878

## Tests

Adds 2 tests.
gaaclarke pushed a commit to gaaclarke/engine that referenced this pull request Aug 30, 2023
## Description

This PR updates the macOS engine in order to answer to keyboard pressed state queries from the framework (as implemented in flutter/flutter#122885).

## Related Issue

macOS engine implementation for flutter/flutter#87391
Similar to:
- Linux: flutter#42346
- Android: flutter#42758

## Tests

Adds 2 tests.
gaaclarke pushed a commit to gaaclarke/engine that referenced this pull request Aug 30, 2023
## Description

This PR updates the Windows engine in order to answer to keyboard pressed state queries from the framework (as implemented in flutter/flutter#122885).

## Related Issue

Windows engine implementation for flutter/flutter#87391.

Similar to:
- Linux: flutter#42346
- Android: flutter#42758
- macOS: flutter#42878

## Tests

Adds 2 tests.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects: desktop autosubmit Merge PR when tree becomes green via auto submit App platform-linux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants