Page MenuHomePhabricator
Differential D3832

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

Authored by atul on Apr 25 2022, 6:16 AM.
Tags
None
Referenced Files
F6250510: D3832.id11949.diff
Fri, Apr 25, 3:49 AM
Unknown Object (File)
Thu, Apr 24, 5:54 PM
Unknown Object (File)
Tue, Apr 22, 1:36 AM
Unknown Object (File)
Sun, Apr 20, 2:03 PM
Unknown Object (File)
Thu, Apr 17, 1:42 PM
Unknown Object (File)
Thu, Apr 17, 1:16 PM
Unknown Object (File)
Thu, Apr 17, 12:40 PM
Unknown Object (File)
Thu, Apr 17, 9:33 AM
View All Files
Subscribers
• abosh
• adrian
ashoat
• karol

Details

Reviewers
• jacek
tomek
Commits
rCOMM0594d6438ca2: [web] Add `nameInputRef` back to `ThreadSettingsGeneralTab`
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
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

atul created this revision.Apr 25 2022, 6:16 AM
Herald added subscribers: • abosh, • karol, • adrian, ashoat. · View Herald TranscriptApr 25 2022, 6:16 AM
atul added inline comments.Apr 25 2022, 6:17 AM
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
Harbormaster completed remote builds in B8503: Diff 11861.Apr 25 2022, 6:21 AM
atul mentioned this in D3821: [web] Temporarily remove refs in `ThreadSettingsModal` altogether.Apr 25 2022, 6:43 AM
atul added a child revision: D3837: [web] Push "Delete thread" functionality down to `ThreadSettingsDeleteTab`.Apr 25 2022, 10:34 AM
atul added a child revision: D3838: [web] Add `accountPasswordInputRef` back to `ThreadSettingsDeleteTab`.Apr 25 2022, 11:10 AM
atul removed a child revision: D3838: [web] Add `accountPasswordInputRef` back to `ThreadSettingsDeleteTab`.Apr 25 2022, 11: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
atul updated this revision to Diff 11949.Apr 26 2022, 10:33 AM

rebase

atul updated this revision to Diff 11950.Apr 26 2022, 10:34 AM

address feedback from @palys-swm

atul added inline comments.Apr 26 2022, 10:38 AM
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.

Harbormaster completed remote builds in B8570: Diff 11949.Apr 26 2022, 10:40 AM
Harbormaster completed remote builds in B8571: Diff 11950.Apr 26 2022, 10:44 AM
tomek accepted this revision.Apr 26 2022, 11:18 AM
This revision is now accepted and ready to land.Apr 26 2022, 11:18 AM
atul updated this revision to Diff 12079.Apr 28 2022, 2:04 PM

rebase before landing

Harbormaster completed remote builds in B8675: Diff 12079.Apr 28 2022, 2:13 PM
Closed by commit rCOMM0594d6438ca2: [web] Add `nameInputRef` back to `ThreadSettingsGeneralTab` (authored by atul). · Explain WhyApr 28 2022, 2:15 PM
This revision was automatically updated to reflect the committed changes.
atul added a commit: rCOMM0594d6438ca2: [web] Add `nameInputRef` back to `ThreadSettingsGeneralTab`.

Revision Contents
Changeset List

Diff 12080

View Options

web/modals/threads/thread-settings-general-tab.react.js

  • This file was copied to lib/keyserver-conn/call-keyserver-endpoint-provider.react.js.
// @flow // @flow
import invariant from 'invariant'; import invariant from 'invariant';
import _memoize from 'lodash/memoize.js';
import * as React from 'react'; import * as React from 'react';
import { createSelector } from 'reselect';
import callServerEndpoint from './call-server-endpoint.js';
import type {
CallServerEndpoint,
CallServerEndpointOptions,
} from './call-server-endpoint.js';
import { useSelector, useDispatch } from './redux-utils.js'; import { useSelector, useDispatch } from './redux-utils.js';
import { ashoatKeyserverID } from './validation-utils.js'; import { ashoatKeyserverID } from './validation-utils.js';
import { setNewSession } from '../keyserver-conn/keyserver-conn-types.js';
import { import {
canResolveKeyserverSessionInvalidation, type ActionFunc,
resolveKeyserverSessionInvalidation, type BindServerCallsParams,
} from '../keyserver-conn/recovery-utils.js'; createBoundServerCallsSelector,
} from '../keyserver-conn/call-keyserver-endpoint-provider.react.js';
import { serverCallStateSelector } from '../selectors/server-calls.js'; import { serverCallStateSelector } from '../selectors/server-calls.js';
import { logInActionSources } from '../types/account-types.js';
import type { PlatformDetails } from '../types/device-types.js';
import type { Endpoint, SocketAPIHandler } from '../types/endpoints.js';
import type { Dispatch } from '../types/redux-types.js';
import type { ClientSessionChange } from '../types/session-types.js';
import type { CurrentUserInfo } from '../types/user-types.js';
let currentlyWaitingForNewCookie = false;
let serverEndpointCallsWaitingForNewCookie: ((
callServerEndpoint: ?CallServerEndpoint,
) => void)[] = [];
// Third param is optional and gets called with newCookie if we get a new cookie
// Necessary to propagate cookie in cookieInvalidationRecovery below
function bindCookieAndUtilsIntoCallServerEndpoint(
params: BindServerCallsParams,
): CallServerEndpoint {
const {
dispatch,
cookie,
urlPrefix,
sessionID,
currentUserInfo,
isSocketConnected,
lastCommunicatedPlatformDetails,
keyserverID,
} = params;
const loggedIn = !!(currentUserInfo && !currentUserInfo.anonymous && true);
const boundSetNewSession = (
sessionChange: ClientSessionChange,
error: ?string,
) =>
setNewSession(
dispatch,
sessionChange,
{
currentUserInfo,
cookiesAndSessions: { [keyserverID]: { cookie, sessionID } },
},
error,
undefined,
keyserverID,
);
const canResolveInvalidation = canResolveKeyserverSessionInvalidation();
// This function gets called before callServerEndpoint sends a request,
// to make sure that we're not in the middle of trying to recover
// an invalidated cookie
const waitIfCookieInvalidated = () => {
if (!canResolveInvalidation) {
// If there is no way to resolve the session invalidation,
// just let the caller callServerEndpoint instance continue
return Promise.resolve(null);
}
if (!currentlyWaitingForNewCookie) {
// Our cookie seems to be valid
return Promise.resolve(null);
}
// Wait to run until we get our new cookie
return new Promise<?CallServerEndpoint>(r =>
serverEndpointCallsWaitingForNewCookie.push(r),
);
};
// This function is a helper for the next function defined below
const attemptToResolveInvalidation = async (
sessionChange: ClientSessionChange,
) => {
const newAnonymousCookie = sessionChange.cookie;
const newSessionChange = await resolveKeyserverSessionInvalidation(
dispatch,
newAnonymousCookie,
urlPrefix,
logInActionSources.cookieInvalidationResolutionAttempt,
keyserverID,
);
currentlyWaitingForNewCookie = false;
const currentWaitingCalls = serverEndpointCallsWaitingForNewCookie;
serverEndpointCallsWaitingForNewCookie = [];
const newCallServerEndpoint = newSessionChange
? bindCookieAndUtilsIntoCallServerEndpoint({
...params,
cookie: newSessionChange.cookie,
sessionID: newSessionChange.sessionID,
currentUserInfo: newSessionChange.currentUserInfo,
})
: null;
for (const func of currentWaitingCalls) {
func(newCallServerEndpoint);
}
return newCallServerEndpoint;
};
// If this function is called, callServerEndpoint got a response invalidating
// its cookie, and is wondering if it should just like... give up?
// Or if there's a chance at redemption
const cookieInvalidationRecovery = (sessionChange: ClientSessionChange) => {
if (!canResolveInvalidation) {
// If there is no way to resolve the session invalidation,
// just let the caller callServerEndpoint instance continue
return Promise.resolve(null);
}
if (!loggedIn) {
// We don't want to attempt any use native credentials of a logged out
// user to log-in after a cookieInvalidation while logged out
return Promise.resolve(null);
}
if (currentlyWaitingForNewCookie) {
return new Promise<?CallServerEndpoint>(r =>
serverEndpointCallsWaitingForNewCookie.push(r),
);
}
currentlyWaitingForNewCookie = true;
return attemptToResolveInvalidation(sessionChange);
};
return (
endpoint: Endpoint,
data: Object,
options?: ?CallServerEndpointOptions,
) =>
callServerEndpoint(
cookie,
boundSetNewSession,
waitIfCookieInvalidated,
cookieInvalidationRecovery,
urlPrefix,
sessionID,
isSocketConnected,
lastCommunicatedPlatformDetails,
socketAPIHandler,
endpoint,
data,
dispatch,
options,
loggedIn,
keyserverID,
);
}
export type ActionFunc<F> = (callServerEndpoint: CallServerEndpoint) => F;
export type BindServerCall = <F>(serverCall: ActionFunc<F>) => F;
export type BindServerCallsParams = {
+dispatch: Dispatch,
+cookie: ?string,
+urlPrefix: string,
+sessionID: ?string,
+currentUserInfo: ?CurrentUserInfo,
+isSocketConnected: boolean,
+lastCommunicatedPlatformDetails: ?PlatformDetails,
+keyserverID: string,
};
// All server calls needs to include some information from the Redux state
// (namely, the cookie). This information is used deep in the server call,
// at the point where callServerEndpoint is called. We don't want to bother
// propagating the cookie (and any future config info that callServerEndpoint
// needs) through to the server calls so they can pass it to callServerEndpoint.
// Instead, we "curry" the cookie onto callServerEndpoint within react-redux's
// connect's mapStateToProps function, and then pass that "bound"
// callServerEndpoint that no longer needs the cookie as a parameter on to
// the server call.
const baseCreateBoundServerCallsSelector = <F>(
actionFunc: ActionFunc<F>,
): (BindServerCallsParams => F) =>
createSelector(
(state: BindServerCallsParams) => state.dispatch,
(state: BindServerCallsParams) => state.cookie,
(state: BindServerCallsParams) => state.urlPrefix,
(state: BindServerCallsParams) => state.sessionID,
(state: BindServerCallsParams) => state.currentUserInfo,
(state: BindServerCallsParams) => state.isSocketConnected,
(state: BindServerCallsParams) => state.lastCommunicatedPlatformDetails,
(state: BindServerCallsParams) => state.keyserverID,
(
dispatch: Dispatch,
cookie: ?string,
urlPrefix: string,
sessionID: ?string,
currentUserInfo: ?CurrentUserInfo,
isSocketConnected: boolean,
lastCommunicatedPlatformDetails: ?PlatformDetails,
keyserverID: string,
) => {
const boundCallServerEndpoint = bindCookieAndUtilsIntoCallServerEndpoint({
dispatch,
cookie,
urlPrefix,
sessionID,
currentUserInfo,
isSocketConnected,
lastCommunicatedPlatformDetails,
keyserverID,
});
return actionFunc(boundCallServerEndpoint);
},
);
type CreateBoundServerCallsSelectorType = <F>(
ActionFunc<F>,
) => BindServerCallsParams => F;
const createBoundServerCallsSelector: CreateBoundServerCallsSelectorType =
(_memoize(baseCreateBoundServerCallsSelector): any);
function useServerCall<F>( function useServerCall<F>(
serverCall: ActionFunc<F>, serverCall: ActionFunc<F>,
paramOverride?: ?Partial<BindServerCallsParams>, paramOverride?: ?Partial<BindServerCallsParams>,
): F { ): F {
const dispatch = useDispatch(); const dispatch = useDispatch();
const serverCallState = useSelector( const serverCallState = useSelector(
serverCallStateSelector(ashoatKeyserverID), serverCallStateSelector(ashoatKeyserverID),
Show All 13 Lines return createBoundServerCallsSelector(serverCall)({
isSocketConnected, isSocketConnected,
dispatch, dispatch,
...paramOverride, ...paramOverride,
keyserverID: ashoatKeyserverID, keyserverID: ashoatKeyserverID,
}); });
}, [serverCall, serverCallState, dispatch, paramOverride]); }, [serverCall, serverCallState, dispatch, paramOverride]);
} }
let socketAPIHandler: ?SocketAPIHandler = null; export { useServerCall };
function registerActiveSocket(passedSocketAPIHandler: ?SocketAPIHandler) {
socketAPIHandler = passedSocketAPIHandler;
}
export {
createBoundServerCallsSelector,
registerActiveSocket,
useServerCall,
bindCookieAndUtilsIntoCallServerEndpoint,
};
View Options

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

  • This file was copied to lib/keyserver-conn/call-keyserver-endpoint-provider.react.js.
// @flow // @flow
import invariant from 'invariant'; import invariant from 'invariant';
import _memoize from 'lodash/memoize.js';
import * as React from 'react'; import * as React from 'react';
import { createSelector } from 'reselect';
import callServerEndpoint from './call-server-endpoint.js';
import type {
CallServerEndpoint,
CallServerEndpointOptions,
} from './call-server-endpoint.js';
import { useSelector, useDispatch } from './redux-utils.js'; import { useSelector, useDispatch } from './redux-utils.js';
import { ashoatKeyserverID } from './validation-utils.js'; import { ashoatKeyserverID } from './validation-utils.js';
import { setNewSession } from '../keyserver-conn/keyserver-conn-types.js';
import { import {
canResolveKeyserverSessionInvalidation, type ActionFunc,
resolveKeyserverSessionInvalidation, type BindServerCallsParams,
} from '../keyserver-conn/recovery-utils.js'; createBoundServerCallsSelector,
} from '../keyserver-conn/call-keyserver-endpoint-provider.react.js';
import { serverCallStateSelector } from '../selectors/server-calls.js'; import { serverCallStateSelector } from '../selectors/server-calls.js';
import { logInActionSources } from '../types/account-types.js';
import type { PlatformDetails } from '../types/device-types.js';
import type { Endpoint, SocketAPIHandler } from '../types/endpoints.js';
import type { Dispatch } from '../types/redux-types.js';
import type { ClientSessionChange } from '../types/session-types.js';
import type { CurrentUserInfo } from '../types/user-types.js';
let currentlyWaitingForNewCookie = false;
let serverEndpointCallsWaitingForNewCookie: ((
callServerEndpoint: ?CallServerEndpoint,
) => void)[] = [];
// Third param is optional and gets called with newCookie if we get a new cookie
// Necessary to propagate cookie in cookieInvalidationRecovery below
function bindCookieAndUtilsIntoCallServerEndpoint(
params: BindServerCallsParams,
): CallServerEndpoint {
const {
dispatch,
cookie,
urlPrefix,
sessionID,
currentUserInfo,
isSocketConnected,
lastCommunicatedPlatformDetails,
keyserverID,
} = params;
const loggedIn = !!(currentUserInfo && !currentUserInfo.anonymous && true);
const boundSetNewSession = (
sessionChange: ClientSessionChange,
error: ?string,
) =>
setNewSession(
dispatch,
sessionChange,
{
currentUserInfo,
cookiesAndSessions: { [keyserverID]: { cookie, sessionID } },
},
error,
undefined,
keyserverID,
);
const canResolveInvalidation = canResolveKeyserverSessionInvalidation();
// This function gets called before callServerEndpoint sends a request,
// to make sure that we're not in the middle of trying to recover
// an invalidated cookie
const waitIfCookieInvalidated = () => {
if (!canResolveInvalidation) {
// If there is no way to resolve the session invalidation,
// just let the caller callServerEndpoint instance continue
return Promise.resolve(null);
}
if (!currentlyWaitingForNewCookie) {
// Our cookie seems to be valid
return Promise.resolve(null);
}
// Wait to run until we get our new cookie
return new Promise<?CallServerEndpoint>(r =>
serverEndpointCallsWaitingForNewCookie.push(r),
);
};
// This function is a helper for the next function defined below
const attemptToResolveInvalidation = async (
sessionChange: ClientSessionChange,
) => {
const newAnonymousCookie = sessionChange.cookie;
const newSessionChange = await resolveKeyserverSessionInvalidation(
dispatch,
newAnonymousCookie,
urlPrefix,
logInActionSources.cookieInvalidationResolutionAttempt,
keyserverID,
);
currentlyWaitingForNewCookie = false;
const currentWaitingCalls = serverEndpointCallsWaitingForNewCookie;
serverEndpointCallsWaitingForNewCookie = [];
const newCallServerEndpoint = newSessionChange
? bindCookieAndUtilsIntoCallServerEndpoint({
...params,
cookie: newSessionChange.cookie,
sessionID: newSessionChange.sessionID,
currentUserInfo: newSessionChange.currentUserInfo,
})
: null;
for (const func of currentWaitingCalls) {
func(newCallServerEndpoint);
}
return newCallServerEndpoint;
};
// If this function is called, callServerEndpoint got a response invalidating
// its cookie, and is wondering if it should just like... give up?
// Or if there's a chance at redemption
const cookieInvalidationRecovery = (sessionChange: ClientSessionChange) => {
if (!canResolveInvalidation) {
// If there is no way to resolve the session invalidation,
// just let the caller callServerEndpoint instance continue
return Promise.resolve(null);
}
if (!loggedIn) {
// We don't want to attempt any use native credentials of a logged out
// user to log-in after a cookieInvalidation while logged out
return Promise.resolve(null);
}
if (currentlyWaitingForNewCookie) {
return new Promise<?CallServerEndpoint>(r =>
serverEndpointCallsWaitingForNewCookie.push(r),
);
}
currentlyWaitingForNewCookie = true;
return attemptToResolveInvalidation(sessionChange);
};
return (
endpoint: Endpoint,
data: Object,
options?: ?CallServerEndpointOptions,
) =>
callServerEndpoint(
cookie,
boundSetNewSession,
waitIfCookieInvalidated,
cookieInvalidationRecovery,
urlPrefix,
sessionID,
isSocketConnected,
lastCommunicatedPlatformDetails,
socketAPIHandler,
endpoint,
data,
dispatch,
options,
loggedIn,
keyserverID,
);
}
export type ActionFunc<F> = (callServerEndpoint: CallServerEndpoint) => F;
export type BindServerCall = <F>(serverCall: ActionFunc<F>) => F;
export type BindServerCallsParams = {
+dispatch: Dispatch,
+cookie: ?string,
+urlPrefix: string,
+sessionID: ?string,
+currentUserInfo: ?CurrentUserInfo,
+isSocketConnected: boolean,
+lastCommunicatedPlatformDetails: ?PlatformDetails,
+keyserverID: string,
};
// All server calls needs to include some information from the Redux state
// (namely, the cookie). This information is used deep in the server call,
// at the point where callServerEndpoint is called. We don't want to bother
// propagating the cookie (and any future config info that callServerEndpoint
// needs) through to the server calls so they can pass it to callServerEndpoint.
// Instead, we "curry" the cookie onto callServerEndpoint within react-redux's
// connect's mapStateToProps function, and then pass that "bound"
// callServerEndpoint that no longer needs the cookie as a parameter on to
// the server call.
const baseCreateBoundServerCallsSelector = <F>(
actionFunc: ActionFunc<F>,
): (BindServerCallsParams => F) =>
createSelector(
(state: BindServerCallsParams) => state.dispatch,
(state: BindServerCallsParams) => state.cookie,
(state: BindServerCallsParams) => state.urlPrefix,
(state: BindServerCallsParams) => state.sessionID,
(state: BindServerCallsParams) => state.currentUserInfo,
(state: BindServerCallsParams) => state.isSocketConnected,
(state: BindServerCallsParams) => state.lastCommunicatedPlatformDetails,
(state: BindServerCallsParams) => state.keyserverID,
(
dispatch: Dispatch,
cookie: ?string,
urlPrefix: string,
sessionID: ?string,
currentUserInfo: ?CurrentUserInfo,
isSocketConnected: boolean,
lastCommunicatedPlatformDetails: ?PlatformDetails,
keyserverID: string,
) => {
const boundCallServerEndpoint = bindCookieAndUtilsIntoCallServerEndpoint({
dispatch,
cookie,
urlPrefix,
sessionID,
currentUserInfo,
isSocketConnected,
lastCommunicatedPlatformDetails,
keyserverID,
});
return actionFunc(boundCallServerEndpoint);
},
);
type CreateBoundServerCallsSelectorType = <F>(
ActionFunc<F>,
) => BindServerCallsParams => F;
const createBoundServerCallsSelector: CreateBoundServerCallsSelectorType =
(_memoize(baseCreateBoundServerCallsSelector): any);
function useServerCall<F>( function useServerCall<F>(
serverCall: ActionFunc<F>, serverCall: ActionFunc<F>,
paramOverride?: ?Partial<BindServerCallsParams>, paramOverride?: ?Partial<BindServerCallsParams>,
): F { ): F {
const dispatch = useDispatch(); const dispatch = useDispatch();
const serverCallState = useSelector( const serverCallState = useSelector(
serverCallStateSelector(ashoatKeyserverID), serverCallStateSelector(ashoatKeyserverID),
Show All 13 Lines return createBoundServerCallsSelector(serverCall)({
isSocketConnected, isSocketConnected,
dispatch, dispatch,
...paramOverride, ...paramOverride,
keyserverID: ashoatKeyserverID, keyserverID: ashoatKeyserverID,
}); });
}, [serverCall, serverCallState, dispatch, paramOverride]); }, [serverCall, serverCallState, dispatch, paramOverride]);
} }
let socketAPIHandler: ?SocketAPIHandler = null; export { useServerCall };
function registerActiveSocket(passedSocketAPIHandler: ?SocketAPIHandler) {
socketAPIHandler = passedSocketAPIHandler;
}
export {
createBoundServerCallsSelector,
registerActiveSocket,
useServerCall,
bindCookieAndUtilsIntoCallServerEndpoint,
};