Paths

Table of Contentst

Differential D3190

[web] Introduce `Members` and `Sidebars` thread menu actions
ClosedPublic
Actions

Authored by • jacek on Feb 14 2022, 3:41 AM.

Tags

None

Referenced Files

	F3537062: D3190.id9618.diff
	Wed, Dec 25, 8:04 PM

	F3537054: D3190.id9987.diff
	Wed, Dec 25, 8:02 PM

	F3536998: D3190.id9600.diff
	Wed, Dec 25, 7:45 PM

	F3536920: D3190.id9716.diff
	Wed, Dec 25, 7:30 PM

	F3536762: D3190.id9724.diff
	Wed, Dec 25, 6:55 PM

	Unknown Object (File)
	Wed, Dec 25, 7:48 AM

	Unknown Object (File)
	Fri, Dec 6, 9:42 PM

	Unknown Object (File)
	Fri, Dec 6, 9:42 PM

Subscribers

Details

Reviewers

tomek
• benschac
atul
ashoat

Commits

rCOMM89b627529fe3: [web] Introduce `Members` and `Sidebars` thread menu actions

Summary

Added Members and Sidebars menu items, that are displayed conditionally.
The onClick functionality will be added in one of next diffs.

Screenshot_Google Chrome_2022-02-14_122438.png (1×880 px, 89 KB)

Test Plan

Checked if Sidebar item appears correclty after adding sidebars to thread, and if Memberes is not displayed in Personal threads.

Diff Detail

Repository

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

• jacek created this revision.Feb 14 2022, 3:41 AM

Herald added subscribers: • karol, atul, • adrian, ashoat. · View Herald TranscriptFeb 14 2022, 3:41 AM

Harbormaster completed remote builds in B6778: Diff 9600.Feb 14 2022, 3:46 AM

• jacek requested review of this revision.Feb 14 2022, 3:46 AM

• jacek edited the summary of this revision. (Show Details)Feb 14 2022, 3:49 AM

• jacek added a parent revision: D3188: [web] Introduce `ThreadMenuItem`.Feb 14 2022, 3:53 AM

• jacek added reviewers: • benschac, atul.

tomek accepted this revision.Feb 14 2022, 4:30 AM

tomek added inline comments.

web/chat/thread-menu.react.js
65–68	This is correct, but we can consider avoiding repetition here by defining `items` outside the memo. The proposed approach has an unexpected usage of dependency array, which is defined earlier, so it might be confusing... not sure if it's worth it.

This revision is now accepted and ready to land.Feb 14 2022, 4:30 AM

Herald added a reviewer: ashoat. · View Herald TranscriptFeb 14 2022, 4:30 AM

This revision now requires review to proceed.Feb 14 2022, 4:30 AM

tomek added inline comments.Feb 14 2022, 5:09 AM

web/chat/thread-menu.react.js
65–68	After reading D3192 (see a comment there) it looks like we shouldn't follow my advice - let's keep it as it is

excluded hasSidebars into separate useMemo for better performance.

Harbormaster completed remote builds in B6793: Diff 9618.Feb 14 2022, 6:45 AM

tomek accepted this revision.Feb 14 2022, 7:34 AM

ashoat requested changes to this revision.Feb 14 2022, 9:19 PM

ashoat added inline comments.

web/chat/thread-menu.react.js
41–43 ↗	(On Diff #9618)	This should be defined closer to where it's used
52–58 ↗	(On Diff #9618)	See comments in D3191

This revision now requires changes to proceed.Feb 14 2022, 9:19 PM

Rebase & moved childThreads closer to where it is used.

Harbormaster completed remote builds in B6868: Diff 9716.Feb 16 2022, 5:03 AM

Simplified logic using some instead of filter

Harbormaster completed remote builds in B6876: Diff 9724.Feb 16 2022, 6:21 AM

ashoat accepted this revision.Feb 21 2022, 10:46 PM

This revision is now accepted and ready to land.Feb 21 2022, 10:46 PM

ashoat mentioned this in D3191: [web] Introduce `Subchannels` thread menu actions.Feb 21 2022, 10:48 PM

Closed by commit rCOMM89b627529fe3: [web] Introduce `Members` and `Sidebars` thread menu actions (authored by • jacek). · Explain WhyMar 1 2022, 2:13 AM

This revision was automatically updated to reflect the committed changes.

• jacek added a commit: rCOMM89b627529fe3: [web] Introduce `Members` and `Sidebars` thread menu actions.

Revision Contents
Changeset List

Path

Size

web/

chat/

thread-menu.react.js

41 lines

Diff 9600

web/chat/thread-menu.react.js

Loading...