Page MenuHomePhabricator

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

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



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

rCOMM Comm
No Lint Coverage
No Test Coverage

Event Timeline

inka edited the test plan for this revision. (Show Details)
inka requested review of this revision.Thu, Sep 7, 1:54 AM
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?

atul requested changes to this revision.Thu, Sep 14, 9:19 AM

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.

tomek added inline comments.

How about renaming it?


What do you think about checking screen name inside this hook? (should be possible using nav hooks)