Paths

Table of Contentst

Differential D8575

[native] add spacing in between each reaction pill
ClosedPublic
Actions

Authored by ginsu on Jul 19 2023, 2:07 PM.

Tags

None

Referenced Files

	F3388279: D8575.id29305.diff
	Fri, Nov 29, 1:30 PM

	F3388276: D8575.id28953.diff
	Fri, Nov 29, 1:29 PM

	Unknown Object (File)
	Tue, Nov 26, 3:54 AM

	Unknown Object (File)
	Tue, Nov 26, 3:40 AM

	Unknown Object (File)
	Tue, Nov 26, 1:22 AM

	Unknown Object (File)
	Sat, Nov 23, 6:29 PM

	Unknown Object (File)
	Sat, Nov 23, 2:35 PM

	Unknown Object (File)
	Sat, Nov 23, 12:52 PM

Subscribers

Details

Reviewers

atul
kamil

Commits

rCOMM37a49bd1df75: [native] add spacing in between each reaction pill

Summary

After separating out each reaction into its own pill, the next step to improve this experience is to add spacing to each reaction pill. This diff adds a margin either to left or right of the reaction pill depending on whether the positioning of the InlineEngagement isRight or not

Depends on D8539

Test Plan

Please see the screenshots below

Before:

Screenshot 2023-07-18 at 1.08.13 AM.png (1×1 px, 927 KB)

Screenshot 2023-07-18 at 1.07.48 AM.png (1×1 px, 854 KB)

After:

Screenshot 2023-07-19 at 5.09.42 PM.png (1×1 px, 927 KB)

Screenshot 2023-07-19 at 5.10.11 PM.png (1×1 px, 862 KB)

Diff Detail

Repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

ginsu created this revision.Jul 19 2023, 2:07 PM

Herald added subscribers: tomek, ashoat. · View Herald TranscriptJul 19 2023, 2:07 PM

ginsu edited the summary of this revision. (Show Details)Jul 19 2023, 2:11 PM

ginsu edited the test plan for this revision. (Show Details)

ginsu added reviewers: atul, kamil.

ginsu edited the summary of this revision. (Show Details)

ginsu added a parent revision: D8539: [native] seperate out each reaction into their own pill.

ginsu attached a referenced file: F645452: Screenshot 2023-07-19 at 5.10.11 PM.png. (Show Details)

ginsu attached a referenced file: F645451: Screenshot 2023-07-19 at 5.09.42 PM.png. (Show Details)

ginsu attached a referenced file: F645450: Screenshot 2023-07-18 at 1.07.48 AM.png. (Show Details)

ginsu attached a referenced file: F645449: Screenshot 2023-07-18 at 1.08.13 AM.png. (Show Details)

switch sidebarMargin and reactionsMargin to sidebarStyle and reactionStyle

Harbormaster completed remote builds in B21060: Diff 28859.Jul 19 2023, 2:28 PM

Harbormaster completed remote builds in B21062: Diff 28862.Jul 19 2023, 2:42 PM

ginsu requested review of this revision.Jul 19 2023, 2:42 PM

ginsu added a child revision: D8576: [native] make reactions prop for InlineEngagement requried.Jul 19 2023, 2:53 PM

kamil accepted this revision.Jul 21 2023, 2:53 AM

kamil added inline comments.

native/chat/inline-engagement.react.js
87 ↗	(On Diff #28862)	array suggests multiple elements

This revision is now accepted and ready to land.Jul 21 2023, 2:53 AM

ginsu added inline comments.Jul 21 2023, 11:46 AM

native/chat/inline-engagement.react.js
87 ↗	(On Diff #28862)	I can't use `styles` because `styles` is already declared in the upper scope, but how about `stylesResult`?

address feedback

forgot to fix reactionStyle

Harbormaster completed remote builds in B21129: Diff 28951.Jul 21 2023, 12:04 PM

Harbormaster completed remote builds in B21131: Diff 28953.Jul 21 2023, 12:09 PM

rebase before landing

This revision was landed with ongoing or failed builds.Jul 31 2023, 1:19 PM

Closed by commit rCOMM37a49bd1df75: [native] add spacing in between each reaction pill (authored by ginsu). · Explain Why

This revision was automatically updated to reflect the committed changes.

ginsu added a commit: rCOMM37a49bd1df75: [native] add spacing in between each reaction pill.

Harbormaster completed remote builds in B21393: Diff 29305.Jul 31 2023, 1:42 PM

Revision Contents
Changeset List

Path

Size

native/

chat/

inline-engagement.react.js

59 lines

Diff 29355

native/chat/inline-engagement.react.js

Loading...