Details

Reviewers

tomek
inka
rohan

Commits

rCOMM8403333bee9c: [native] Introduce markdown component for chat mentions

Summary

Solution for ENG-4560
This diff creates a new component for chat mentions which handles bolding mentions when user has access to given chat.

Depends on D8844.

Test Plan

Before testing, please apply the entire native stack.

Create a community with three users.
Create a private chat for two chosen users.
Mention private chat in community chat (e. g. @[[256|8282:default text]]).

For users assigned to previously created private chat, the mention should be bold and render current thread name:

For users who are not in private chat, the mention should be rendered with plain text message style:

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

• patryk created this revision.Aug 17 2023, 2:05 AM

• patryk held this revision as a draft.

Herald added subscribers: tomek, ashoat. · View Herald TranscriptAug 17 2023, 2:05 AM

Handle styles

Harbormaster completed remote builds in B21828: Diff 30000.Aug 17 2023, 2:23 AM

Harbormaster completed remote builds in B21829: Diff 30001.Aug 17 2023, 2:28 AM

Fix nit.

Harbormaster completed remote builds in B21838: Diff 30010.Aug 17 2023, 7:20 AM

Match useNavigateToThreadWithFadeAnimation hook code with useAnimatedNavigateToSidebar

Harbormaster completed remote builds in B21886: Diff 30065.Aug 18 2023, 3:28 AM

• patryk edited the summary of this revision. (Show Details)Aug 18 2023, 6:21 AM

• patryk edited the test plan for this revision. (Show Details)

• patryk added reviewers: tomek, inka, rohan.

Refactor useHandleChatMentionClick

Harbormaster completed remote builds in B21891: Diff 30072.Aug 18 2023, 7:22 AM

Rebase

Harbormaster completed remote builds in B21906: Diff 30093.Aug 19 2023, 3:58 AM

Revert rebase

Harbormaster completed remote builds in B21907: Diff 30094.Aug 19 2023, 4:16 AM

• patryk edited the summary of this revision. (Show Details)Aug 19 2023, 9:53 AM

• patryk edited the test plan for this revision. (Show Details)

• patryk added a parent revision: D8844: [native] Refactor markdown-link component.

Remove click handlers from this diff

• patryk edited the summary of this revision. (Show Details)Aug 19 2023, 10:00 AM

• patryk added a child revision: D8873: [native] Make chat mentions clickable.Aug 19 2023, 10:06 AM

Harbormaster completed remote builds in B21915: Diff 30102.Aug 19 2023, 10:18 AM

• patryk added a child revision: D8871: [web][native] Refactor getDefaultTextMessageRules.Aug 19 2023, 3:23 PM

• patryk removed a child revision: D8871: [web][native] Refactor getDefaultTextMessageRules.Aug 19 2023, 3:26 PM

• patryk edited the test plan for this revision. (Show Details)Aug 20 2023, 7:22 AM

• patryk edited the test plan for this revision. (Show Details)

• patryk edited the test plan for this revision. (Show Details)Aug 20 2023, 7:51 AM

• patryk mentioned this in D8846: [native] Use chat mention candidates in markdown rules.Aug 20 2023, 7:55 AM

• patryk mentioned this in D8849: [native] Add chat mention rule to text message rules.Aug 20 2023, 8:01 AM

• patryk attached a referenced file: F697982: t4.png. (Show Details)Aug 20 2023, 11:08 PM

• patryk attached a referenced file: F697980: t3.png. (Show Details)

• patryk published this revision for review.Aug 20 2023, 11:12 PM

Remove style prop

Harbormaster completed remote builds in B21969: Diff 30171.Aug 22 2023, 1:06 AM

Now that you removed the style prop, wouldn't you need to include a bolded style here directly on the Text component? Or are you doing that in a later diff

This revision now requires changes to proceed.Aug 22 2023, 7:15 AM

Accepting after realizing what you're doing in D8849!

This revision is now accepted and ready to land.Aug 22 2023, 7:41 AM

Extract shouldBePressable condition from useMarkdownOnPressUtils

Harbormaster completed remote builds in B22014: Diff 30225.Aug 22 2023, 8:08 AM

inka added inline comments.Aug 28 2023, 4:06 AM

native/markdown/markdown-chat-mention.react.js
12 ↗	(On Diff #30225)	Is this needed?

Also - what makes the text not bold if the user can't access the chat, now that the style prop is passed from the parent?
Oh I see, It's done in the parent. I think it'd be best if we were checking the permissions in one place - can you move it to this component please? Since you are already checking hasAccessToChat here

This revision now requires changes to proceed.Aug 28 2023, 4:09 AM

• patryk added inline comments.Aug 29 2023, 11:39 PM

native/markdown/markdown-chat-mention.react.js
12 ↗	(On Diff #30225)	It is not, will introduce this in the next diff (D8873)!

Remove threadInfo prop

Move style condition to chat mention component

Harbormaster completed remote builds in B22257: Diff 30554.Aug 30 2023, 7:34 AM

Harbormaster completed remote builds in B22258: Diff 30555.Aug 30 2023, 7:52 AM

inka accepted this revision.Aug 31 2023, 7:28 AM

This revision is now accepted and ready to land.Aug 31 2023, 7:28 AM

tomek requested changes to this revision.Sep 12 2023, 1:42 AM

tomek added inline comments.

native/markdown/markdown-chat-mention.react.js
11 ↗	(On Diff #30555)	We aren't using this prop
19–23 ↗	(On Diff #30555)	This `Text` component is empty - shouldn't we render its content?

This revision now requires changes to proceed.Sep 12 2023, 1:42 AM

ashoat added inline comments.Sep 12 2023, 11:11 AM

native/markdown/markdown-chat-mention.react.js
11 ↗	(On Diff #30555)	It's used in `{...rest}` below, and I assume it's specified here to override the optionality in the base (Although I'm not sure if that's overwritten by the `...TextProps` that follows...)
19–23 ↗	(On Diff #30555)	It's not empty, it's being forwarded `props.children` via `{...rest}`

tomek accepted this revision.Sep 13 2023, 2:37 AM

tomek added inline comments.

native/markdown/markdown-chat-mention.react.js
19–23 ↗	(On Diff #30555)	Ok, that makes sense. But I think it would be less confusing if we destructured `children` from `props` and used it explicitly as a child of `Text`.

This revision is now accepted and ready to land.Sep 13 2023, 2:37 AM

Move hasAccessToChat to rules.react.js

Harbormaster completed remote builds in B22859: Diff 31442.Sep 27 2023, 8:21 AM

Closed by commit rCOMM8403333bee9c: [native] Introduce markdown component for chat mentions (authored by • patryk). · Explain WhySep 28 2023, 1:25 AM

This revision was automatically updated to reflect the committed changes.

• patryk added a commit: rCOMM8403333bee9c: [native] Introduce markdown component for chat mentions.

• patryk mentioned this in rCOMM2e88d37a2e83: [native] Use chat mention candidates in markdown rules.Sep 28 2023, 1:36 AM

• patryk mentioned this in rCOMM0acbf97dddc5: [native] Add chat mention rule to text message rules.Sep 28 2023, 1:53 AM

Diff 30065

View Options

native/chat/message-list-types.js

	// @flow			// @flow

	import { HeaderBackButton as BaseHeaderBackButton } from '@react-navigation/elements';			import { HeaderBackButton as BaseHeaderBackButton } from '@react-navigation/elements';
				import invariant from 'invariant';
	import * as React from 'react';			import * as React from 'react';
	import { Text } from 'react-native';			import { Alert, Text } from 'react-native';
	import { TouchableOpacity } from 'react-native-gesture-handler';			import { TouchableOpacity } from 'react-native-gesture-handler';

				import type { NavigationRoute } from '../navigation/route-names';
	import { useColors } from '../themes/colors.js';			import { useColors } from '../themes/colors.js';

	type Props = React.ElementConfig<typeof BaseHeaderBackButton>;			type ChangeRolesHeaderLeftButtonProps = {
	function ChangeRolesHeaderLeftButton(props: Props): React.Node {			+route: NavigationRoute<'ChangeRolesScreen'>,
				...React.ElementConfig<typeof BaseHeaderBackButton>,
				};

				function ChangeRolesHeaderLeftButton(
				props: ChangeRolesHeaderLeftButtonProps,
				): React.Node {
				const { memberInfo, role: selectedRole } = props.route.params;
				const { role: memberRole } = memberInfo;

				const onCancel = React.useCallback(() => {
				const { onPress } = props;
				invariant(onPress, 'onPress must be defined');

				if (selectedRole === memberRole) {
				onPress();
				return;
				}

				Alert.alert(
				'Unsaved Changes',
				'You have unsaved changes, are you sure you want to cancel? You will lose all your progress',
				[
				{ text: 'Cancel', onPress },
				{ text: 'Stay', style: 'cancel' },
				],
				{ cancelable: true },
				);
				}, [memberRole, props, selectedRole]);
				rohanAuthorUnsubmitted Done Inline Actions I think the wording here may be a bit confusing. In my experience, 'cancel' typically means close the alert, and there's another option like 'confirm' or something that will mean you're ok with discarding changes. Here it seems like 'cancel' means we're ok with discarding changes. Not sure if it's just me though (cc @ted) rohan: I think the wording here may be a bit confusing. In my experience, 'cancel' typically means…
				ashoatUnsubmitted Not Done Inline Actions What do we do in the current edit-message workflow? Heads-up I generally handle copy, not @ted ashoat: What do we do in the current edit-message workflow? Heads-up I generally handle copy, not @ted
				rohanAuthorUnsubmitted Done Inline Actions Ah my bad. It looks like in message editing: `Continue editing` closes the alert and continues editing while `Discard edit` closes the alert and exits editing rohan: Ah my bad. It looks like in message editing: `Continue editing` closes the alert and continues…
				ashoatUnsubmitted Not Done Inline Actions I agree we shouldn't use the term `Cancel` here... how about `Leave`? I don't think we need `{ cancelable: true }`... we can just default to `Stay` if the user dismisses the `Alert` Can we try to match copy for title / description with the edit-message workflow? ashoat: 1. I agree we shouldn't use the term `Cancel` here... how about `Leave`? 2. I don't think we…
				rohanAuthorUnsubmitted Done Inline Actions Yeah that makes sense, I'll update the diff rohan: Yeah that makes sense, I'll update the diff

	const { panelForegroundSecondaryLabel } = useColors();			const { panelForegroundSecondaryLabel } = useColors();
	const labelStyle = React.useMemo(			const labelStyle = React.useMemo(
	() => ({			() => ({
	color: panelForegroundSecondaryLabel,			color: panelForegroundSecondaryLabel,
	marginLeft: 10,			marginLeft: 10,
	}),			}),
	[panelForegroundSecondaryLabel],			[panelForegroundSecondaryLabel],
	);			);

	return (			return (
	<TouchableOpacity onPress={props.onPress}>			<TouchableOpacity onPress={onCancel}>
	<Text style={labelStyle}>Cancel</Text>			<Text style={labelStyle}>Cancel</Text>
	</TouchableOpacity>			</TouchableOpacity>
	);			);
	}			}

	export default ChangeRolesHeaderLeftButton;			export default ChangeRolesHeaderLeftButton;

View Options

native/markdown/markdown-chat-mention.react.js

	// @flow			// @flow

	import { HeaderBackButton as BaseHeaderBackButton } from '@react-navigation/elements';			import { HeaderBackButton as BaseHeaderBackButton } from '@react-navigation/elements';
				import invariant from 'invariant';
	import * as React from 'react';			import * as React from 'react';
	import { Text } from 'react-native';			import { Alert, Text } from 'react-native';
	import { TouchableOpacity } from 'react-native-gesture-handler';			import { TouchableOpacity } from 'react-native-gesture-handler';

				import type { NavigationRoute } from '../navigation/route-names';
	import { useColors } from '../themes/colors.js';			import { useColors } from '../themes/colors.js';

	type Props = React.ElementConfig<typeof BaseHeaderBackButton>;			type ChangeRolesHeaderLeftButtonProps = {
	function ChangeRolesHeaderLeftButton(props: Props): React.Node {			+route: NavigationRoute<'ChangeRolesScreen'>,
				...React.ElementConfig<typeof BaseHeaderBackButton>,
				};

				function ChangeRolesHeaderLeftButton(
				props: ChangeRolesHeaderLeftButtonProps,
				): React.Node {
				const { memberInfo, role: selectedRole } = props.route.params;
				const { role: memberRole } = memberInfo;

				const onCancel = React.useCallback(() => {
				const { onPress } = props;
				invariant(onPress, 'onPress must be defined');

				if (selectedRole === memberRole) {
				onPress();
				return;
				}

				Alert.alert(
				'Unsaved Changes',
				'You have unsaved changes, are you sure you want to cancel? You will lose all your progress',
				[
				{ text: 'Cancel', onPress },
				{ text: 'Stay', style: 'cancel' },
				],
				{ cancelable: true },
				);
				}, [memberRole, props, selectedRole]);
				rohanAuthorUnsubmitted Done Inline Actions I think the wording here may be a bit confusing. In my experience, 'cancel' typically means close the alert, and there's another option like 'confirm' or something that will mean you're ok with discarding changes. Here it seems like 'cancel' means we're ok with discarding changes. Not sure if it's just me though (cc @ted) rohan: I think the wording here may be a bit confusing. In my experience, 'cancel' typically means…
				ashoatUnsubmitted Not Done Inline Actions What do we do in the current edit-message workflow? Heads-up I generally handle copy, not @ted ashoat: What do we do in the current edit-message workflow? Heads-up I generally handle copy, not @ted
				rohanAuthorUnsubmitted Done Inline Actions Ah my bad. It looks like in message editing: `Continue editing` closes the alert and continues editing while `Discard edit` closes the alert and exits editing rohan: Ah my bad. It looks like in message editing: `Continue editing` closes the alert and continues…
				ashoatUnsubmitted Not Done Inline Actions I agree we shouldn't use the term `Cancel` here... how about `Leave`? I don't think we need `{ cancelable: true }`... we can just default to `Stay` if the user dismisses the `Alert` Can we try to match copy for title / description with the edit-message workflow? ashoat: 1. I agree we shouldn't use the term `Cancel` here... how about `Leave`? 2. I don't think we…
				rohanAuthorUnsubmitted Done Inline Actions Yeah that makes sense, I'll update the diff rohan: Yeah that makes sense, I'll update the diff

	const { panelForegroundSecondaryLabel } = useColors();			const { panelForegroundSecondaryLabel } = useColors();
	const labelStyle = React.useMemo(			const labelStyle = React.useMemo(
	() => ({			() => ({
	color: panelForegroundSecondaryLabel,			color: panelForegroundSecondaryLabel,
	marginLeft: 10,			marginLeft: 10,
	}),			}),
	[panelForegroundSecondaryLabel],			[panelForegroundSecondaryLabel],
	);			);

	return (			return (
	<TouchableOpacity onPress={props.onPress}>			<TouchableOpacity onPress={onCancel}>
	<Text style={labelStyle}>Cancel</Text>			<Text style={labelStyle}>Cancel</Text>
	</TouchableOpacity>			</TouchableOpacity>
	);			);
	}			}

	export default ChangeRolesHeaderLeftButton;			export default ChangeRolesHeaderLeftButton;

View Options

native/markdown/markdown-utils.js

	// @flow			// @flow

	import { HeaderBackButton as BaseHeaderBackButton } from '@react-navigation/elements';			import { HeaderBackButton as BaseHeaderBackButton } from '@react-navigation/elements';
				import invariant from 'invariant';
	import * as React from 'react';			import * as React from 'react';
	import { Text } from 'react-native';			import { Alert, Text } from 'react-native';
	import { TouchableOpacity } from 'react-native-gesture-handler';			import { TouchableOpacity } from 'react-native-gesture-handler';

				import type { NavigationRoute } from '../navigation/route-names';
	import { useColors } from '../themes/colors.js';			import { useColors } from '../themes/colors.js';

	type Props = React.ElementConfig<typeof BaseHeaderBackButton>;			type ChangeRolesHeaderLeftButtonProps = {
	function ChangeRolesHeaderLeftButton(props: Props): React.Node {			+route: NavigationRoute<'ChangeRolesScreen'>,
				...React.ElementConfig<typeof BaseHeaderBackButton>,
				};

				function ChangeRolesHeaderLeftButton(
				props: ChangeRolesHeaderLeftButtonProps,
				): React.Node {
				const { memberInfo, role: selectedRole } = props.route.params;
				const { role: memberRole } = memberInfo;

				const onCancel = React.useCallback(() => {
				const { onPress } = props;
				invariant(onPress, 'onPress must be defined');

				if (selectedRole === memberRole) {
				onPress();
				return;
				}

				Alert.alert(
				'Unsaved Changes',
				'You have unsaved changes, are you sure you want to cancel? You will lose all your progress',
				[
				{ text: 'Cancel', onPress },
				{ text: 'Stay', style: 'cancel' },
				],
				{ cancelable: true },
				);
				}, [memberRole, props, selectedRole]);
				rohanAuthorUnsubmitted Done Inline Actions I think the wording here may be a bit confusing. In my experience, 'cancel' typically means close the alert, and there's another option like 'confirm' or something that will mean you're ok with discarding changes. Here it seems like 'cancel' means we're ok with discarding changes. Not sure if it's just me though (cc @ted) rohan: I think the wording here may be a bit confusing. In my experience, 'cancel' typically means…
				ashoatUnsubmitted Not Done Inline Actions What do we do in the current edit-message workflow? Heads-up I generally handle copy, not @ted ashoat: What do we do in the current edit-message workflow? Heads-up I generally handle copy, not @ted
				rohanAuthorUnsubmitted Done Inline Actions Ah my bad. It looks like in message editing: `Continue editing` closes the alert and continues editing while `Discard edit` closes the alert and exits editing rohan: Ah my bad. It looks like in message editing: `Continue editing` closes the alert and continues…
				ashoatUnsubmitted Not Done Inline Actions I agree we shouldn't use the term `Cancel` here... how about `Leave`? I don't think we need `{ cancelable: true }`... we can just default to `Stay` if the user dismisses the `Alert` Can we try to match copy for title / description with the edit-message workflow? ashoat: 1. I agree we shouldn't use the term `Cancel` here... how about `Leave`? 2. I don't think we…
				rohanAuthorUnsubmitted Done Inline Actions Yeah that makes sense, I'll update the diff rohan: Yeah that makes sense, I'll update the diff

	const { panelForegroundSecondaryLabel } = useColors();			const { panelForegroundSecondaryLabel } = useColors();
	const labelStyle = React.useMemo(			const labelStyle = React.useMemo(
	() => ({			() => ({
	color: panelForegroundSecondaryLabel,			color: panelForegroundSecondaryLabel,
	marginLeft: 10,			marginLeft: 10,
	}),			}),
	[panelForegroundSecondaryLabel],			[panelForegroundSecondaryLabel],
	);			);

	return (			return (
	<TouchableOpacity onPress={props.onPress}>			<TouchableOpacity onPress={onCancel}>
	<Text style={labelStyle}>Cancel</Text>			<Text style={labelStyle}>Cancel</Text>
	</TouchableOpacity>			</TouchableOpacity>
	);			);
	}			}

	export default ChangeRolesHeaderLeftButton;			export default ChangeRolesHeaderLeftButton;

[native] Introduce markdown component for chat mentions
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 30065

native/chat/message-list-types.js

native/markdown/markdown-chat-mention.react.js

native/markdown/markdown-utils.js

[native] Introduce markdown component for chat mentionsClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 30065

native/chat/message-list-types.js

native/markdown/markdown-chat-mention.react.js

native/markdown/markdown-utils.js

[native] Introduce markdown component for chat mentions
ClosedPublic
Actions

Revision Contents
Changeset List