Skip to content

Refactor TextInputChannel #142

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jul 15, 2021
Merged

Conversation

bbrto21
Copy link

@bbrto21 bbrto21 commented Jul 13, 2021

  • Introduce TizenInputMethodContext
  • Do not handle key event for PlatformView in TextInputChannel
    Each PlatformView must Implement to handle key event.
    After this patch, the input panel of webview_flutter does not work properly.
    but it have to handle every key event on itself like webview_flutter_ewk does.

Signed-off-by: Boram Bae boram21.bae@samsung.com

* Introduce TizenInputMethodContext
* Do not handle key event for PlatformView in TextInputChannel
  Each PlatformView must Implement to handle key event.
  After this patch, the input panel of webview_flutter does not work properly.
  but it have to handle every key event on itself like webview_flutter_ewk does.

Signed-off-by: Boram Bae <boram21.bae@samsung.com>
@bwikbs
Copy link
Member

bwikbs commented Jul 14, 2021

  void DispatchCompositionUpdateEvent(const std::string& key);
  void DispatchCompositionEndEvent(const std::string& key);

Is this intentional? Shouldn't we remove this?

@bbrto21
Copy link
Author

bbrto21 commented Jul 14, 2021

  void DispatchCompositionUpdateEvent(const std::string& key);
  void DispatchCompositionEndEvent(const std::string& key);

Is this intentional? Shouldn't we remove this?

Yes, the text composition of the PlatformView must be independent of the TextInputChannel.
IMHO, I think these interfaces are unnecessary.

@bwikbs
Copy link
Member

bwikbs commented Jul 14, 2021

If we don't need it, how about removing it this time? We need a engine release because of the platformview interface change , so let's do it together.

@bbrto21
Copy link
Author

bbrto21 commented Jul 14, 2021

If we don't need it, how about removing it this time? We need a engine release because of the platformview interface change , so let's do it together.

I will remove it immediately.

Comment on lines 46 to 51
if (engine->text_input_channel) {
if (is_down) {
if (is_down && engine->text_input_channel->IsSoftwareKeyboardShowing()) {
engine->text_input_channel->OnKeyDown(key);
}
if (engine->text_input_channel->IsSoftwareKeyboardShowing()) {
return ECORE_CALLBACK_PASS_ON;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

The text input channel only handles the key event when the software keyboard is shown?

Copy link
Author

Choose a reason for hiding this comment

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

Well... strictly, I should say no, If the TextInputModel is valid, it can handle key-event.
It is created when TextInputChannel receives TextInput.setClient as a message.
(The message for showing the keyboard is TextInput.show.)

Copy link
Author

Choose a reason for hiding this comment

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

How about this?

Copy link
Member

Choose a reason for hiding this comment

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

Mmmm. You have to carefully see how the original logic looks like. It skips sending a key event to key_event_channel only when the software keyboard is shown.

Copy link
Author

Choose a reason for hiding this comment

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

I think... it's changed a bit, but the behavior hasn't changed much.

bbrto21 added 3 commits July 14, 2021 14:20
Signed-off-by: Boram Bae <boram21.bae@samsung.com>
Signed-off-by: Boram Bae <boram21.bae@samsung.com>
* Use SendKeyEvent as the method name for handling key events like others.
* SendKeyEvent returns true if the key event has been handled.

Signed-off-by: Boram Bae <boram21.bae@samsung.com>
@bbrto21
Copy link
Author

bbrto21 commented Jul 14, 2021

I will remove it immediately.

@bwikbs
Please, review again

Copy link
Member

@bwikbs bwikbs left a comment

Choose a reason for hiding this comment

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

Please, review again

Thanks! 👍

bbrto21 added 2 commits July 15, 2021 12:59
Signed-off-by: Boram Bae <boram21.bae@samsung.com>
Signed-off-by: Boram Bae <boram21.bae@samsung.com>
@swift-kim swift-kim merged commit 6280c13 into flutter-tizen:flutter-2.2.1-tizen Jul 15, 2021
swift-kim pushed a commit that referenced this pull request Sep 27, 2021
* Refactor TextInputChannel

* Introduce TizenInputMethodContext
* Do not handle key event for PlatformView in TextInputChannel
  Each PlatformView must Implement to handle key event.
  After this patch, the input panel of webview_flutter does not work properly.
  but it have to handle every key event on itself like webview_flutter_ewk does.

Signed-off-by: Boram Bae <boram21.bae@samsung.com>

* Remove unnecessary PlatformView APIs

Signed-off-by: Boram Bae <boram21.bae@samsung.com>

* Tidy up based on review

Signed-off-by: Boram Bae <boram21.bae@samsung.com>

* Always Send key event to TextInputChannel

* Use SendKeyEvent as the method name for handling key events like others.
* SendKeyEvent returns true if the key event has been handled.

Signed-off-by: Boram Bae <boram21.bae@samsung.com>

* Use #if defined() instead of #ifdef

Signed-off-by: Boram Bae <boram21.bae@samsung.com>

* Rename to ShouldNotFilterEvent

Signed-off-by: Boram Bae <boram21.bae@samsung.com>
swift-kim pushed a commit that referenced this pull request Nov 14, 2021
* Refactor TextInputChannel

* Introduce TizenInputMethodContext
* Do not handle key event for PlatformView in TextInputChannel
  Each PlatformView must Implement to handle key event.
  After this patch, the input panel of webview_flutter does not work properly.
  but it have to handle every key event on itself like webview_flutter_ewk does.

Signed-off-by: Boram Bae <boram21.bae@samsung.com>

* Remove unnecessary PlatformView APIs

Signed-off-by: Boram Bae <boram21.bae@samsung.com>

* Tidy up based on review

Signed-off-by: Boram Bae <boram21.bae@samsung.com>

* Always Send key event to TextInputChannel

* Use SendKeyEvent as the method name for handling key events like others.
* SendKeyEvent returns true if the key event has been handled.

Signed-off-by: Boram Bae <boram21.bae@samsung.com>

* Use #if defined() instead of #ifdef

Signed-off-by: Boram Bae <boram21.bae@samsung.com>

* Rename to ShouldNotFilterEvent

Signed-off-by: Boram Bae <boram21.bae@samsung.com>
swift-kim pushed a commit that referenced this pull request Dec 9, 2021
* Refactor TextInputChannel

* Introduce TizenInputMethodContext
* Do not handle key event for PlatformView in TextInputChannel
  Each PlatformView must Implement to handle key event.
  After this patch, the input panel of webview_flutter does not work properly.
  but it have to handle every key event on itself like webview_flutter_ewk does.

Signed-off-by: Boram Bae <boram21.bae@samsung.com>

* Remove unnecessary PlatformView APIs

Signed-off-by: Boram Bae <boram21.bae@samsung.com>

* Tidy up based on review

Signed-off-by: Boram Bae <boram21.bae@samsung.com>

* Always Send key event to TextInputChannel

* Use SendKeyEvent as the method name for handling key events like others.
* SendKeyEvent returns true if the key event has been handled.

Signed-off-by: Boram Bae <boram21.bae@samsung.com>

* Use #if defined() instead of #ifdef

Signed-off-by: Boram Bae <boram21.bae@samsung.com>

* Rename to ShouldNotFilterEvent

Signed-off-by: Boram Bae <boram21.bae@samsung.com>
swift-kim pushed a commit that referenced this pull request Dec 17, 2021
* Refactor TextInputChannel

* Introduce TizenInputMethodContext

* Remove unnecessary PlatformView APIs

* Tidy up based on review

* Always Send key event to TextInputChannel

* Use #if defined() instead of #ifdef

* Rename to ShouldNotFilterEvent

Signed-off-by: Boram Bae <boram21.bae@samsung.com>
swift-kim pushed a commit that referenced this pull request Feb 7, 2022
* Refactor TextInputChannel

* Introduce TizenInputMethodContext

* Remove unnecessary PlatformView APIs

* Tidy up based on review

* Always Send key event to TextInputChannel

* Use #if defined() instead of #ifdef

* Rename to ShouldNotFilterEvent

Signed-off-by: Boram Bae <boram21.bae@samsung.com>
swift-kim pushed a commit that referenced this pull request Feb 11, 2022
* Refactor TextInputChannel

* Introduce TizenInputMethodContext

* Remove unnecessary PlatformView APIs

* Tidy up based on review

* Always Send key event to TextInputChannel

* Use #if defined() instead of #ifdef

* Rename to ShouldNotFilterEvent

Signed-off-by: Boram Bae <boram21.bae@samsung.com>
swift-kim pushed a commit that referenced this pull request May 12, 2022
* Refactor TextInputChannel

* Introduce TizenInputMethodContext

* Remove unnecessary PlatformView APIs

* Tidy up based on review

* Always Send key event to TextInputChannel

* Use #if defined() instead of #ifdef

* Rename to ShouldNotFilterEvent

Signed-off-by: Boram Bae <boram21.bae@samsung.com>
swift-kim pushed a commit that referenced this pull request Aug 5, 2022
* Refactor TextInputChannel

* Introduce TizenInputMethodContext

* Remove unnecessary PlatformView APIs

* Tidy up based on review

* Always Send key event to TextInputChannel

* Use #if defined() instead of #ifdef

* Rename to ShouldNotFilterEvent

Signed-off-by: Boram Bae <boram21.bae@samsung.com>
swift-kim pushed a commit that referenced this pull request Sep 1, 2022
* Refactor TextInputChannel

* Introduce TizenInputMethodContext

* Remove unnecessary PlatformView APIs

* Tidy up based on review

* Always Send key event to TextInputChannel

* Use #if defined() instead of #ifdef

* Rename to ShouldNotFilterEvent

Signed-off-by: Boram Bae <boram21.bae@samsung.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants