Page MenuHomePhabricator
Differential D3767

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

Authored by atul on Apr 18 2022, 2:51 PM.
Tags
None
Referenced Files
F3391792: D3767.diff
Sat, Nov 30, 6:08 AM
Unknown Object (File)
Wed, Nov 27, 6:44 AM
Unknown Object (File)
Wed, Nov 27, 6:42 AM
Unknown Object (File)
Wed, Nov 27, 6:42 AM
Unknown Object (File)
Wed, Nov 27, 5:40 AM
Unknown Object (File)
Thu, Nov 21, 11:42 PM
Unknown Object (File)
Wed, Nov 13, 12:50 PM
Unknown Object (File)
Wed, Nov 13, 10:42 AM
View All Files
Subscribers
• abosh
• adrian
ashoat
• benschac
• karol

Details

Reviewers
• jacek
tomek
Commits
rCOMM35a0538473a4: [web] Move `onChangeColor` up from `ThreadSettingsModal` to…
Summary

Move onChangeColor 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.

Depends on D3766

Test Plan
  1. Open ThreadSettingsModal
  2. Change the color a bunch
  3. Log values in onChangeColor to ensure that they continue to be as expected
  4. Hit save after changing the color and ensure that the thread description changed 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 18 2022, 2:51 PM
Herald added subscribers: • abosh, • benschac, • karol and 2 others. · View Herald TranscriptApr 18 2022, 2:51 PM
Harbormaster completed remote builds in B8280: Diff 11576.Apr 18 2022, 2:56 PM
atul requested review of this revision.Apr 18 2022, 2:56 PM
atul added a child revision: D3768: [web] Move `onChangeThreadType` up from `ThreadSettingsModal` to `ConnectedThreadSettingsModal`.Apr 18 2022, 3:04 PM
ashoat added a comment.Apr 18 2022, 9:47 PM

specifically to turn it into a functional component.

That is helpful to see, can you add that to the diff descriptions of the rest of the elements in your stack?

atul updated this revision to Diff 11648.Apr 19 2022, 3:04 PM

rebase

Harbormaster completed remote builds in B8338: Diff 11648.Apr 19 2022, 3:19 PM
atul added a comment.Apr 20 2022, 12:47 PM

That is helpful to see, can you add that to the diff descriptions of the rest of the elements in your stack?

Yeah, added it to the four diffs in the stack that were missing that clarification

tomek accepted this revision.Apr 21 2022, 3:08 AM
tomek added inline comments.
web/modals/threads/thread-settings-modal.react.js
447–456 ↗(On Diff #11648)
This revision is now accepted and ready to land.Apr 21 2022, 3:08 AM
atul updated this revision to Diff 11745.Apr 21 2022, 10:56 AM

rebase before landing

atul updated this revision to Diff 11746.Apr 21 2022, 10:58 AM

address feedback

Harbormaster completed remote builds in B8418: Diff 11745.Apr 21 2022, 11:01 AM
Harbormaster completed remote builds in B8419: Diff 11746.Apr 21 2022, 11:05 AM
Closed by commit rCOMM35a0538473a4: [web] Move `onChangeColor` up from `ThreadSettingsModal` to… (authored by atul). · Explain WhyApr 21 2022, 11:05 AM
This revision was automatically updated to reflect the committed changes.
atul added a commit: rCOMM35a0538473a4: [web] Move `onChangeColor` up from `ThreadSettingsModal` to….

Revision Contents
Changeset List

PathSize
web/
modals/
threads/
25 lines

Diff 11745

View Options

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

Loading...