[linux] Adds raw key events plugin. #144
Conversation
| AddPlugin(window, std::move(input_plugin)); | ||
|
|
||
| auto input_plugin = std::make_unique<TextInputPlugin>(); | ||
| state->keyboard_hook_handlers.push_back(input_plugin.get()); | ||
|
|
||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
| #include "library/linux/src/internal/key_event_plugin.h" | ||
| #include <flutter_embedder.h> |
There was a problem hiding this comment.
Nit: whitespace before system includes
| void KeyEventPlugin::KeyboardHook(GLFWwindow *window, int key, int scancode, | ||
| int action, int mods) { | ||
| Json::Value args; | ||
| args[kKeyCodeKey] = key; |
There was a problem hiding this comment.
Can you add a TODO comment to translate to a cross-platform key code system rather than just reflecting the platform-specific one up to the Dart layer?
| @@ -58,6 +58,10 @@ std::unique_ptr<MethodCall> JsonMethodCodec::DecodeMethodCallInternal( | |||
| std::unique_ptr<std::vector<uint8_t>> JsonMethodCodec::EncodeMethodCallInternal( | |||
| const MethodCall &method_call) const { | |||
| Json::Value message(Json::objectValue); | |||
| if (method_call.method_name().empty()) { | |||
There was a problem hiding this comment.
Why is this code necessary now? Is this unrelated cleanup?
There was a problem hiding this comment.
This change shouldn't be made to MethodCodec.
The issue here is that flutter/keyevent is a MessageChannel, not a MethodChannel, on the Dart side. MethodChannel is essentially a layer over MessageChannel that establishes the name/argument structure. Making *MethodCodec allow non-Method-structured messages goes against that distinction.
The right fix, which would align with the overall work of #102 is to split out JsonMessageCodec from JsonMethodCodec, but that will also require building out support for Message-based plugins. The next step of plugin refactoring that I have planned would help lay the foundation for that.
Is support for raw key events on Linux time critical, or is this something we can revisit once I'm further along with #102? If the former, I think the best way forward in the short term to support the direction of #102 would be:
- Split MessageCodec out from MethodCodec
- Add a new class above Plugin (this would be temporary, since Plugin will be significantly changed or even eliminated as part of Make the plugin API structure match the existing Flutter plugin API #102 eventually anyway) that operates on generic data messages.
- Make Plugin do the conversion from data to method call, then forward to the existing Plugin methods.
- Change PluginHandler to take in that new superclass instead.
There was a problem hiding this comment.
We can probably wait. We were trying to get the overall raw key event feature to work in a short term time frame, but we can wait on the Linux implementation side.
There was a problem hiding this comment.
Okay, let's let this PR wait for a bit then. I'm hoping to do more work on plugin refactoring next week, and then we can re-evaluate.
If someone wants to do the MessageCodec/MethodCodec split in the mean time that would be useful parallel work that we'll need regardless.
| @@ -58,6 +58,10 @@ std::unique_ptr<MethodCall> JsonMethodCodec::DecodeMethodCallInternal( | |||
| std::unique_ptr<std::vector<uint8_t>> JsonMethodCodec::EncodeMethodCallInternal( | |||
| const MethodCall &method_call) const { | |||
| Json::Value message(Json::objectValue); | |||
| if (method_call.method_name().empty()) { | |||
There was a problem hiding this comment.
This change shouldn't be made to MethodCodec.
The issue here is that flutter/keyevent is a MessageChannel, not a MethodChannel, on the Dart side. MethodChannel is essentially a layer over MessageChannel that establishes the name/argument structure. Making *MethodCodec allow non-Method-structured messages goes against that distinction.
The right fix, which would align with the overall work of #102 is to split out JsonMessageCodec from JsonMethodCodec, but that will also require building out support for Message-based plugins. The next step of plugin refactoring that I have planned would help lay the foundation for that.
Is support for raw key events on Linux time critical, or is this something we can revisit once I'm further along with #102? If the former, I think the best way forward in the short term to support the direction of #102 would be:
- Split MessageCodec out from MethodCodec
- Add a new class above Plugin (this would be temporary, since Plugin will be significantly changed or even eliminated as part of Make the plugin API structure match the existing Flutter plugin API #102 eventually anyway) that operates on generic data messages.
- Make Plugin do the conversion from data to method call, then forward to the existing Plugin methods.
- Change PluginHandler to take in that new superclass instead.
|
I didn't end up having much time to work on 102 this week, but in starting to look at it, I found a more minimal set of incremental steps to take to unblock this, and just sent both for review:
With those two landed, what you should be able to do here is change your new key handler to not inherit from Plugin, and just have it take a BinaryMessenger in its constructor or as a setter. Then instead of InvokeMethod, you'll call Send on the messenger after using JsonMessageCodec to encode your message. Since it won't be a plugin you'll need to have something own it; I'm fine with just adding it to FlutterEmbedderState. (Eventually, once 102 is finished, it will end up re-aligned with whatever Plugins turn into, so it won't stay they permanently.) |
#143