Paths

Table of Contentst

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

Authored by atul on Apr 20 2022, 2:57 PM.

Details

Reviewers

• jacek
tomek

Commits

rCOMM8ccccf404f3c: [web] Move `changeThreadSettingsAction` up from `ThreadSettingsModal` to…

Summary

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

As part of the work to refactor ThreadSettingsModal... specifically to turn it into a functional component (any day now...)

Depends on D3794

Test Plan

Open ThreadSettingsModal
Mess with the settings
Hit save and make sure things continue to look/work as expected

Diff Detail

Repository

rCOMM Comm

Branch

actuallylandapril21 (branched from master)

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

atul created this revision.Apr 20 2022, 2:57 PM

Herald added subscribers: • abosh, • benschac, • karol and 2 others. · View Herald TranscriptApr 20 2022, 2:57 PM

atul added inline comments.Apr 20 2022, 2:58 PM

web/modals/threads/thread-settings-modal.react.js
471–472 ↗	(On Diff #11681)	See: https://phabricator.ashoat.com/D3783#104491

Harbormaster completed remote builds in B8369: Diff 11681.Apr 20 2022, 3:03 PM

atul requested review of this revision.Apr 20 2022, 3:03 PM

atul added a child revision: D3796: [web] Move `onSubmit` up from `ThreadSettingsModal` to `ConnectedThreadSettingsModal`.Apr 20 2022, 3:21 PM

tomek accepted this revision.Apr 21 2022, 8:50 AM

tomek added inline comments.

web/modals/threads/thread-settings-modal.react.js
475 ↗	(On Diff #11681)	Only `threadInfo.id` is used in this callback (except in the invariant). Can we change the deps array to reflect that?

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

rebase before landing

Harbormaster completed remote builds in B8472: Diff 11813.Apr 22 2022, 2:09 PM

atul added inline comments.Apr 22 2022, 2:19 PM

web/modals/threads/thread-settings-modal.react.js
475 ↗	(On Diff #11681)	Ran into the same issue as here: https://phabricator.ashoat.com/D3783?id=11652#inline-23122

Closed by commit rCOMM8ccccf404f3c: [web] Move `changeThreadSettingsAction` up from `ThreadSettingsModal` to… (authored by atul). · Explain WhyApr 22 2022, 2:21 PM

This revision was automatically updated to reflect the committed changes.

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

Revision Contents
Changeset List

Path

Size

web/

modals/

threads/

thread-settings-modal.react.js

46 lines

Diff 11813

View Options

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

[web] Move `changeThreadSettingsAction` up from `ThreadSettingsModal` to `ConnectedThreadSettingsModal`ClosedPublicActions