add loading state

matrix-org · Sep 14, 2022 · 96e138a · 96e138a
1 parent 21907a7
commit 96e138a
Show file tree

Hide file tree

Showing 7 changed files with 80 additions and 18 deletions.
diff --git a/res/css/components/views/settings/devices/_DeviceDetailHeading.pcss b/res/css/components/views/settings/devices/_DeviceDetailHeading.pcss
@@ -33,7 +33,14 @@ limitations under the License.
 }
 
 .mx_DeviceDetailHeading_renameFormButtons {
+    display: flex;
+    flex-direction: row;
     gap: $spacing-8;
+
+    .mx_Spinner {
+        width: auto;
+        flex-grow: 0;
+    }
 }
 
 .mx_DeviceDetailHeading_renameFormInput {

diff --git a/src/components/views/settings/devices/CurrentDeviceSection.tsx b/src/components/views/settings/devices/CurrentDeviceSection.tsx
@@ -31,6 +31,7 @@ interface Props {
     isSigningOut: boolean;
     onVerifyCurrentDevice: () => void;
     onSignOutCurrentDevice: () => void;
+    onSaveDeviceName: (deviceName: string) => Promise<void>;
 }
 
 const CurrentDeviceSection: React.FC<Props> = ({
@@ -39,14 +40,16 @@ const CurrentDeviceSection: React.FC<Props> = ({
     isSigningOut,
     onVerifyCurrentDevice,
     onSignOutCurrentDevice,
+    onSaveDeviceName,
 }) => {
     const [isExpanded, setIsExpanded] = useState(false);
 
     return <SettingsSubsection
         heading={_t('Current session')}
         data-testid='current-session-section'
     >
-        { isLoading && <Spinner /> }
+        { /* only show big spinner on first load */ }
+        { isLoading && !device && <Spinner /> }
         { !!device && <>
             <DeviceTile
                 device={device}
@@ -61,7 +64,9 @@ const CurrentDeviceSection: React.FC<Props> = ({
                 <DeviceDetails
                     device={device}
                     isSigningOut={isSigningOut}
+                    isLoading={isLoading}
                     onSignOutDevice={onSignOutCurrentDevice}
+                    onSaveDeviceName={onSaveDeviceName}
                 />
             }
             <br />

diff --git a/src/components/views/settings/devices/DeviceDetailHeading.tsx b/src/components/views/settings/devices/DeviceDetailHeading.tsx
@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import React, { useEffect, useState } from 'react';
+import React, { FormEvent, useEffect, useState } from 'react';
 
 import { _t } from '../../../../languageHandler';
 import AccessibleButton from '../../elements/AccessibleButton';
@@ -27,7 +27,7 @@ import { DeviceWithVerification } from './types';
 interface Props {
     device: DeviceWithVerification;
     isLoading: boolean;
-    saveDeviceName: (deviceName: string) => void;
+    saveDeviceName: (deviceName: string) => Promise<void>;
 }
 
 const DeviceNameEditor: React.FC<Props & { stopEditing: () => void }> = ({
@@ -37,19 +37,26 @@ const DeviceNameEditor: React.FC<Props & { stopEditing: () => void }> = ({
 
     useEffect(() => {
         setDeviceName(device.display_name);
-    }, [device]);
+    }, [device.display_name]);
 
     const onInputChange = (event: React.ChangeEvent<HTMLInputElement>): void =>
         setDeviceName(event.target.value);
 
-    const onSubmit = () => saveDeviceName(deviceName);
+    const onSubmit = async (event: FormEvent<HTMLFormElement>) => {
+        event.preventDefault();
+        await saveDeviceName(deviceName);
+        stopEditing();
+    };
+
     const headingId = `device-rename-${device.device_id}`;
     const descriptionId = `device-rename-description-${device.device_id}`;
 
     return <form
         aria-disabled={isLoading}
         className="mx_DeviceDetailHeading_renameForm"
-        onSubmit={onSubmit}>
+        onSubmit={onSubmit}
+        method="post"
+    >
         <p
             id={headingId}
             className="mx_DeviceDetailHeading_renameFormHeading"
@@ -68,6 +75,7 @@ const DeviceNameEditor: React.FC<Props & { stopEditing: () => void }> = ({
                 aria-labelledby={headingId}
                 aria-describedby={descriptionId}
                 className="mx_DeviceDetailHeading_renameFormInput"
+                maxLength={100}
             />
             <Caption
                 id={descriptionId}
@@ -84,13 +92,15 @@ const DeviceNameEditor: React.FC<Props & { stopEditing: () => void }> = ({
             >
                 { _t('Save') }
             </AccessibleButton>
-            { isLoading && <Spinner w={16} h={16} /> }
             <AccessibleButton
                 onClick={stopEditing}
                 kind="secondary"
                 data-testid='device-rename-cancel-cta'
                 disabled={isLoading}
-            >{ _t('Cancel') }</AccessibleButton>
+            >
+                { _t('Cancel') }
+            </AccessibleButton>
+            { isLoading && <Spinner w={16} h={16} /> }
         </div>
     </form>;
 };
@@ -99,6 +109,7 @@ export const DeviceDetailHeading: React.FC<Props> = ({
     device, isLoading, saveDeviceName,
 }) => {
     const [isEditing, setIsEditing] = useState(false);
+
     return isEditing
         ? <DeviceNameEditor
             device={device}

diff --git a/src/components/views/settings/devices/DeviceDetails.tsx b/src/components/views/settings/devices/DeviceDetails.tsx
@@ -27,6 +27,7 @@ import { DeviceWithVerification } from './types';
 interface Props {
     device: DeviceWithVerification;
     isSigningOut: boolean;
+    isLoading: boolean;
     onVerifyDevice?: () => void;
     onSignOutDevice: () => void;
     onSetDeviceName: (deviceName: string) => void;
@@ -40,6 +41,7 @@ interface MetadataTable {
 const DeviceDetails: React.FC<Props> = ({
     device,
     isSigningOut,
+    isLoading,
     onVerifyDevice,
     onSignOutDevice,
     onSaveDeviceName,
@@ -65,7 +67,7 @@ const DeviceDetails: React.FC<Props> = ({
         <section className='mx_DeviceDetails_section'>
             <DeviceDetailHeading
                 device={device}
-                isLoading={false}
+                isLoading={isLoading}
                 saveDeviceName={onSaveDeviceName}
             />
             <DeviceVerificationStatusCard

diff --git a/src/components/views/settings/devices/useOwnDevices.ts b/src/components/views/settings/devices/useOwnDevices.ts
@@ -23,6 +23,7 @@ import { logger } from "matrix-js-sdk/src/logger";
 
 import MatrixClientContext from "../../../../contexts/MatrixClientContext";
 import { DevicesDictionary, DeviceWithVerification } from "./types";
+import { _t } from "../../../../languageHandler";
 
 const isDeviceVerified = (
     matrixClient: MatrixClient,
@@ -76,10 +77,13 @@ export enum OwnDevicesError {
 export type DevicesState = {
     devices: DevicesDictionary;
     currentDeviceId: string;
-    isLoading: boolean;
+    isLoadingDeviceList: boolean;
+    // device ids with pending requests
+    pendingDeviceIds: DeviceWithVerification['device_id'][];
     // not provided when current session cannot request verification
     requestDeviceVerification?: (deviceId: DeviceWithVerification['device_id']) => Promise<VerificationRequest>;
     refreshDevices: () => Promise<void>;
+    saveDeviceName: (deviceId: DeviceWithVerification['device_id'], deviceName: string) => Promise<void>;
     error?: OwnDevicesError;
 };
 export const useOwnDevices = (): DevicesState => {
@@ -89,11 +93,14 @@ export const useOwnDevices = (): DevicesState => {
     const userId = matrixClient.getUserId();
 
     const [devices, setDevices] = useState<DevicesState['devices']>({});
-    const [isLoading, setIsLoading] = useState(true);
+    const [isLoadingDeviceList, setIsLoadingDeviceList] = useState(true);
+
+    // device ids with pending requests
+    const [pendingDeviceIds, setPendingDeviceIds] = useState<DeviceWithVerification['device_id'][]>([]);
     const [error, setError] = useState<OwnDevicesError>();
 
     const refreshDevices = useCallback(async () => {
-        setIsLoading(true);
+        setIsLoadingDeviceList(true);
         try {
             // realistically we should never hit this
             // but it satisfies types
@@ -102,7 +109,7 @@ export const useOwnDevices = (): DevicesState => {
             }
             const devices = await fetchDevicesWithVerification(matrixClient, userId);
             setDevices(devices);
-            setIsLoading(false);
+            setIsLoadingDeviceList(false);
         } catch (error) {
             if ((error as MatrixError).httpStatus == 404) {
                 // 404 probably means the HS doesn't yet support the API.
@@ -111,7 +118,7 @@ export const useOwnDevices = (): DevicesState => {
                 logger.error("Error loading sessions:", error);
                 setError(OwnDevicesError.Default);
             }
-            setIsLoading(false);
+            setIsLoadingDeviceList(false);
         }
     }, [matrixClient, userId]);
 
@@ -130,12 +137,37 @@ export const useOwnDevices = (): DevicesState => {
         }
         : undefined;
 
+    const saveDeviceName = useCallback(
+        async (deviceId: DeviceWithVerification['device_id'], deviceName: string): Promise<void> => {
+            const device = devices[deviceId];
+
+            // no change
+            if (deviceName === device?.display_name) {
+                return;
+            }
+
+            try {
+                setPendingDeviceIds(p => ([...p, deviceId]));
+                await matrixClient.setDeviceDetails(
+                    deviceId,
+                    { display_name: deviceName },
+                );
+                await refreshDevices();
+            } catch (error) {
+                logger.error("Error setting session display name", error);
+                throw new Error(_t("Failed to set display name"));
+            }
+            setPendingDeviceIds(p => p.filter(id => id !== deviceId));
+        }, [matrixClient, devices, refreshDevices, setPendingDeviceIds]);
+
     return {
         devices,
         currentDeviceId,
+        isLoadingDeviceList,
+        error,
+        pendingDeviceIds,
         requestDeviceVerification,
         refreshDevices,
-        isLoading,
-        error,
+        saveDeviceName,
     };
 };
diff --git a/src/components/views/settings/tabs/user/SessionManagerTab.tsx b/src/components/views/settings/tabs/user/SessionManagerTab.tsx
@@ -88,9 +88,11 @@ const SessionManagerTab: React.FC = () => {
     const {
         devices,
         currentDeviceId,
-        isLoading,
+        isLoadingDeviceList,
+        pendingDeviceIds,
         requestDeviceVerification,
         refreshDevices,
+        saveDeviceName,
     } = useOwnDevices();
     const [filter, setFilter] = useState<DeviceSecurityVariation>();
     const [expandedDeviceIds, setExpandedDeviceIds] = useState<DeviceWithVerification['device_id'][]>([]);
@@ -167,8 +169,9 @@ const SessionManagerTab: React.FC = () => {
         />
         <CurrentDeviceSection
             device={currentDevice}
-            isLoading={isLoading}
             isSigningOut={signingOutDeviceIds.includes(currentDevice?.device_id)}
+            isLoading={isLoadingDeviceList || pendingDeviceIds.includes(currentDevice.device_id)}
+            onSaveDeviceName={(deviceName) => saveDeviceName(currentDevice.device_id, deviceName)}
             onVerifyCurrentDevice={onVerifyCurrentDevice}
             onSignOutCurrentDevice={onSignOutCurrentDevice}
         />

diff --git a/test/components/views/settings/devices/__snapshots__/DeviceDetailHeading-test.tsx.snap b/test/components/views/settings/devices/__snapshots__/DeviceDetailHeading-test.tsx.snap
@@ -6,6 +6,7 @@ Object {
     <form
       aria-disabled="false"
       class="mx_DeviceDetailHeading_renameForm"
+      method="post"
     >
       <p
         class="mx_DeviceDetailHeading_renameFormHeading"
@@ -23,6 +24,7 @@ Object {
             autocomplete="off"
             data-testid="device-rename-input"
             id="mx_Field_1"
+            maxlength="100"
             type="text"
             value="My device"
           />