[native] Block editing messages on search and pinned messages screens, and of the first message in a thread
Needs ReviewPublic
Actions

Authored by inka on Thu, Sep 7, 1:35 AM.

Details

Reviewers

rohan
ginsu
atul
tomek

Summary

issues: https://linear.app/comm/issue/ENG-4834/editing-sidebar-source-message-behaves-weirdly#comment-e38584c8
https://linear.app/comm/issue/ENG-4847/not-possible-to-pin-the-source-messge#comment-9971bb16
This is a bit of a hacky solution to prevent editing in search, pinned messages and of the first message in a thread. The first two cases will be resolved by https://linear.app/comm/issue/ENG-3172/implement-navigating-to-the-message-picked-from-the-search-screen, where we will implement navigating to a message. So that hack will be reverted. The third one looks like a bug, and we will probably want to investigate it at some point, and fix it.

On web everything works correctly, so I left it for now

Test Plan

checked that "edit" is not present in tooltip in search results, pinned messages screen and for the first message in a thread. Checked that it is present for a message in parent thread from which the thread was created, as well as other messages in the thread and parent chat.

Diff Detail

Repository

rCOMM Comm

Branch

inka/edit_fix

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

inka created this revision.Thu, Sep 7, 1:35 AM

Herald added a subscriber: ashoat. · View Herald TranscriptThu, Sep 7, 1:35 AM

inka edited the summary of this revision. (Show Details)Thu, Sep 7, 1:36 AM

inka edited the summary of this revision. (Show Details)Thu, Sep 7, 1:45 AM

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

Harbormaster completed remote builds in B22421: Diff 30825.Thu, Sep 7, 1:54 AM

inka requested review of this revision.Thu, Sep 7, 1:54 AM

tomek added inline comments.Thu, Sep 7, 2:34 AM

native/chat/text-message-tooltip-modal.react.js
134–135 ↗	(On Diff #30825)	This is really hacky. At least, we should use a constant defined somewhere, e.g. `MessageSearchRouteName`. Ideally, a screen should tell the tooltip if the message should be editable. E.g. as a parameter to `createTooltip` function, or through `TooltipContext`... do you think we can do something like this?

Requesting changes for feedback above

This revision now requires changes to proceed.Thu, Sep 14, 9:19 AM

Use constant. Move logic to ConnectedTextMessage, where other conditions for whether a message can be edited are checked, and a canEditMessage value used to create visibleEntryIDs which are used by TooltipContext to determine whether the item should be rendered, is created.

Harbormaster completed remote builds in B22756: Diff 31308.Wed, Sep 20, 4:06 AM

tomek accepted this revision.Wed, Sep 20, 5:42 AM

tomek added inline comments.