Skip to content

Windows/Linux keyboard shortcuts at a wordwrap #96323

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.

Sign up for GitHub

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

Jump to bottom
Merged
fluttergithubbot merged 12 commits into from
Feb 11, 2022
Merged

Windows/Linux keyboard shortcuts at a wordwrap #96323

fluttergithubbot merged 12 commits into from
Feb 11, 2022

Conversation

@justinmc
Copy link
Contributor

@justinmc justinmc commented Jan 8, 2022 •
edited
Loading

Windows, Mac, and Linux all have different behavior for how wordwraps are treated by keyboard shortcuts that jump to the beginning/end of a line. Previously, Flutter always stopped at wordwrapped lines. This PR aims to get the behavior correct on all platforms.

I believe I covered all of the cases affected by this PR in the tests, so to see exactly what the behavior should be on each platform, check the tests. These are the high level features:

  • When jumping to the start/end of a line multiple times, some platforms will jump between lines when it's a wordwrap and not a newline.
  • On Mac, home/end scroll to the start/end of the field without moving the cursor.
  • On Mac, shift + home/end selects to the start/end of the document.
  • Bugfix: Some of these kinds of shortcuts weren't calling bringIntoView, so they wouldn't cause the field to scroll.

Fixes #90993

@flutter-dashboard flutter-dashboard bot added a: text input Entering text in a text field or keyboard related problems framework flutter/packages/flutter repository. See also f: labels. labels Jan 8, 2022
@justinmc justinmc self-assigned this Jan 8, 2022
@justinmc
Copy link
Contributor Author

justinmc commented Jan 11, 2022

I'm going to consider this blocked on #95948, which will bring back the extend/expand distinction needed to fix my last TODO (shift+home/end expands on Mac).

@justinmc justinmc force-pushed the line-break-continue branch from 9c74d81 to a5f819c Compare January 14, 2022 20:59
justinmc
justinmc commented Jan 14, 2022
View reviewed changes
packages/flutter/test/widgets/editable_text_test.dart Outdated
Copy link
Contributor Author

@justinmc justinmc Jan 14, 2022

Choose a reason for hiding this comment

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

I think this was a typo in this test (probably by me in an earlier PR).

@justinmc justinmc force-pushed the line-break-continue branch from a5f819c to 7ed042b Compare January 14, 2022 21:02
@justinmc justinmc marked this pull request as ready for review January 14, 2022 21:07
LongCatIsLooong
LongCatIsLooong reviewed Jan 31, 2022
View reviewed changes
packages/flutter/lib/src/widgets/editable_text.dart Outdated
Copy link
Contributor

@LongCatIsLooong LongCatIsLooong Jan 31, 2022

Choose a reason for hiding this comment

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

I think ideally this shouldn't be implemented in _LineBreak, as it's only supposed to tell you where the boundary is.

Copy link
Contributor Author

@justinmc justinmc Feb 2, 2022

Choose a reason for hiding this comment

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

I've moved it to the action and I think it makes more sense there, thanks for the suggestion.

In order to do this I added continuesAtWrap to DirectionalCaretMovementIntent even though most of its subclasses don't use continuesAtWrap. Let me know if you think that's not alright.

@goderbauer
Copy link
Member

goderbauer commented Feb 9, 2022

(triage) @justinmc Looks like google testing got stuck on this one and I don't see a way to restart it.. Maybe push an empty commit?

@justinmc
Copy link
Contributor Author

justinmc commented Feb 10, 2022

I pushed a merge commit, hopefully that fixes it. Thanks for the reminder.

@fluttergithubbot
Copy link
Contributor

fluttergithubbot commented Feb 10, 2022

This pull request is not suitable for automatic merging in its current state.

  • The status or check suite customer_testing-linux has failed. Please fix the issues identified (or deflake) before re-applying this label.

@justinmc justinmc force-pushed the line-break-continue branch from ca982bc to 6a97a50 Compare February 10, 2022 04:21
@fluttergithubbot fluttergithubbot merged commit c42b36f into flutter:master Feb 11, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 11, 2022
@engine-flutter-autoroll
c42b36f Windows/Linux keyboard shortcuts at a wordwrap (flutter/flutt…
b30b0a6 
…er#96323)
This was referenced Feb 11, 2022
Closed
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 11, 2022
@engine-flutter-autoroll
c42b36f Windows/Linux keyboard shortcuts at a wordwrap (flutter/flutt…
ee6de4e 
…er#96323)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 11, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Feb 11, 2022
@engine-flutter-autoroll
c42b36f Windows/Linux keyboard shortcuts at a wordwrap (flutter/flutt…
07dc571 
…er#96323)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 11, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Feb 11, 2022
@engine-flutter-autoroll
c42b36f Windows/Linux keyboard shortcuts at a wordwrap (flutter/flutt…
4b66495 
…er#96323)
@justinmc justinmc deleted the line-break-continue branch February 11, 2022 17:39
clocksmith pushed a commit to clocksmith/flutter that referenced this pull request Mar 8, 2022
@justinmc @clocksmith
Windows/Linux keyboard shortcuts at a wordwrap (flutter#96323)
f028cb2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@LongCatIsLooong LongCatIsLooong LongCatIsLooong approved these changes

Assignees

@justinmc justinmc

Labels

a: text input Entering text in a text field or keyboard related problems framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Windows and Linux text navigation shortcuts treat newline and wordwrap differently

4 participants

@justinmc @goderbauer @fluttergithubbot @LongCatIsLooong