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

Authored by ashoat on Feb 6 2023, 5:49 PM.

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	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	We can use a shorthand
73–75	We can consider destructuring `selection` out of `rest` props because we're always overriding it here. const { clearableTextInputRef, onUpdateSyncedSelectionData, onSelectionChange, selection, ...rest } = props;

@tomek's feedback

Harbormaster completed remote builds in B16224: Diff 22189.Feb 7 2023, 7:37 AM

Remove inaccurate comment

Harbormaster failed remote builds in B16233: Diff 22203!Feb 7 2023, 9:05 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

Path

Size

native/

components/

selectable-text-input.react.js

52 lines

Diff 22170

View Options

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

// @flow		// @flow

import invariant from 'invariant';		import invariant from 'invariant';
import * as React from 'react';		import * as React from 'react';
import { View } from 'react-native';		import { View } from 'react-native';
import { ScrollView } from 'react-native-gesture-handler';		import { ScrollView } from 'react-native-gesture-handler';

import { fetchPinnedMessages } from 'lib/actions/message-actions.js';		import { fetchPinnedMessages } from 'lib/actions/message-actions.js';
import { messageListData } from 'lib/selectors/chat-selectors.js';		import { messageListData } from 'lib/selectors/chat-selectors.js';
import { createMessageInfo } from 'lib/shared/message-utils.js';		import { createMessageInfo } from 'lib/shared/message-utils.js';
		import { isComposableMessageType } from 'lib/types/message-types.js';
import type { ThreadInfo } from 'lib/types/thread-types.js';		import type { ThreadInfo } from 'lib/types/thread-types.js';
import { useServerCall } from 'lib/utils/action-utils.js';		import { useServerCall } from 'lib/utils/action-utils.js';

import { useHeightMeasurer } from './chat-context.js';		import { useHeightMeasurer } from './chat-context.js';
import type { ChatNavigationProp } from './chat.react';		import type { ChatNavigationProp } from './chat.react';
import MessageResult from './message-result.react.js';		import MessageResult from './message-result.react.js';
import type { NavigationRoute } from '../navigation/route-names';		import type { NavigationRoute } from '../navigation/route-names';
import { useSelector } from '../redux/redux-utils.js';		import { useSelector } from '../redux/redux-utils.js';
▲ Show 20 Lines • Show All 46 Lines • ▼ Show 20 Lines	const chatMessageInfos = useSelector(
messageListData(threadInfo.id, translatedMessageResults),		messageListData(threadInfo.id, translatedMessageResults),
);		);

const sortedUniqueChatMessageInfoItems = React.useMemo(() => {		const sortedUniqueChatMessageInfoItems = React.useMemo(() => {
if (!chatMessageInfos) {		if (!chatMessageInfos) {
return [];		return [];
}		}

const pinnedMessageIDs = new Set();		const pinnedMessageIDs = new Set();
translatedMessageResults.forEach(item => pinnedMessageIDs.add(item.id));		translatedMessageResults.forEach(item => pinnedMessageIDs.add(item.id));
		inkaUnsubmitted Not Done Inline Actions We could probably go back to using `isPinned` property here. The thing we undid in D8097. This would make the code more readable, but it's up to you inka: We could probably go back to using `isPinned` property here. The thing we undid in D8097. This…

const chatMessageInfoItems = chatMessageInfos.filter(		const chatMessageInfoItems = chatMessageInfos.filter(
item =>		item =>
item.itemType === 'message' &&		item.itemType === 'message' &&
pinnedMessageIDs.has(item.messageInfo.id),		pinnedMessageIDs.has(item.messageInfo.id) &&
		isComposableMessageType(item.messageInfo.type),
		rohanAuthorUnsubmitted Done Inline Actions As mentioned in the description, this is because toggle_pin messages are visible robotext, so if we don't filter those out then they will become visible in the message results modal. This is done here, after the `ChatMessageInfo`s have been created, so we can accurately use the updated fetched related messages to determine the `isPinned` flag. If we filtered this earlier, then we'd be in the same situation as before This also is fine because we only allow the user to pin composable message types so we won't exclude anything from the view that we need to show. rohan: As mentioned in the description, this is because toggle_pin messages are visible robotext, so…
);		);

// By the nature of using messageListData and passing in		// By the nature of using messageListData and passing in
// the desired translatedMessageResults as additional		// the desired translatedMessageResults as additional
// messages, we will have duplicate ChatMessageInfoItems.		// messages, we will have duplicate ChatMessageInfoItems.
const uniqueChatMessageInfoItemsMap = new Map();		const uniqueChatMessageInfoItemsMap = new Map();
chatMessageInfoItems.forEach(		chatMessageInfoItems.forEach(
item =>		item =>
▲ Show 20 Lines • Show All 93 Lines • Show Last 20 Lines

[10/n] Native Typeahead - SelectableTextInput for AndroidClosedPublicActions