Paths

Table of Contentst

Differential D3763

[web] Move `changeQueued` up from `ThreadSettingsModal` to `ConnectedThreadSettingsModal`
ClosedPublic
Actions

Authored by atul on Apr 18 2022, 1:41 PM.

Tags

None

Referenced Files

	F3174672: D3763.diff
	Thu, Nov 7, 5:31 PM

	Unknown Object (File)
	Tue, Nov 5, 5:07 AM

	Unknown Object (File)
	Sun, Nov 3, 1:26 PM

	Unknown Object (File)
	Sun, Nov 3, 1:26 PM

	Unknown Object (File)
	Sun, Nov 3, 1:26 PM

	Unknown Object (File)
	Mon, Oct 28, 2:33 AM

	Unknown Object (File)
	Thu, Oct 10, 11:01 AM

	Unknown Object (File)
	Thu, Oct 10, 3:24 AM

Subscribers

Details

Reviewers

tomek
• jacek

Commits

rCOMM8fe46e3b77f1: [web] Move `changeQueued` up from `ThreadSettingsModal` to…

Summary

Move changeQueued out from inner class component to outer "Connected" functional component.

(Also refactored the logic to simplify things/avoid lodash)

As part of the work to refactor ThreadSettingsModal... specifically to turn it into a functional component

Depends on D3762

Test Plan

Made changes in ThreadSettingsModal
Logged value of changeQueued and made sure it was as expected

Diff Detail

Repository

Branch

master

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

atul created this revision.Apr 18 2022, 1:41 PM

Herald added subscribers: • abosh, • benschac, • karol and 2 others. · View Herald TranscriptApr 18 2022, 1:41 PM

atul added inline comments.Apr 18 2022, 1:44 PM

web/modals/threads/thread-settings-modal.react.js
438–442 ↗	(On Diff #11569)	Experimented to make sure that this logic works as expected.

remove curly braces

atul added a reviewer: ashoat.Apr 18 2022, 1:46 PM

Harbormaster failed remote builds in B8273: Diff 11569!Apr 18 2022, 1:46 PM

Adding @ashoat as first pass reviewer since he initially wrote the logic I refactored

Harbormaster completed remote builds in B8274: Diff 11570.Apr 18 2022, 1:52 PM

atul requested review of this revision.Apr 18 2022, 1:52 PM

further simplify changeQueued logic with .values

val => v... looks better?

Harbormaster completed remote builds in B8275: Diff 11571.Apr 18 2022, 2:04 PM

Harbormaster completed remote builds in B8276: Diff 11572.Apr 18 2022, 2:09 PM

atul added a child revision: D3764: [web] Remove usage of `setTab` and replace with `setCurrentTabType`.Apr 18 2022, 2:16 PM

I don't recall anything about this logic. Feel free to add me back if you think there is some unique insight I am able to provide

rebase

Harbormaster completed remote builds in B8334: Diff 11644.Apr 19 2022, 3:10 PM

tomek accepted this revision.Apr 21 2022, 2:52 AM

tomek added inline comments.

web/modals/threads/thread-settings-modal.react.js
447–448 ↗	(On Diff #11644)	A combination of `filter` and `length > 0` can be expressed by using `some`. At this point we could even use something like `.some(Boolean)` but it is not that maintainable - introducing a change that sets a boolean might cause a hard to catch bug.

This revision is now accepted and ready to land.Apr 21 2022, 2:52 AM

rebase before addressing feedback

atul added inline comments.Apr 21 2022, 8:57 AM

web/modals/threads/thread-settings-modal.react.js
447–448 ↗	(On Diff #11644)	Oh nice, didn't know about `some`, looks cleaner

Harbormaster failed remote builds in B8406: Diff 11730!Apr 21 2022, 9:00 AM

rebase

Harbormaster completed remote builds in B8407: Diff 11731.Apr 21 2022, 9:07 AM

Closed by commit rCOMM8fe46e3b77f1: [web] Move `changeQueued` up from `ThreadSettingsModal` to… (authored by atul). · Explain WhyApr 21 2022, 9:50 AM

This revision was automatically updated to reflect the committed changes.

atul added a commit: rCOMM8fe46e3b77f1: [web] Move `changeQueued` up from `ThreadSettingsModal` to….

atul removed a child revision: D3764: [web] Remove usage of `setTab` and replace with `setCurrentTabType`.Apr 24 2022, 1:58 PM

atul added a child revision: D3764: [web] Remove usage of `setTab` and replace with `setCurrentTabType`.Apr 25 2022, 6:44 AM

Revision Contents
Changeset List

Path

Size

web/

modals/

threads/

thread-settings-modal.react.js

25 lines

Diff 11571

web/modals/threads/thread-settings-modal.react.js

Loading...