[GAL-4417] Mobile mention #1992
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Bundle SizesCompared against 68b13ed Route: No significant changes found Dynamic import: No significant changes found |
…z/mobile-mention
| ); | ||
| }; | ||
|
|
||
| type CommentProps = { |
There was a problem hiding this comment.
lets rename the type too to match the component name if it's no longer just for comments
| mentionsRef | ||
| ); | ||
|
|
||
| const processedText = useMemo(() => { |
There was a problem hiding this comment.
would be awesome to have tests for this logic
| ); | ||
|
|
||
| const processedText = useMemo(() => { | ||
| const elements: CommentElement[] = []; |
There was a problem hiding this comment.
I think decomposing this into different helper functions could help make this easier to read. it could also make debugging the issue from slack a bit easier
| <WarningLinkBottomSheet redirectUrl={redirectUrl} ref={bottomSheetRef} /> | ||
| <ProcessedText | ||
| text={captionWithMarkdownLinks} | ||
| mentionsRef={removeNullValues(feedPost.mentions)} |
There was a problem hiding this comment.
I think removeNullValues is worth memoizing
| useEffect(() => { | ||
| if (activeCommentId && ref.current) { | ||
| const index = comments.findIndex((comment) => comment.dbid === activeCommentId); | ||
| if (index !== -1) { | ||
| setTimeout(() => { | ||
| if (!ref.current) { | ||
| return; | ||
| } | ||
| ref.current.scrollToIndex({ index, animated: true }); | ||
| }, 200); | ||
| } | ||
| } | ||
| }, [activeCommentId, comments]); |
| mentionsRef | ||
| ); | ||
|
|
||
| const processedText = useMemo(() => { |
|
|
||
| const navigation = useNavigation<MainTabStackNavigatorProp>(); | ||
|
|
||
| const noficationData: NotificationDataType = useMemo(() => { |
| } | ||
|
|
||
| // Calculate the length difference between the old alias and the new mention | ||
| const lengthDifference = mention.label.length + 2 - aliasKeyword.length; // +1 for the @ |
There was a problem hiding this comment.
+1 or +2 in your comment?
There was a problem hiding this comment.
Updated the comment!
It should be + 2
| [mentions, message, setMessage, aliasKeyword, selection.start] | ||
| ); | ||
|
|
||
| const handleSetMessage = useCallback( |
There was a problem hiding this comment.
imo i think the logic in handleSetMention and handleSetMessage should be unit tested
There was a problem hiding this comment.
Agree! Will write the unit test for this hooks
| @@ -0,0 +1,123 @@ | |||
| import { graphql, readInlineData } from 'react-relay'; | |||
Summary of Changes
ProcessedTextcomponent to supportplain link,markdown linkandmentionsDemo or Before/After Pics
Full flow mentions
CleanShot.2023-10-10.at.12.15.49.mp4
Notification metioned from comment
CleanShot.2023-10-10.at.12.21.38.mp4
Notification mentioned from post
CleanShot.2023-10-10.at.12.22.08.mp4
ProcessedText on comment
CleanShot.2023-10-10.at.12.22.41.mp4
ProcessedText on post
CleanShot.2023-10-10.at.12.23.43.mp4
Edge Cases
Testing Steps
Checklist
Please make sure to review and check all of the following: