Skip to content

Conversation

@camsim99
Copy link
Contributor

@camsim99 camsim99 commented Aug 10, 2022

Same as #104460.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@flutter-dashboard flutter-dashboard bot added a: tests "flutter test", flutter_test, or one of our tests a: text input Entering text in a text field or keyboard related problems f: cupertino flutter/packages/flutter/cupertino repository f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. c: contributor-productivity Team-specific productivity, code health, technical debt. labels Aug 10, 2022
@camsim99 camsim99 changed the title Spellcheck Add Spellcheck to EditableText (Android) Aug 10, 2022
@camsim99 camsim99 marked this pull request as ready for review August 10, 2022 21:37
@camsim99 camsim99 requested a review from keyonghan as a code owner August 10, 2022 21:37
@camsim99 camsim99 requested review from justinmc and removed request for keyonghan August 10, 2022 21:43
Copy link
Contributor

@justinmc justinmc left a comment

Choose a reason for hiding this comment

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

LGTM with nits. Mainly just the question about making some functions private.

text: text.substring(composingRegion.start, composingRegion.end)
)
);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see a problem with keeping everything here in its own file in the widgets library 👍 . Unless @camsim99 or another reviewer has a better idea.

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 definitely be useful to group the methods together especially when the menu logic needs to be added somewhere.

@camsim99 camsim99 requested a review from cyanglaz August 12, 2022 16:19
@camsim99
Copy link
Contributor Author

@cyanglaz Would you be willing to give this a second look?

Copy link
Contributor

@cyanglaz cyanglaz left a comment

Choose a reason for hiding this comment

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

LGTM

@camsim99 camsim99 added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 16, 2022
@auto-submit auto-submit bot merged commit ff9fe35 into flutter:master Aug 16, 2022
jonahwilliams pushed a commit that referenced this pull request Aug 16, 2022
jonahwilliams pushed a commit that referenced this pull request Aug 16, 2022
Copy link

@007hz 007hz left a comment

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: tests "flutter test", flutter_test, or one of our tests a: text input Entering text in a text field or keyboard related problems autosubmit Merge PR when tree becomes green via auto submit App c: contributor-productivity Team-specific productivity, code health, technical debt. f: cupertino flutter/packages/flutter/cupertino repository f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants