Skip to content
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

Support onSubmitEditing event #3423

Merged
merged 4 commits into from
Oct 17, 2019
Merged

Support onSubmitEditing event #3423

merged 4 commits into from
Oct 17, 2019

Conversation

ddalp
Copy link
Contributor

@ddalp ddalp commented Oct 15, 2019

#3083

  • Hookup onSubmitEditing event for TextInput when enter key down is received and not handled
  • Add a e2e test to verify the event firing.
Microsoft Reviewers: Open in CodeFlow

@ddalp ddalp requested a review from a team as a code owner October 15, 2019 17:26
@ghost ghost added the vnext label Oct 15, 2019
m_controlKeyDownRevoker = control.KeyDown(
winrt::auto_revoke, [=](auto &&, winrt::KeyRoutedEventArgs const &args) {
if (args.Key() == winrt::VirtualKey::Enter && !args.Handled()) {
auto instance = wkinstance.lock();
Copy link
Contributor

@licanhua licanhua Oct 15, 2019

Choose a reason for hiding this comment

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

auto instance = wkinstance.lock(); [](start = 10, length = 34)

this will be more readable:
if (auto instance = wkinstance.lock()) {
} #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do


In reply to: 335118934 [](ancestors = 335118934)

HstringToDynamic(control.as<winrt::PasswordBox>().Password()));
}
if (!m_updating && instance != nullptr) {
instance->DispatchEvent(
Copy link
Contributor

@licanhua licanhua Oct 15, 2019

Choose a reason for hiding this comment

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

Don't raise the event if multline flag is specified #WontFix

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is covered in checking if the keydown is already handled above. True for Multiline since enter key is used for text editing.


In reply to: 335120678 [](ancestors = 335120678)

winrt::auto_revoke, [=](auto &&, winrt::KeyRoutedEventArgs const &args) {
if (args.Key() == winrt::VirtualKey::Enter && !args.Handled()) {
auto instance = wkinstance.lock();
folly::dynamic eventDataSubmitEditing = {};
Copy link
Contributor

@licanhua licanhua Oct 15, 2019

Choose a reason for hiding this comment

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

eventDataSubmitEditing [](start = 25, length = 22)

The event has three parameters: {nativeEvent: {text, eventCount, target}}. I don't know if android/iOS provides eventCount or not. #WontFix

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We also don't send eventCount for onEndEidting event, I want to keep the behavior same for onSubmitEditing for now.


In reply to: 335124057 [](ancestors = 335124057)

Copy link
Contributor

@licanhua licanhua left a comment

Choose a reason for hiding this comment

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

:shipit:

@ddalp ddalp merged commit f31f870 into microsoft:master Oct 17, 2019
@ddalp ddalp deleted the submitediting branch October 17, 2019 02:43
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.

2 participants