Page MenuHomePhabricator
Differential D5833

[web] Add unread indicator to sidebar modals
ClosedPublic
Actions

Authored by • adar on Dec 6 2022, 7:37 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Apr 24, 12:35 PM
Unknown Object (File)
Thu, Apr 24, 11:07 AM
Unknown Object (File)
Thu, Apr 24, 3:26 AM
Unknown Object (File)
Wed, Apr 23, 9:32 PM
Unknown Object (File)
Wed, Apr 23, 5:50 AM
Unknown Object (File)
Wed, Apr 23, 12:31 AM
Unknown Object (File)
Tue, Apr 22, 1:16 PM
Unknown Object (File)
Fri, Apr 18, 12:44 AM
View All Files
Subscribers
ashoat
tomek

Details

Reviewers
atul
ginsu
rohan
Commits
rCOMM3482588d8bf6: [web] Add unread indicator to sidebar modals
Summary

This is the second part of ENG-2089. This diff adds the basic unread indicators to the sidebar modal. Changes font color and font weight of the last unread message.

Test Plan

Screenshots that show that unread thread messages appear unread in the sidebar modal in both 'All Threads' and 'My Threads' tabs.

Unread thread messageRead thread message
Screen Shot 2022-12-07 at 2.29.54 PM.png (2×4 px, 597 KB)
Screen Shot 2022-12-07 at 2.30.11 PM.png (2×4 px, 529 KB)
Screen Shot 2022-12-07 at 2.30.01 PM.png (1×832 px, 73 KB)
Screen Shot 2022-12-07 at 2.30.06 PM.png (1×836 px, 74 KB)
Screen Shot 2022-12-07 at 2.30.17 PM.png (1×826 px, 72 KB)
Screen Shot 2022-12-07 at 2.30.20 PM.png (1×830 px, 72 KB)

Diff Detail

Repository
rCOMM Comm
Branch
master
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

• adar created this revision.Dec 6 2022, 7:37 AM
• adar held this revision as a draft.
Herald added subscribers: tomek, ashoat. · View Herald TranscriptDec 6 2022, 7:37 AM
• adar edited the test plan for this revision. (Show Details)Dec 6 2022, 7:40 AM
• adar added inline comments.Dec 6 2022, 7:43 AM
web/modals/threads/sidebars/sidebar.react.js
72

note that eslint complained when lastActivityClassName and longTextEllipsisClassName are not including in the dependency array, but since they're just strings derived from classNames i don't think they need to be in here see (D5697)

Harbormaster completed remote builds in B14016: Diff 19191.Dec 6 2022, 7:49 AM
• adar published this revision for review.Dec 6 2022, 8:10 AM
atul requested changes to this revision.Dec 6 2022, 10:51 AM

Should we also be styling the "new thread" in the example screenshot you posted?

web/modals/threads/sidebars/sidebar.react.js
72

Even though they're strings, I think lastMessage will still need to be recomputed if they change

This revision now requires changes to proceed.Dec 6 2022, 10:51 AM
• adar added a comment.Dec 6 2022, 11:02 AM
In D5833#174057, @atul wrote:

Should we also be styling the "new thread" in the example screenshot you posted?

Can do!

• adar updated this revision to Diff 19232.Dec 7 2022, 11:34 AM
• adar edited the test plan for this revision. (Show Details)

make thread name appear bold as well

• adar edited the test plan for this revision. (Show Details)Dec 7 2022, 11:36 AM
Harbormaster failed remote builds in B14048: Diff 19232!Dec 7 2022, 11:45 AM
Harbormaster completed remote builds in B14048: Diff 19232.Dec 7 2022, 11:50 AM
atul accepted this revision.Dec 7 2022, 12:10 PM

Looks good!

(and appreciate that you updated the screenshots to match!)

This revision is now accepted and ready to land.Dec 7 2022, 12:10 PM
Closed by commit rCOMM3482588d8bf6: [web] Add unread indicator to sidebar modals (authored by • adar). · Explain WhyDec 7 2022, 12:21 PM
This revision was automatically updated to reflect the committed changes.
• adar added a commit: rCOMM3482588d8bf6: [web] Add unread indicator to sidebar modals.

Revision Contents
Changeset List

PathSize
web/
modals/
threads/
sidebars/
25 lines
5 lines

Diff 19191

View Options

web/modals/threads/sidebars/sidebar.react.js

Show First 20 Lines • Show All 69 Lines • ▼ Show 20 Lines if (!isViewer && startsCluster) {
height += authorNameHeight; height += authorNameHeight;
} }
if (endsCluster) { if (endsCluster) {
height += clusterEndHeight; height += clusterEndHeight;
} }
if (textMessageSendFailed(item)) { if (textMessageSendFailed(item)) {
height += failedSendHeight; height += failedSendHeight;
} }
const label = getMessageLabel(item.hasBeenEdited, threadInfo); const label = getMessageLabel(item.hasBeenEdited, threadInfo.id);
if (item.threadCreatedFromMessage || Object.keys(item.reactions).length > 0) { if (item.threadCreatedFromMessage || Object.keys(item.reactions).length > 0) {
height += height +=
inlineEngagementStyle.height + inlineEngagementStyle.height +
inlineEngagementStyle.marginTop + inlineEngagementStyle.marginTop +
inlineEngagementStyle.marginBottom; inlineEngagementStyle.marginBottom;
} else if (label) { } else if (label) {
height += height +=
inlineEngagementLabelStyle.height + inlineEngagementLabelStyle.height +
▲ Show 20 Lines • Show All 367 Lines • Show Last 20 Lines
View Options

web/modals/threads/sidebars/sidebars-modal.css

Show First 20 Lines • Show All 69 Lines • ▼ Show 20 Lines if (!isViewer && startsCluster) {
height += authorNameHeight; height += authorNameHeight;
} }
if (endsCluster) { if (endsCluster) {
height += clusterEndHeight; height += clusterEndHeight;
} }
if (textMessageSendFailed(item)) { if (textMessageSendFailed(item)) {
height += failedSendHeight; height += failedSendHeight;
} }
const label = getMessageLabel(item.hasBeenEdited, threadInfo); const label = getMessageLabel(item.hasBeenEdited, threadInfo.id);
if (item.threadCreatedFromMessage || Object.keys(item.reactions).length > 0) { if (item.threadCreatedFromMessage || Object.keys(item.reactions).length > 0) {
height += height +=
inlineEngagementStyle.height + inlineEngagementStyle.height +
inlineEngagementStyle.marginTop + inlineEngagementStyle.marginTop +
inlineEngagementStyle.marginBottom; inlineEngagementStyle.marginBottom;
} else if (label) { } else if (label) {
height += height +=
inlineEngagementLabelStyle.height + inlineEngagementLabelStyle.height +
▲ Show 20 Lines • Show All 367 Lines • Show Last 20 Lines