Page MenuHomePhabricator

[native] Introduce markdown component for chat mentions
ClosedPublic

Authored by patryk on Aug 17 2023, 2:05 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Dec 22, 12:16 PM
Unknown Object (File)
Nov 12 2024, 1:52 AM
Unknown Object (File)
Nov 11 2024, 1:50 AM
Unknown Object (File)
Nov 10 2024, 7:43 PM
Unknown Object (File)
Nov 10 2024, 6:06 AM
Unknown Object (File)
Oct 27 2024, 2:37 PM
Unknown Object (File)
Oct 27 2024, 2:37 PM
Unknown Object (File)
Oct 27 2024, 2:37 PM
Subscribers

Details

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.

  1. Create a community with three users.
  2. Create a private chat for two chosen users.
  3. 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:

t4.png (1×794 px, 522 KB)

For users who are not in private chat, the mention should be rendered with plain text message style:
t3.png (1×794 px, 520 KB)

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

patryk held this revision as a draft.

Match useNavigateToThreadWithFadeAnimation hook code with useAnimatedNavigateToSidebar

patryk edited the test plan for this revision. (Show Details)
patryk added reviewers: tomek, inka, rohan.

Refactor useHandleChatMentionClick

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

Remove click handlers from this diff

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

Remove style prop

rohan requested changes to this revision.Aug 22 2023, 7:15 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

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

Is this needed?

inka requested changes to this revision.EditedAug 28 2023, 4:09 AM

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
native/markdown/markdown-chat-mention.react.js
12

It is not, will introduce this in the next diff (D8873)!

Move style condition to chat mention component

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
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 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