[NUI] Support software keyboard input #349

JSUYA · 2022-09-15T07:57:38Z

Converts key event information from NUI to ecore_imf_event and delivers it.

related issue: #340
related pr : flutter-tizen/flutter-tizen#448

shell/platform/tizen/flutter_tizen_view.h

JSUYA · 2022-09-15T08:06:49Z

shell/platform/tizen/tizen_input_method_context.cc

+  imf_event.key = key;
+  imf_event.string = string;
+  imf_event.modifiers = EcoreInputModifiersToEcoreImfModifiers(modifiers);
+  imf_event.locks = EcoreInputModifiersToEcoreImfLocks(modifiers);
+  imf_event.keycode = scan_code;


EcoreEvent passes more information than this.
But I don't know why they are needed.
When I tested, only key and string were sufficient.
(ewk webview only uses key and string. (evas event))
https://github.com/flutter-tizen/plugins/pull/400/files/50ac06ef0278cdf8ab3d814ab3aee7d0fabe6135#diff-c41c7de9898e3e8e225393fb4381982fb6248999451c306948584e1c330d10afR289)

If we need more, I can get it from NUI, but there is no API to get compose and time stamp information in NUI's key event.

JSUYA · 2022-09-19T02:07:32Z

I will update this patch when #351 is merged.

shell/platform/tizen/tizen_input_method_context.cc

swift-kim · 2022-09-19T03:32:32Z

shell/platform/tizen/tizen_view_elementary.cc

+void TizenViewElementary::KeyEvent(const char* key,
+                                   const char* string,
+                                   const char* compose,
+                                   uint32_t modifiers,
+                                   uint32_t scan_code,
+                                   bool is_down) {
+  FT_UNIMPLEMENTED();


Will this ever be implemented in the future or be no-op?

Actually, I'm not sure if this will ever be used in ElmTizenVIew.
I modified it to no-op.

We may have a better design in the future..

I wrapped the code used only for nui with NUI_SUPPORT and deleted TizenViewElementary::OnKey.
Could you please review it again?

I'm afraid the code is not very predictable but I'm okay with the change.

bbrto21 · 2022-09-21T08:39:48Z

Could you please rebase to resolve conflict :)

shell/platform/tizen/tizen_input_method_context.cc

swift-kim · 2022-09-28T03:11:18Z

shell/platform/tizen/tizen_input_method_context.cc

+bool TizenInputMethodContext::HandleNuiKeyEvent(const char* device_name,
+                                                uint32_t device_class,
+                                                uint32_t device_subclass,
+                                                const char* key,
+                                                const char* string,
+                                                uint32_t modifiers,
+                                                uint32_t scan_code,
+                                                size_t timestamp,
+                                                bool is_down) {


This method (or TizenViewNui::OnKey) has too many parameters. They may be wrapped up into a class or struct but I'm not sure if it's a good solution.

These are all Ecore_Events, but NUI doesn't provide all properties of EcoreEvent (maybe?).
I think we can add a struct NuiEventKey(?) structure, but I'd like to improve it in another PR if needed.

shell/platform/tizen/tizen_input_method_context.cc

shell/platform/tizen/flutter_tizen.cc

swift-kim · 2022-10-05T06:22:16Z

shell/platform/tizen/public/flutter_tizen.h

+    size_t timestamp = 0,
+    const char* device_name = nullptr,
+    uint32_t device_class = 0,
+    uint32_t device_subclass = 0);


Does the C standard support "default arguments" or function overloading?

Default argument is c++ feature.
But because of extern "C", symbols will be saved without default argument applied.(Maybe?) and since there are no duplicate symbols, it doesn't seem to be a problem now.
Is there any good way?
(The failure of githubaction doesn't seem to be due to this?)

Even if the code compiles without any error, it doesn't seem a good practice. A better way would be explicitly passing the default values (nullptr or 0) or defining a struct type as I mentioned above.

I reverted this patch(Change to default parameter) This was bad code.
I will consider the struct type parameter for NUI later. Until then, I want to keep it as it is.
+) @Swanseo0

Converts key event information from NUI to ecore_imf_event and delivers it.

This reverts commit a98e998.

JSUYA · 2022-10-12T04:47:00Z

@swift-kim @Swanseo0 Are there any more points left to review? Can I merge this?

swift-kim · 2022-10-12T05:45:15Z

@JSUYA Yes, you can. Don't forget to

Clean up the commit message when squash merging the commits.
Link the issue Improve text input on NUI view #340 if this PR will close the issue.

Pass timestamp and device info to embedder. related pr : flutter-tizen/engine#349

JSUYA commented Sep 15, 2022

View reviewed changes

shell/platform/tizen/flutter_tizen_view.h Outdated Show resolved Hide resolved

JSUYA commented Sep 15, 2022

View reviewed changes

JSUYA force-pushed the dev/nui_view_swkeyboard branch from d587afc to a5b909b Compare September 15, 2022 09:05

swift-kim reviewed Sep 19, 2022

View reviewed changes

swift-kim changed the title ~~Support s/w keyboard input~~ [NUI] Support software keyboard input Sep 19, 2022

JSUYA force-pushed the dev/nui_view_swkeyboard branch 2 times, most recently from 1addb73 to 3f1f76a Compare September 27, 2022 02:33

JSUYA mentioned this pull request Sep 27, 2022

Add more NUI key information flutter-tizen/flutter-tizen#448

Merged

swift-kim reviewed Sep 27, 2022

View reviewed changes

shell/platform/tizen/tizen_input_method_context.cc Outdated Show resolved Hide resolved

JSUYA force-pushed the dev/nui_view_swkeyboard branch from 45ef385 to 5a11ee2 Compare September 28, 2022 01:23

swift-kim reviewed Sep 28, 2022

View reviewed changes

Swanseo0 reviewed Sep 29, 2022

View reviewed changes

shell/platform/tizen/flutter_tizen.cc Show resolved Hide resolved

JSUYA force-pushed the dev/nui_view_swkeyboard branch from 5a11ee2 to a98e998 Compare October 5, 2022 06:02

swift-kim reviewed Oct 5, 2022

View reviewed changes

JSUYA added 9 commits October 11, 2022 12:15

Support software keyboard input

d59781b

Converts key event information from NUI to ecore_imf_event and delivers it.

Update based on review

d6e4a06

Update based on review

bebb254

Add device info

7f8cef0

Rename NuiEventKey to NuiKeyEvent

bb8d7f7

Update based on review

eee1ce2

Resolve merge conflict

25f4900

Change to default parameter

dc26259

Revert "Change to default parameter"

bb88ea0

This reverts commit a98e998.

JSUYA force-pushed the dev/nui_view_swkeyboard branch from a797b1f to bb88ea0 Compare October 11, 2022 04:07

bbrto21 approved these changes Oct 12, 2022

View reviewed changes

JSUYA merged commit 87e3e3b into flutter-tizen:flutter-3.3.0-tizen Oct 12, 2022

JSUYA linked an issue Oct 12, 2022 that may be closed by this pull request

Improve text input on NUI view #340

Closed

JSUYA mentioned this pull request Oct 12, 2022

Improve text input on NUI view #340

Closed

JSUYA added a commit to flutter-tizen/flutter-tizen that referenced this pull request Oct 12, 2022

Add more NUI key information (#448)

805f840

Pass timestamp and device info to embedder. related pr : flutter-tizen/engine#349

[NUI] Support software keyboard input #349

[NUI] Support software keyboard input #349

Uh oh!

Conversation

JSUYA commented Sep 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JSUYA commented Sep 19, 2022

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bbrto21 commented Sep 21, 2022

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JSUYA Oct 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JSUYA commented Oct 12, 2022

Uh oh!

swift-kim commented Oct 12, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

JSUYA commented Sep 15, 2022 •

edited

Loading

JSUYA Oct 5, 2022 •

edited

Loading