Page MenuHomePhabricator
Differential D3795

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

Authored by atul on Apr 20 2022, 2:57 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Nov 27, 5:43 AM
Unknown Object (File)
Wed, Nov 27, 5:40 AM
Unknown Object (File)
Mon, Nov 25, 11:20 AM
Unknown Object (File)
Fri, Nov 22, 2:01 PM
Unknown Object (File)
Fri, Nov 22, 2:01 PM
Unknown Object (File)
Fri, Nov 22, 2:01 PM
Unknown Object (File)
Fri, Nov 22, 2:01 PM
Unknown Object (File)
Fri, Nov 22, 1:59 PM
View All Files
Subscribers
• abosh
• adrian
ashoat
• benschac
• karol

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
  1. Open ThreadSettingsModal
  2. Mess with the settings
  3. 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
atul updated this revision to Diff 11813.Apr 22 2022, 2:04 PM

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

PathSize
web/
modals/
threads/
46 lines

Diff 11813

View Options

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

Loading...