Move accountPassword 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.
Depends on D3756
Details
Details
Close reading... should be a noop
Will test the entirety of ThreadSettingsModal thoroughly once the refactor is complete (plan on using this as an opportunity to write some automated tests)
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| web/modals/threads/thread-settings-modal.react.js | ||
|---|---|---|
| 433–435 ↗ | (On Diff #11555) |
There's a subtle difference in how this works. Previously, we were focusing after password in state was set to ''. Now we focus immediately, so in theory it is possible to focus before password is set. I guess it doesn't hurt, but we should be careful when stating that nothing has changed. If we want to keep the previous approach, we can try something like https://stackoverflow.com/questions/54954091/how-to-use-callback-with-usestate-hook-in-react/61842546#61842546 |
Comment Actions
Close reading... should be a noop
There's a subtle difference in how this works. Previously, we were focusing after password in state was set to ''. Now we focus immediately, so in theory it is possible to focus before password is set. I guess it doesn't hurt, but we should be careful when stating that nothing has changed.
If we want to keep the previous approach, we can try something like https://stackoverflow.com/questions/54954091/how-to-use-callback-with-usestate-hook-in-react/61842546#61842546
Ah you're right, there's no guarantee that the input will be cleared by the time the input is focused. In practice there doesn't seem to be any issue with this approach and the experience remains smooth... but it might be good to introduce something like the useStateCallback hook described in the StackOverflow thread you attached for correctness/predictability.
We're using an approach similar to the one in this diff for the LoginForm component, so there's at least one other place in the codebase where we should make similar changes. Made a Linear task to track the issue: https://linear.app/comm/issue/ENG-1023/[web]-explore-synchronously-focusing-inputs-after-state-has-been-set