Page MenuHomePhabricator
Differential D3769

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

Authored by atul on Apr 18 2022, 3:25 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Nov 6, 11:40 AM
Unknown Object (File)
Wed, Nov 6, 11:40 AM
Unknown Object (File)
Sat, Nov 2, 4:23 PM
Unknown Object (File)
Tue, Oct 22, 11:15 AM
Unknown Object (File)
Tue, Oct 22, 11:15 AM
Unknown Object (File)
Tue, Oct 22, 11:15 AM
Unknown Object (File)
Tue, Oct 22, 11:15 AM
Unknown Object (File)
Tue, Oct 22, 2:05 AM
View All Files
Subscribers
• abosh
• adrian
ashoat
• benschac
• karol

Details

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

Move onChangeAccountPassword 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 D3768

Test Plan
  1. Open ThreadSettingsModal
  2. Change account password under delete tab
  3. Log values in onChangeAccountPassword to ensure that the state continues to change as expected

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

atul created this revision.Apr 18 2022, 3:25 PM
Herald added subscribers: • abosh, • benschac, • karol and 2 others. · View Herald TranscriptApr 18 2022, 3:25 PM
Harbormaster completed remote builds in B8282: Diff 11578.Apr 18 2022, 3:30 PM
atul requested review of this revision.Apr 18 2022, 3:30 PM
atul added a child revision: D3770: [web] Move `hasPermissionForTab` up from `ThreadSettingsModal` to `ConnectedThreadSettingsModal`.Apr 18 2022, 3:41 PM
atul updated this revision to Diff 11650.Apr 19 2022, 3:04 PM

rebase

Harbormaster failed remote builds in B8340: Diff 11650!Apr 19 2022, 3:24 PM
Harbormaster completed remote builds in B8340: Diff 11650.Apr 19 2022, 3:41 PM
tomek accepted this revision.Apr 21 2022, 3:11 AM
tomek added inline comments.
web/modals/threads/thread-settings-modal.react.js
457–463

When a callback doesn't depend on anything, we can define the function outside the component. But on the other hand, it will break the flow of the code, so it's not necessarily worth it.

This revision is now accepted and ready to land.Apr 21 2022, 3:11 AM
atul added inline comments.Apr 21 2022, 11:18 AM
web/modals/threads/thread-settings-modal.react.js
457–463

Even though there aren't any dependencies, doesn't using React.useCallback provide referential equality when passed to the inner ThreadSettingsModal class component?

atul updated this revision to Diff 11752.Apr 21 2022, 11:19 AM

rebase before landing

Harbormaster completed remote builds in B8423: Diff 11752.Apr 21 2022, 11:25 AM
Closed by commit rCOMMd287cfc2fe2f: [web] Move `onChangeAccountPassword` up from `ThreadSettingsModal` to… (authored by atul). · Explain WhyApr 21 2022, 11:25 AM
This revision was automatically updated to reflect the committed changes.
atul added a commit: rCOMMd287cfc2fe2f: [web] Move `onChangeAccountPassword` up from `ThreadSettingsModal` to….
tomek added inline comments.Apr 25 2022, 2:09 AM
web/modals/threads/thread-settings-modal.react.js
457–463

When a function is defined in global scope, it never changes and is always equal to itself. It's true that callback provide the equality, but it is only needed when a function is redefined, e.g. in render function.

Revision Contents
Changeset List

PathSize
web/
modals/
threads/
17 lines

Diff 11650

View Options

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

Loading...