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
F3489886: D6318.diff
Wed, Dec 18, 1:59 PM
Unknown Object (File)
Nov 12 2024, 12:23 AM
Unknown Object (File)
Nov 12 2024, 12:23 AM
Unknown Object (File)
Nov 12 2024, 12:23 AM
Unknown Object (File)
Nov 12 2024, 12:23 AM
Unknown Object (File)
Nov 5 2024, 5:04 AM
Unknown Object (File)
Nov 1 2024, 6:18 PM
Unknown Object (File)
Nov 1 2024, 6:17 PM
View All 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
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
69 lines

Diff 21262

View Options

web/chat/inline-engagement.css

Loading...
View Options

web/chat/inline-engagement.react.js

Loading...