Skip to content

Conversation

@jonahwilliams
Copy link
Contributor

@jonahwilliams jonahwilliams commented Jul 29, 2018

Extends the movement granularity support by text to byWord. To mimic the Android implementation, we skip words which only contain whitespace and unicode separator values.

Fixes #13389
Work towards #13406

@jonahwilliams jonahwilliams changed the title [WIP] add support for all movement granularities to semantics Add support for by word movement granularities to semantics Aug 6, 2018
@jonahwilliams
Copy link
Contributor Author

Analyze died quickly for some reason so macos tests didn't run, should pass and will rerun when it is done.

@jonahwilliams jonahwilliams requested review from HansMuller and removed request for HansMuller August 6, 2018 20:17
Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

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

LGTM

return;
final int baseOffset = !extentSelection ? nextWord.start : _selection.baseOffset;
onSelectionChanged(
new TextSelection(baseOffset: baseOffset, extentOffset: nextWord.start), this, SelectionChangedCause.keyboard);
Copy link
Member

Choose a reason for hiding this comment

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

nit: maybe put each argument on a separate line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

final TextRange nextWord = _getNextWord(currentWord.end);
if (nextWord == null)
return;
final int baseOffset = !extentSelection ? nextWord.start : _selection.baseOffset;
Copy link
Member

Choose a reason for hiding this comment

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

This might be easier to comprehend if you switch it around and remove the negation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

if (!_onlyWhitespace(range))
return range;
offset = range.start - 1;
}
Copy link
Member

Choose a reason for hiding this comment

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

is this missing a return statement if offset drops below 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

for (int i = range.start; i < range.end; i++) {
final int codeUnit = text.codeUnitAt(i);
switch (codeUnit) {
case 0x9: // horizontal tab
Copy link
Member

Choose a reason for hiding this comment

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

Could you link to where this list is coming from in case we need to update it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@jonahwilliams jonahwilliams merged commit e56a73e into flutter:master Aug 17, 2018
@jonahwilliams jonahwilliams deleted the add_movement_granulairities branch August 17, 2018 19:18
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

add support for more granularities for moving text input cursor

3 participants