Page MenuHomePhabricator
Differential D6318

[web] implement new InlineEngagement design
ClosedPublic
Actions

Authored by ginsu on Jan 19 2023, 9:29 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Jun 19, 7:07 PM
Unknown Object (File)
Fri, Jun 14, 2:23 AM
Unknown Object (File)
Thu, Jun 13, 3:11 PM
Unknown Object (File)
Thu, Jun 13, 11:59 AM
Unknown Object (File)
Wed, Jun 12, 10:29 PM
Unknown Object (File)
Tue, Jun 11, 11:12 PM
Unknown Object (File)
Fri, Jun 7, 7:17 AM
Unknown Object (File)
Fri, Jun 7, 6:33 AM
View All 77 Files
Subscribers
ashoat
atul
tomek

Details

Reviewers
atul
tomek
Commits
rCOMM1401e536a93c: [web] implement new InlineEngagement design
Summary

implemented new InlineEngagement design. The new design splits the InlineEngagement into two seperate parts

Screenshot 2023-01-19 at 5.50.52 PM.png (1×1 px, 139 KB)

Depends on D6317 and D6302
Linear Task: ENG-2720

Test Plan

Please watch the demo video to see the new InlineEngagement

Diff Detail

Repository
rCOMM Comm
Branch
eng-2465 (branched from master)
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

ginsu created this revision.Jan 19 2023, 9:29 PM
Herald added subscribers: atul, tomek, ashoat. · View Herald TranscriptJan 19 2023, 9:29 PM
ginsu edited the summary of this revision. (Show Details)Jan 19 2023, 9:32 PM
ginsu edited the test plan for this revision. (Show Details)
ginsu added reviewers: atul, tomek.
ginsu added a parent revision: D6317: [lib] remove the word reply/replies from InlineEngagement.
ginsu attached a referenced file: F335515: Screen Recording 2023-01-19 at 11.53.46 PM.mov. (Show Details)
ginsu attached a referenced file: F335514: Screenshot 2023-01-19 at 5.50.52 PM.png. (Show Details)
Harbormaster completed remote builds in B15489: Diff 21129.Jan 19 2023, 9:43 PM
ginsu requested review of this revision.Jan 19 2023, 9:43 PM
ginsu updated this revision to Diff 21207.Jan 23 2023, 9:47 AM

rebase

Harbormaster completed remote builds in B15546: Diff 21207.Jan 23 2023, 9:59 AM
ginsu added a child revision: D6348: [web/lib] put logic for creating message reaction list in reaction utils.Jan 23 2023, 10:04 AM
ginsu edited the summary of this revision. (Show Details)Jan 23 2023, 10:06 AM
ginsu added a parent revision: D6302: [web/lib] introduce MessageReactionsModal.
atul accepted this revision.Jan 24 2023, 10:14 AM

Looks good, minor notes inline.

web/chat/inline-engagement.react.js
27 ↗(On Diff #21207)

Do we really need this? Are we not able to check the truthiness of ThreadInfo directly in threadsContainerClasses and reactionsContainerClasses?

28 ↗(On Diff #21207)

Could we replace this with

const reactionsExist = reactions?.size > 0;

?

Could we also move threadInfoExists and reactionsExist directly above threadsContainerClasses and reactionsContainerClasses so they're closer to usage?

This revision is now accepted and ready to land.Jan 24 2023, 10:14 AM
ginsu added inline comments.Jan 24 2023, 12:30 PM
web/chat/inline-engagement.react.js
28 ↗(On Diff #21207)

Getting a flow error about cannot comparing undefined to number when attempting to replace reactionsExist with the statement you provided

ginsu updated this revision to Diff 21262.Jan 24 2023, 12:35 PM

address atuls comments

atul added inline comments.Jan 24 2023, 12:35 PM
web/chat/inline-engagement.react.js
28 ↗(On Diff #21207)

Oh yeah that makes sense never mind

Harbormaster completed remote builds in B15596: Diff 21262.Jan 24 2023, 12:50 PM
Closed by commit rCOMM1401e536a93c: [web] implement new InlineEngagement design (authored by ginsu). · Explain WhyJan 24 2023, 1:18 PM
This revision was automatically updated to reflect the committed changes.
ginsu added a commit: rCOMM1401e536a93c: [web] implement new InlineEngagement design.

Revision Contents
Changeset List

PathSize
web/
chat/
43 lines
70 lines

Diff 21129

View Options

web/chat/inline-engagement.css

Loading...
View Options

web/chat/inline-engagement.react.js

Loading...