Page MenuHomePhabricator
Differential D6642

[10/n] Native Typeahead - SelectableTextInput for Android
ClosedPublic
Actions

Authored by ashoat on Feb 6 2023, 5:49 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Oct 29, 5:28 AM
Unknown Object (File)
Tue, Oct 29, 5:27 AM
Unknown Object (File)
Tue, Oct 29, 5:27 AM
Unknown Object (File)
Tue, Oct 29, 5:27 AM
Unknown Object (File)
Tue, Oct 29, 5:27 AM
Unknown Object (File)
Tue, Oct 29, 5:27 AM
Unknown Object (File)
Tue, Oct 29, 5:23 AM
Unknown Object (File)
Mon, Oct 28, 11:59 PM
View All Files
Subscribers
None

Details

Reviewers
• przemek
tomek
atul
Commits
rCOMM54ac3ec9be61: [10/n] Native Typeahead - SelectableTextInput for Android
Summary

Putting this one up first, as it's the "base case". iOS basically has to do everything that Android does.

Depends on D6641

Test Plan
  • Tested both iOS and Android
  • Made sure there was no flicker
  • Tested @-tag in both middle and end of text
  • Tested moving cursor after selecting @-tag

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

ashoat created this revision.Feb 6 2023, 5:49 PM
ashoat added a child revision: D6643: [11/n] Native Typeahead - SelectableTextInput for iOS.
Harbormaster completed remote builds in B16206: Diff 22170.Feb 6 2023, 6:05 PM
ashoat requested review of this revision.Feb 6 2023, 6:05 PM
ashoat added inline comments.Feb 7 2023, 6:07 AM
native/components/selectable-text-input.react.js
37 ↗(On Diff #22170)

I should remove this comment – we *are* doing something

tomek accepted this revision.Feb 7 2023, 6:41 AM
tomek added 1 blocking reviewer(s): • przemek.
tomek added inline comments.
native/components/selectable-text-input.react.js
60–62 ↗(On Diff #22170)

We can use a shorthand

73–75 ↗(On Diff #22170)

We can consider destructuring selection out of rest props because we're always overriding it here.

const {
  clearableTextInputRef,
  onUpdateSyncedSelectionData,
  onSelectionChange,
  selection,
  ...rest
} = props;
ashoat updated this revision to Diff 22189.Feb 7 2023, 7:23 AM

@tomek's feedback

Harbormaster completed remote builds in B16224: Diff 22189.Feb 7 2023, 7:37 AM
ashoat updated this revision to Diff 22203.Feb 7 2023, 9:04 AM

Remove inaccurate comment

Harbormaster failed remote builds in B16233: Diff 22203!Feb 7 2023, 9:05 AM
ashoat updated this revision to Diff 22217.Feb 7 2023, 9:11 AM

Rebase

Harbormaster completed remote builds in B16245: Diff 22217.Feb 7 2023, 9:52 AM
ashoat removed 1 blocking reviewer(s): • przemek.Feb 7 2023, 2:28 PM
This revision is now accepted and ready to land.Feb 7 2023, 2:28 PM
Closed by commit rCOMM54ac3ec9be61: [10/n] Native Typeahead - SelectableTextInput for Android (authored by ashoat). · Explain WhyFeb 7 2023, 2:37 PM
This revision was automatically updated to reflect the committed changes.
ashoat added a commit: rCOMM54ac3ec9be61: [10/n] Native Typeahead - SelectableTextInput for Android.
• przemek added inline comments.Feb 8 2023, 5:35 AM
native/components/selectable-text-input.react.js
60 ↗(On Diff #22252)

It looks like we're not using onSelection when creating this component in ChatInputBar Is it there to keep compatibility with TextInput API (i.e. someone sometime in future may want to use that normal callback) or is there something I missed?

ashoat added inline comments.Feb 8 2023, 6:05 AM
native/components/selectable-text-input.react.js
60 ↗(On Diff #22252)

Yes, we don't want to break compatibility. You can see we forward all TextInput props... SelectableTextInput is meant as a drop-in replacement for TextInput that addresses issues with selectable text

Revision Contents
Changeset List

PathSize
native/
components/
50 lines

Diff 22203

View Options

native/components/selectable-text-input.react.js

Loading...