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

send newline char when input type is multiline #20660

Merged

Conversation

maks
Copy link
Contributor

@maks maks commented Aug 20, 2020

Description

This PR re-instates the functionality for sending a newline char for TextFields on Linux desktop which have a keyboardType: TextInputType.multiline set on the widget. This functionality was lost in the move from the GLFW to the new GTK embedding.

Related Issues

Fixes: flutter/flutter#62207

Tests

There is no existing test for this, so the regression was not picked up in the move from glfw to gtk. I'm sorry but I don't know how to go about adding a test for this.

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the contributor guide and followed the process outlined there for submitting PRs.
  • I signed the CLA.
  • I read and followed the C++, Objective-C, Java style guides for the engine.
  • I read the tree hygiene wiki page, which explains my responsibilities.
  • I updated/added relevant documentation.
  • All existing and new tests are passing.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read handling breaking changes.

@maks maks marked this pull request as ready for review August 21, 2020 00:12
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@domesticmouse
Copy link
Contributor

/cc @stuartmorgan

shell/platform/linux/fl_text_input_plugin.cc Outdated Show resolved Hide resolved
shell/platform/linux/fl_text_input_plugin.cc Outdated Show resolved Hide resolved
shell/platform/linux/fl_text_input_plugin.cc Outdated Show resolved Hide resolved
shell/platform/linux/fl_text_input_plugin.cc Show resolved Hide resolved
shell/platform/linux/fl_text_input_plugin.cc Show resolved Hide resolved
shell/platform/linux/fl_text_input_plugin.cc Outdated Show resolved Hide resolved
@maks
Copy link
Contributor Author

maks commented Aug 24, 2020

@stuartmorgan thanks for the very quick review! Hopefully I've fixed all the issues from your feedback.

Copy link
Contributor

@stuartmorgan stuartmorgan left a comment

Choose a reason for hiding this comment

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

Functionality looks great, just minor comment nits and then it can be landed. Thanks!

shell/platform/linux/fl_text_input_plugin.cc Outdated Show resolved Hide resolved
shell/platform/linux/fl_text_input_plugin.cc Outdated Show resolved Hide resolved
Copy link
Contributor

@stuartmorgan stuartmorgan left a comment

Choose a reason for hiding this comment

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

LGTM! I'll set the CQ bit once tests have come back green. Thanks for fixing this!

@stuartmorgan stuartmorgan added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Aug 25, 2020
@fluttergithubbot fluttergithubbot merged commit 57fdf0a into flutter:master Aug 25, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 25, 2020
@maks maks deleted the 62207-linux-textfield-add-newline branch August 25, 2020 23:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects: desktop cla: yes needs tests platform-linux waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TextField fails to accept newline in Linux client
6 participants