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

Authored by atul on Apr 18 2022, 10:25 AM.

Details

Reviewers

• jacek
tomek
ashoat

Commits

rCOMM2a11f335d584: [web] Move `errorMessage` state up from `ThreadSettingsModal` to…

Summary

Move errorMessage out of the inner class component state, and move it out to the wrapping "Connected" functional component.

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

Test Plan

Kill the node server
Try to save a change to thead settings
Observe that error message appears as expected

Note: The error message styling (showing up to the right of the button) isn't great. That's "on the todo list" and will be handled once the "noop" refactorings are complete.

Diff Detail

Repository

rCOMM Comm

Branch

apr19 (branched from master)

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

atul created this revision.Apr 18 2022, 10:25 AM

Herald added subscribers: • abosh, • benschac, • karol and 2 others. · View Herald TranscriptApr 18 2022, 10:25 AM

atul retitled this revision from [web] Use `React.useState('')` for `errorMessage` in `ThreadSettingsModal` to [web] Move `errorMessage` state up from `ThreadSettingsModal` to `ConnectedThreadSettingsModal`.Apr 18 2022, 10:26 AM

Harbormaster completed remote builds in B8262: Diff 11553.Apr 18 2022, 10:30 AM

atul requested review of this revision.Apr 18 2022, 10:30 AM

oops, missed newline

Harbormaster completed remote builds in B8263: Diff 11554.Apr 18 2022, 10:37 AM

atul edited the test plan for this revision. (Show Details)Apr 18 2022, 10:38 AM

atul added a child revision: D3757: [web] Move `accountPassword` state up from `ThreadSettingsModal` to `ConnectedThreadSettingsModal`.Apr 18 2022, 10:43 AM

Why is this necessary? Each diff should "stand on its own" and be reviewable without additional context. "As part of the work to refactor ThreadSettingsModal" is not enough of an explanation for me to understand why this state needs to be lifted. If the explanation is in another diff, a simple link to that diff should be enough!

This revision now requires changes to proceed.Apr 18 2022, 9:14 PM

(Please apply the above feedback to every diff in your stack to which it applies)

atul requested review of this revision.Apr 18 2022, 10:01 PM

atul edited the summary of this revision. (Show Details)

Cool, that's helpful. Please edit the diff description of every diff to include that info!

This revision is now accepted and ready to land.Apr 18 2022, 10:02 PM

rebase before landing

This revision was landed with ongoing or failed builds.Apr 19 2022, 11:29 AM

Closed by commit rCOMM2a11f335d584: [web] Move `errorMessage` state up from `ThreadSettingsModal` to… (authored by atul). · Explain Why

This revision was automatically updated to reflect the committed changes.

atul added a commit: rCOMM2a11f335d584: [web] Move `errorMessage` state up from `ThreadSettingsModal` to….

Harbormaster completed remote builds in B8326: Diff 11632.Apr 19 2022, 11:40 AM