Details

Reviewers

• jacek
tomek

Commits

rCOMMc07ce55fa912: [web] Push `onChange[Name/Description/Color]` callbacks down to…

Summary

Push onChange[Name/Description/Color] callbacks down to the ThreadSettingsGeneralTab component to clean up parent ThreadSettingsModal component and move them closer to where they're used.

Added threadInfo, queuedChanges, and setQueuedChanges props to ThreadSettingsGeneralTab component so callbacks could be "constructed" from the "child" component
Adding those props also lets us determine threadDescriptionValue, etc within child component instead of having to pass in from parent

Depends on D3826

Test Plan

Open ThreadSettingsModal
Make sure that the "General" tab continues to look/work as expected

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

atul created this revision.Apr 24 2022, 1:49 PM

Herald added subscribers: • abosh, • karol, • adrian, ashoat. · View Herald TranscriptApr 24 2022, 1:49 PM

atul added inline comments.Apr 24 2022, 1:50 PM

web/modals/threads/thread-settings-modal.react.js
135 ↗	(On Diff #11832)	No longer need optional chaining because `threadInfo` is guaranteed to exist within `ThreadSettingsGeneralTab`

Harbormaster completed remote builds in B8487: Diff 11832.Apr 24 2022, 1:54 PM

atul requested review of this revision.Apr 24 2022, 1:54 PM

access properties with obj.blah instead of obj['blah']

Harbormaster completed remote builds in B8488: Diff 11833.Apr 24 2022, 2:05 PM

atul added a child revision: D3828: [web] Push `onChangeThreadType` callback down to `ThreadSettingsPrivacyTab` component.Apr 24 2022, 2:17 PM

tomek accepted this revision.Apr 26 2022, 8:20 AM

tomek added inline comments.

web/modals/threads/thread-settings-general-tab.react.js
31–44 ↗	(On Diff #11833)	It's safer to use a callback version of set state. That would also allow us reducing deps list.

This revision is now accepted and ready to land.Apr 26 2022, 8:20 AM

rebase

Harbormaster completed remote builds in B8568: Diff 11947.Apr 26 2022, 10:37 AM

atul added inline comments.Apr 28 2022, 1:36 PM

web/modals/threads/thread-settings-general-tab.react.js
31–44 ↗	(On Diff #11833)	Oh nice, will put up a diff changing all of these: https://linear.app/comm/issue/ENG-1061/[web]-use-callback-version-of-setstate-in-threadsettingsmodal