[native] Displaying 'Edit' labels next to edited messages
ClosedPublic
Actions

Authored by • kuba on Mar 14 2023, 1:56 AM.

Details

Reviewers

michal
inka
ginsu

Commits

rCOMM8378e0da47ca: [native] Displaying 'Edit' labels next to edited messages

Summary

Added displaying 'Edit' label next to edited messages.

Test Plan

message not edited and without thread/reactions,
message edited without thread/reactions,
message not edited with thread/reactions,
message edited with thread,
message edited with reactions,
message edited with thread and reactions,
many edited messages in a row,

Screenshots:

Screenshot 2023-03-22 at 21.08.44.png (850×1 px, 321 KB)

Screenshot 2023-03-22 at 21.08.57.png (374×1 px, 171 KB)

Made sure on iOS that no Message height warnings are displayed in the console.

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

• kuba created this revision.Mar 14 2023, 1:56 AM

Herald added subscribers: atul, tomek, ashoat. · View Herald TranscriptMar 14 2023, 1:56 AM

• kuba added a parent revision: D7065: [native] Added displaying labels to InlineEngagement.Mar 14 2023, 1:56 AM

Harbormaster completed remote builds in B17312: Diff 23711.Mar 14 2023, 2:10 AM

• kuba requested review of this revision.Mar 14 2023, 2:10 AM

• kuba added a child revision: D7127: [lib][keyserver] Introduce new threadPermissions types for editing messages.Mar 21 2023, 11:22 AM

• kuba edited the test plan for this revision. (Show Details)Mar 22 2023, 1:09 PM

Added messageHeight of edited messages

• kuba mentioned this in D7065: [native] Added displaying labels to InlineEngagement.Mar 28 2023, 12:48 AM

• kuba added inline comments.

native/chat/utils.js
77	If we already display threads/reactions container, the height is not changed.

Harbormaster completed remote builds in B17724: Diff 24259.Mar 28 2023, 1:02 AM

Can you amend the Test Plan to include confirming that no warnings like this are presented: https://github.com/CommE2E/comm/blob/2901fdbf2fe11c8564c13fa3fe2513b8a05f5e29/native/chat/message.react.js#L118-L125

What designs are you using? The designs I found (here) show the label next to the message.
I also see no comments regarding this, in what I guess is the task this diff is addressing (here)

In D7066#214557, @inka wrote:

What designs are you using? The designs I found (here) show the label next to the message.
I also see no comments regarding this, in what I guess is the task this diff is addressing (here)

The designs that I used are here: https://linear.app/comm/issue/DES-38#comment-3d7826f4. They cover also displaying both the Edited label and threads/reactions icons.
Added link to that linear task.

Thank you for adding the link to the task. The DES task you linked is for Desktop though, and this is Native. Was it decided anywhere that we should use Desktop designs for Native?

In D7066#214942, @inka wrote:

Thank you for adding the link to the task. The DES task you linked is for Desktop though, and this is Native. Was it decided anywhere that we should use Desktop designs for Native?

Thank you for checking it!
You are right. I have used designs from this issue, and matched styling with the web version.

I prepared changes to match the other designs. I asked Ted about what is the final decision.

In D7066#214942, @inka wrote:

Thank you for adding the link to the task. The DES task you linked is for Desktop though, and this is Native. Was it decided anywhere that we should use Desktop designs for Native?

Ted confirmed that we stay with the currently implemented design: https://linear.app/comm/issue/DES-39#comment-58e3dea0

Update test plan: https://phab.comm.dev/D7066#214478