Page MenuHomePhabricator

[web] Add `nameInputRef` back to `ThreadSettingsGeneralTab`
ClosedPublic

Authored by atul on Apr 25 2022, 6:16 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Dec 25, 8:14 AM
Unknown Object (File)
Thu, Dec 19, 1:56 PM
Unknown Object (File)
Thu, Dec 19, 1:22 PM
Unknown Object (File)
Thu, Dec 19, 1:22 PM
Unknown Object (File)
Thu, Dec 19, 1:22 PM
Unknown Object (File)
Thu, Dec 19, 1:22 PM
Unknown Object (File)
Thu, Dec 19, 1:22 PM
Unknown Object (File)
Thu, Dec 19, 1:22 PM

Details

Summary

Bring back nameInputRef to ThreadSettingsGeneralTab using the React.useRef() hook.

We add nameInputRef.current?.focus() to the useEffect so the input is focused when it appears.


Depends on D3828

Test Plan
  1. Open ThreadSettingsModal
  2. Ensure that the thread name input is focused

Diff Detail

Repository
rCOMM Comm
Branch
clean (branched from master)
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

web/modals/threads/thread-settings-modal.react.js
183–184 ↗(On Diff #11861)

This will be handled implicitly by the ThreadSettingsModalGeneralTab:useEffect

201–202 ↗(On Diff #11861)

This has been "pushed down" to the ThreadSettingsGeneralTab useEffect

atul published this revision for review.Apr 25 2022, 6:18 AM
tomek requested changes to this revision.Apr 26 2022, 8:30 AM
tomek added inline comments.
web/modals/threads/thread-settings-general-tab.react.js
35 ↗(On Diff #11861)

Are you sure we need to focus only initially? What was the original behavior?

This revision now requires changes to proceed.Apr 26 2022, 8:30 AM

address feedback from @palys-swm

web/modals/threads/thread-settings-general-tab.react.js
35 ↗(On Diff #11861)

The intention of the original behavior was to focus the input when changing the thread settings failed.

However, it looks like this functionality didn't actually work. It didn't work because we were attempting to .focus() an input that was in the disabled state. This was a noop and the input wasn't actually focused once it returned to the enabled state.


The solution I came up with to achieve the intended behavior was adding inputDisabled to the deplist of the useEffect hook.

That way the input will be focused when
A. The ThreadSettingsGeneralTab component first loads
B. When inputDisabled toggles from true to false... which only happens after the form is submitted and there is an error. If the changes are made successfully, the modal is immediately dismissed so there's no opportunity for the ThreadSettingsGeneralTab component to re-render and focus the name input.

This revision is now accepted and ready to land.Apr 26 2022, 11:18 AM