Paths

Table of Contentst

Differential D6318

[web] implement new InlineEngagement design
ClosedPublic
Actions

Authored by ginsu on Jan 20 2023, 5:29 AM.

Tags

None

Referenced Files

	F335515: Screen Recording 2023-01-19 at 11.53.46 PM.mov
	Jan 20 2023, 5:32 AM

	F335514: Screenshot 2023-01-19 at 5.50.52 PM.png
	Jan 20 2023, 5:32 AM

Subscribers

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

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

ginsu created this revision.Jan 20 2023, 5:29 AM

Herald added subscribers: atul, tomek, ashoat. · View Herald TranscriptJan 20 2023, 5:29 AM

ginsu edited the summary of this revision. (Show Details)Jan 20 2023, 5:32 AM

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 20 2023, 5:43 AM

ginsu requested review of this revision.Jan 20 2023, 5:43 AM

rebase

Harbormaster completed remote builds in B15546: Diff 21207.Jan 23 2023, 5:59 PM

ginsu added a child revision: D6348: [web/lib] put logic for creating message reaction list in reaction utils.Jan 23 2023, 6:04 PM

ginsu edited the summary of this revision. (Show Details)Jan 23 2023, 6:06 PM

ginsu added a parent revision: D6302: [web/lib] introduce MessageReactionsModal.

Looks good, minor notes inline.

web/chat/inline-engagement.react.js
27	Do we really need this? Are we not able to check the truthiness of `ThreadInfo` directly in `threadsContainerClasses` and `reactionsContainerClasses`?
28	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, 6:14 PM

ginsu added inline comments.Jan 24 2023, 8:30 PM

web/chat/inline-engagement.react.js
28	Getting a flow error about cannot comparing undefined to number when attempting to replace `reactionsExist` with the statement you provided

address atuls comments

atul added inline comments.Jan 24 2023, 8:35 PM

web/chat/inline-engagement.react.js
28	Oh yeah that makes sense never mind

Harbormaster completed remote builds in B15596: Diff 21262.Jan 24 2023, 8:50 PM

Closed by commit rCOMM1401e536a93c: [web] implement new InlineEngagement design. · Explain WhyJan 24 2023, 9: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

Path

Size

web/

chat/

inline-engagement.css

43 lines

inline-engagement.react.js

70 lines

Diff 21207

web/chat/inline-engagement.css

Loading...

web/chat/inline-engagement.react.js

Loading...