Page MenuHomePhabricator
Differential D14544

[web] Display a deleted message on web
ClosedPublic
Actions

Authored by tomek on Fri, Apr 4, 4:15 AM.
Tags
None
Referenced Files
F5764418: D14544.id.diff
Mon, Apr 14, 3:40 PM
F5762323: D14544.id47724.diff
Mon, Apr 14, 2:41 PM
F5727446: D14544.id47655.diff
Mon, Apr 14, 8:46 AM
Unknown Object (File)
Mon, Apr 14, 1:22 AM
Unknown Object (File)
Sun, Apr 13, 11:34 AM
Unknown Object (File)
Sun, Apr 13, 6:32 AM
Unknown Object (File)
Fri, Apr 11, 12:31 AM
Unknown Object (File)
Thu, Apr 10, 4:27 AM
View All 26 Files
Subscribers
None

Details

Reviewers
kamil
ashoat
Commits
rCOMMe1027e8f6c24: [web] Display a deleted message on web
Summary

Used a color that seems to be distinctive enough both from viewer's and others messages.

https://linear.app/comm/issue/ENG-10346/replace-deleted-messages-by-a-text-message

web-deleted-2.png (1×890 px, 106 KB)

Depends on D14543

Test Plan

Checked if deleted messages on web are displayed as deleted.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

tomek created this revision.Fri, Apr 4, 4:15 AM
tomek updated this revision to Diff 47656.Fri, Apr 4, 4:18 AM

Simplify

tomek edited the summary of this revision. (Show Details)Fri, Apr 4, 4:18 AM

Because of https://linear.app/comm/issue/ENG-10516/light-mode-seems-to-be-broken-on-web I wasn't able to test this properly in light mode.

Harbormaster completed remote builds in B33903: Diff 47655.Fri, Apr 4, 4:34 AM
Harbormaster completed remote builds in B33904: Diff 47656.
tomek requested review of this revision.Fri, Apr 4, 4:36 AM
tomek added a child revision: D14545: [native] Display a deleted message on native.Fri, Apr 4, 4:38 AM
ashoat requested changes to this revision.Fri, Apr 4, 8:49 AM

The white background feels like too strong of a contrast... I feel like a deleted message should be less visible than a non-deleted message. Could we try something closer to what Rahul had in his designs?

web/chat/deleted-message.react.js
40 ↗(On Diff #47656)

Do we really need to render Markdown here just to italicize some text?

This revision now requires changes to proceed.Fri, Apr 4, 8:49 AM
ashoat mentioned this in D14545: [native] Display a deleted message on native.Fri, Apr 4, 8:49 AM
tomek updated this revision to Diff 47724.Wed, Apr 9, 10:06 AM

Simplify. Update the style.

tomek edited the summary of this revision. (Show Details)Wed, Apr 9, 10:06 AM
In D14544#403424, @ashoat wrote:

The white background feels like too strong of a contrast... I feel like a deleted message should be less visible than a non-deleted message. Could we try something closer to what Rahul had in his designs?

Updated the design

web-deleted-2.png (1×890 px, 106 KB)

web/chat/deleted-message.react.js
40 ↗(On Diff #47656)

It can be simplified - replaced with <i>

Harbormaster completed remote builds in B33942: Diff 47724.Wed, Apr 9, 10:23 AM
ashoat accepted this revision.Wed, Apr 9, 8:04 PM
This revision is now accepted and ready to land.Wed, Apr 9, 8:04 PM
Closed by commit rCOMMe1027e8f6c24: [web] Display a deleted message on web (authored by tomek). · Explain WhyTue, Apr 15, 2:11 AM
This revision was automatically updated to reflect the committed changes.
tomek added a commit: rCOMMe1027e8f6c24: [web] Display a deleted message on web.

Revision Contents
Changeset List

Diff 47790

View Options

web/chat/chat-message-list.css

Loading...
View Options

web/chat/deleted-message.react.js

Loading...
View Options

web/chat/message.react.js

Loading...
View Options

web/theme.css

Loading...