Paths

Table of Contentst

Differential D4610

[web] Introduce `SidebarsModal`
ClosedPublic
Actions

Authored by • jacek on Jul 22 2022, 8:25 AM.

Tags

None

Referenced Files

	F3301076: D4610.diff
	Sun, Nov 17, 11:46 PM

	Unknown Object (File)
	Sat, Nov 9, 3:34 PM

	Unknown Object (File)
	Sat, Nov 9, 3:21 PM

	Unknown Object (File)
	Sat, Nov 9, 2:58 PM

	Unknown Object (File)
	Sat, Nov 9, 1:21 PM

	Unknown Object (File)
	Sat, Nov 9, 9:08 AM

	Unknown Object (File)
	Oct 17 2024, 11:01 PM

	Unknown Object (File)
	Oct 15 2024, 9:28 AM

Subscribers

Details

Reviewers

tomek
atul
ashoat

Commits

rCOMM496e35e418ab: [web] Introduce `SidebarsModal`

Summary

Introducing new SidebarsModal component with two tabs - with all sidebars and those displayed in chat list (that we are member of)

Screenshot_Google Chrome_2022-07-22_173150.png (679×498 px, 39 KB)

Test Plan

The modal is not used yet, next diff replaces an old one with this component in menu and after clicking "see more sidebars".

Diff Detail

Repository

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

• jacek created this revision.Jul 22 2022, 8:25 AM

Herald added subscribers: • abosh, • adrian, ashoat. · View Herald TranscriptJul 22 2022, 8:25 AM

• jacek edited the summary of this revision. (Show Details)Jul 22 2022, 8:31 AM

• jacek added a child revision: D4611: [web] Open `SidebarsModal` instead `SidebarListModal` on web.

• jacek added a parent revision: D4609: [web] Introduce `SidebarList` component.

• jacek added a parent revision: D4606: [web] Introduce `useFilteredChildThreads` hook.

Harbormaster completed remote builds in B10777: Diff 14818.Jul 22 2022, 8:38 AM

• jacek requested review of this revision.Jul 22 2022, 8:38 AM

New copy @ashoat

web/modals/threads/sidebars/sidebars-modal.css
4 ↗	(On Diff #14818)	Is this overflow a good idea? We already have `overflow: auto` on `.sidebarList`.
web/modals/threads/sidebars/sidebars-modal.react.js
26–29 ↗	(On Diff #14818)	This function never changes, so we can consider defining it outside the component
49–50 ↗	(On Diff #14818)

Looks good!

This revision is now accepted and ready to land.Jul 25 2022, 7:00 AM

• jacek added inline comments.Jul 26 2022, 4:15 AM

web/modals/threads/sidebars/sidebars-modal.css
4 ↗	(On Diff #14818)	You're right, it's not needed here

Address Tomek's sugegstions. Moved callback to thread-utils.js

• jacek requested review of this revision.Jul 26 2022, 4:39 AM

Harbormaster completed remote builds in B10878: Diff 14956.Jul 26 2022, 4:49 AM

tomek accepted this revision.Jul 26 2022, 6:57 AM

tomek added inline comments.

lib/shared/thread-utils.js
164 ↗	(On Diff #14956)	I don't think that double negation is necessary because `===` already returns a boolean

This revision is now accepted and ready to land.Jul 26 2022, 6:57 AM

• jacek added inline comments.Jul 26 2022, 8:09 AM

lib/shared/thread-utils.js
164 ↗	(On Diff #14956)	Thanks for catching!

remove redundant boolean cast

Harbormaster completed remote builds in B10896: Diff 14983.Jul 26 2022, 8:25 AM

Closed by commit rCOMM496e35e418ab: [web] Introduce `SidebarsModal` (authored by • jacek). · Explain WhyJul 26 2022, 8:30 AM

This revision was automatically updated to reflect the committed changes.

• jacek added a commit: rCOMM496e35e418ab: [web] Introduce `SidebarsModal`.

Revision Contents
Changeset List

Path

Size

lib/

shared/

thread-utils.js

5 lines

web/

modals/

threads/

sidebars/

sidebars-modal.css

8 lines

sidebars-modal.react.js

80 lines

Diff 14983

lib/shared/thread-utils.js

Loading...

web/modals/threads/sidebars/sidebars-modal.css

Loading...

web/modals/threads/sidebars/sidebars-modal.react.js

Loading...