From 056eed21c5f43696e6532b4f2fc66b2075fbdac4 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 14 Sep 2022 14:51:59 +0200 Subject: [PATCH 01/12] split heading into component --- res/css/_components.pcss | 1 + .../devices/_DeviceDetailHeading.pcss | 19 ++++++++++ .../settings/devices/DeviceDetailHeading.tsx | 34 ++++++++++++++++++ .../views/settings/devices/DeviceDetails.tsx | 10 ++++-- .../devices/DeviceDetailHeading-test.tsx | 30 ++++++++++++++++ .../CurrentDeviceSection-test.tsx.snap | 12 ++++--- .../__snapshots__/DeviceDetails-test.tsx.snap | 36 ++++++++++++------- 7 files changed, 124 insertions(+), 18 deletions(-) create mode 100644 res/css/components/views/settings/devices/_DeviceDetailHeading.pcss create mode 100644 src/components/views/settings/devices/DeviceDetailHeading.tsx create mode 100644 test/components/views/settings/devices/DeviceDetailHeading-test.tsx diff --git a/res/css/_components.pcss b/res/css/_components.pcss index b6b98aee3e8..e4b9c999f01 100644 --- a/res/css/_components.pcss +++ b/res/css/_components.pcss @@ -28,6 +28,7 @@ @import "./components/views/location/_ZoomButtons.pcss"; @import "./components/views/messages/_MBeaconBody.pcss"; @import "./components/views/messages/shared/_MediaProcessingError.pcss"; +@import "./components/views/settings/devices/_DeviceDetailHeading.pcss"; @import "./components/views/settings/devices/_DeviceDetails.pcss"; @import "./components/views/settings/devices/_DeviceExpandDetailsButton.pcss"; @import "./components/views/settings/devices/_DeviceSecurityCard.pcss"; diff --git a/res/css/components/views/settings/devices/_DeviceDetailHeading.pcss b/res/css/components/views/settings/devices/_DeviceDetailHeading.pcss new file mode 100644 index 00000000000..6ca86f6ec54 --- /dev/null +++ b/res/css/components/views/settings/devices/_DeviceDetailHeading.pcss @@ -0,0 +1,19 @@ +/* +Copyright 2022 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +.mx_DeviceDetailHeading { + +} diff --git a/src/components/views/settings/devices/DeviceDetailHeading.tsx b/src/components/views/settings/devices/DeviceDetailHeading.tsx new file mode 100644 index 00000000000..92659aec820 --- /dev/null +++ b/src/components/views/settings/devices/DeviceDetailHeading.tsx @@ -0,0 +1,34 @@ +/* +Copyright 2022 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import React from 'react'; + +import Heading from '../../typography/Heading'; +import { DeviceWithVerification } from './types'; + +interface Props { + device: DeviceWithVerification; + isLoading: boolean; + setDeviceName: (deviceName: string) => void; +} + +export const DeviceDetailHeading: React.FC = ({ + device, isLoading, setDeviceName, +}) => { + return
+ { device.display_name ?? device.device_id } +
; +}; diff --git a/src/components/views/settings/devices/DeviceDetails.tsx b/src/components/views/settings/devices/DeviceDetails.tsx index c773e2cfdbf..07d385f38bf 100644 --- a/src/components/views/settings/devices/DeviceDetails.tsx +++ b/src/components/views/settings/devices/DeviceDetails.tsx @@ -20,7 +20,7 @@ import { formatDate } from '../../../../DateUtils'; import { _t } from '../../../../languageHandler'; import AccessibleButton from '../../elements/AccessibleButton'; import Spinner from '../../elements/Spinner'; -import Heading from '../../typography/Heading'; +import { DeviceDetailHeading } from './DeviceDetailHeading'; import { DeviceVerificationStatusCard } from './DeviceVerificationStatusCard'; import { DeviceWithVerification } from './types'; @@ -29,6 +29,7 @@ interface Props { isSigningOut: boolean; onVerifyDevice?: () => void; onSignOutDevice: () => void; + onSetDeviceName: (deviceName: string) => void; } interface MetadataTable { @@ -41,6 +42,7 @@ const DeviceDetails: React.FC = ({ isSigningOut, onVerifyDevice, onSignOutDevice, + onSetDeviceName, }) => { const metadata: MetadataTable[] = [ { @@ -61,7 +63,11 @@ const DeviceDetails: React.FC = ({ ]; return
- { device.display_name ?? device.device_id } + ', () => { + const defaultProps = {}; + const getComponent = (props = {}) => + ; + + it('renders', () => { + const component = getComponent(); + expect(component).toBeTruthy(); + }); +}); diff --git a/test/components/views/settings/devices/__snapshots__/CurrentDeviceSection-test.tsx.snap b/test/components/views/settings/devices/__snapshots__/CurrentDeviceSection-test.tsx.snap index a3c9d7de2b3..b02d77d5984 100644 --- a/test/components/views/settings/devices/__snapshots__/CurrentDeviceSection-test.tsx.snap +++ b/test/components/views/settings/devices/__snapshots__/CurrentDeviceSection-test.tsx.snap @@ -9,11 +9,15 @@ HTMLCollection [
-

- alices_device -

+

+ alices_device +

+
diff --git a/test/components/views/settings/devices/__snapshots__/DeviceDetails-test.tsx.snap b/test/components/views/settings/devices/__snapshots__/DeviceDetails-test.tsx.snap index 6de93f65af0..6d01237619d 100644 --- a/test/components/views/settings/devices/__snapshots__/DeviceDetails-test.tsx.snap +++ b/test/components/views/settings/devices/__snapshots__/DeviceDetails-test.tsx.snap @@ -9,11 +9,15 @@ exports[` renders a verified device 1`] = `
-

- my-device -

+

+ my-device +

+
@@ -130,11 +134,15 @@ exports[` renders device with metadata 1`] = `
-

- My Device -

+

+ My Device +

+
@@ -255,11 +263,15 @@ exports[` renders device without metadata 1`] = `
-

- my-device -

+

+ my-device +

+
From 1b2cd71666bf7fca98f4c8b812a97a82d92a4aa2 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 14 Sep 2022 15:13:58 +0200 Subject: [PATCH 02/12] switch between editing and view --- .../devices/_DeviceDetailHeading.pcss | 7 +++ .../settings/devices/_DeviceDetails.pcss | 1 + .../settings/devices/DeviceDetailHeading.tsx | 53 ++++++++++++++++--- .../views/settings/devices/DeviceDetails.tsx | 4 +- 4 files changed, 57 insertions(+), 8 deletions(-) diff --git a/res/css/components/views/settings/devices/_DeviceDetailHeading.pcss b/res/css/components/views/settings/devices/_DeviceDetailHeading.pcss index 6ca86f6ec54..7eca7174966 100644 --- a/res/css/components/views/settings/devices/_DeviceDetailHeading.pcss +++ b/res/css/components/views/settings/devices/_DeviceDetailHeading.pcss @@ -15,5 +15,12 @@ limitations under the License. */ .mx_DeviceDetailHeading { + display: flex; + flex-direction: row; + justify-content: space-between; + align-items: center; +} +.mx_DeviceDetailHeading_renameCta { + flex-shrink: 0; } diff --git a/res/css/components/views/settings/devices/_DeviceDetails.pcss b/res/css/components/views/settings/devices/_DeviceDetails.pcss index 47766cec459..ebb725d28ea 100644 --- a/res/css/components/views/settings/devices/_DeviceDetails.pcss +++ b/res/css/components/views/settings/devices/_DeviceDetails.pcss @@ -35,6 +35,7 @@ limitations under the License. display: grid; grid-gap: $spacing-16; justify-content: left; + grid-template-columns: 100%; &:last-child { padding-bottom: 0; diff --git a/src/components/views/settings/devices/DeviceDetailHeading.tsx b/src/components/views/settings/devices/DeviceDetailHeading.tsx index 92659aec820..f2b6a8f87ba 100644 --- a/src/components/views/settings/devices/DeviceDetailHeading.tsx +++ b/src/components/views/settings/devices/DeviceDetailHeading.tsx @@ -14,21 +14,62 @@ See the License for the specific language governing permissions and limitations under the License. */ -import React from 'react'; +import React, { useEffect, useState } from 'react'; +import { _t } from '../../../../languageHandler'; +import AccessibleButton from '../../elements/AccessibleButton'; import Heading from '../../typography/Heading'; import { DeviceWithVerification } from './types'; interface Props { device: DeviceWithVerification; isLoading: boolean; - setDeviceName: (deviceName: string) => void; + saveDeviceName: (deviceName: string) => void; } -export const DeviceDetailHeading: React.FC = ({ - device, isLoading, setDeviceName, +const DeviceNameEditor: React.FC void }> = ({ + device, isLoading, saveDeviceName, stopEditing, }) => { - return
- { device.display_name ?? device.device_id } + const [deviceName, setDeviceName] = useState(device.display_name || ''); + + useEffect(() => { + setDeviceName(device.display_name); + }, [device]); + + return
+ { deviceName } + saveDeviceName('test')} + >{ _t('Save') } + + { _t('Cancel') } +
; }; + +export const DeviceDetailHeading: React.FC = ({ + device, isLoading, saveDeviceName, +}) => { + const [isEditing, setIsEditing] = useState(false); + return isEditing + ? setIsEditing(false)} + /> + :
+ { device.display_name ?? device.device_id } + setIsEditing(true)} + className='mx_DeviceDetailHeading_renameCta' + > + { _t('Rename') } + +
; +}; diff --git a/src/components/views/settings/devices/DeviceDetails.tsx b/src/components/views/settings/devices/DeviceDetails.tsx index 07d385f38bf..61e556d0fde 100644 --- a/src/components/views/settings/devices/DeviceDetails.tsx +++ b/src/components/views/settings/devices/DeviceDetails.tsx @@ -42,7 +42,7 @@ const DeviceDetails: React.FC = ({ isSigningOut, onVerifyDevice, onSignOutDevice, - onSetDeviceName, + onSaveDeviceName, }) => { const metadata: MetadataTable[] = [ { @@ -66,7 +66,7 @@ const DeviceDetails: React.FC = ({ Date: Wed, 14 Sep 2022 15:36:05 +0200 Subject: [PATCH 03/12] style file --- .../views/settings/devices/_DeviceDetailHeading.pcss | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/res/css/components/views/settings/devices/_DeviceDetailHeading.pcss b/res/css/components/views/settings/devices/_DeviceDetailHeading.pcss index 7eca7174966..841f764be34 100644 --- a/res/css/components/views/settings/devices/_DeviceDetailHeading.pcss +++ b/res/css/components/views/settings/devices/_DeviceDetailHeading.pcss @@ -24,3 +24,7 @@ limitations under the License. .mx_DeviceDetailHeading_renameCta { flex-shrink: 0; } + +.mx_DeviceDetailHeading_editor { + display: grid; +} From 291234613b7efcb1de313838c5cccb82a3195fd8 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 14 Sep 2022 16:11:21 +0200 Subject: [PATCH 04/12] basic tests --- .../settings/devices/DeviceDetailHeading.tsx | 40 ++++++-- .../devices/DeviceDetailHeading-test.tsx | 85 ++++++++++++++++- .../DeviceDetailHeading-test.tsx.snap | 91 +++++++++++++++++++ 3 files changed, 201 insertions(+), 15 deletions(-) create mode 100644 test/components/views/settings/devices/__snapshots__/DeviceDetailHeading-test.tsx.snap diff --git a/src/components/views/settings/devices/DeviceDetailHeading.tsx b/src/components/views/settings/devices/DeviceDetailHeading.tsx index f2b6a8f87ba..a3b7adadcfc 100644 --- a/src/components/views/settings/devices/DeviceDetailHeading.tsx +++ b/src/components/views/settings/devices/DeviceDetailHeading.tsx @@ -18,6 +18,7 @@ import React, { useEffect, useState } from 'react'; import { _t } from '../../../../languageHandler'; import AccessibleButton from '../../elements/AccessibleButton'; +import Field from '../../elements/Field'; import Heading from '../../typography/Heading'; import { DeviceWithVerification } from './types'; @@ -36,19 +37,37 @@ const DeviceNameEditor: React.FC void }> = ({ setDeviceName(device.display_name); }, [device]); - return
- { deviceName } + const onInputChange = (event: React.ChangeEvent): void => + setDeviceName(event.target.value); + + const onSubmit = () => saveDeviceName(deviceName); + + return
+ saveDeviceName('test')} - >{ _t('Save') } - + onClick={onSubmit} + kind="confirm" + data-testid='device-rename-submit-cta' + disabled={isLoading} + >{ _t('Save') } { _t('Cancel') } - -
; + kind="cancel" + data-testid='device-rename-cancel-cta' + disabled={isLoading} + >{ _t('Cancel') } + ; }; export const DeviceDetailHeading: React.FC = ({ @@ -68,6 +87,7 @@ export const DeviceDetailHeading: React.FC = ({ kind='link_inline' onClick={() => setIsEditing(true)} className='mx_DeviceDetailHeading_renameCta' + data-testid='device-heading-rename-cta' > { _t('Rename') } diff --git a/test/components/views/settings/devices/DeviceDetailHeading-test.tsx b/test/components/views/settings/devices/DeviceDetailHeading-test.tsx index bc7600b74d2..33e690ba7b2 100644 --- a/test/components/views/settings/devices/DeviceDetailHeading-test.tsx +++ b/test/components/views/settings/devices/DeviceDetailHeading-test.tsx @@ -14,17 +14,92 @@ See the License for the specific language governing permissions and limitations under the License. */ +import { fireEvent, render, RenderResult } from '@testing-library/react'; import React from 'react'; -import DeviceDetailHeading from '../../../../../src/components/views/settings/devices/DeviceDetailHeading'; +import {DeviceDetailHeading} from '../../../../../src/components/views/settings/devices/DeviceDetailHeading'; describe('', () => { - const defaultProps = {}; + const device = { + device_id: '123', + display_name: 'My device', + isVerified: true, + }; + const defaultProps = { + device, + isLoading: false, + saveDeviceName: jest.fn(), + }; const getComponent = (props = {}) => ; - it('renders', () => { - const component = getComponent(); - expect(component).toBeTruthy(); + const setInputValue = (getByTestId: RenderResult['getByTestId'] , value: string) => { + const input = getByTestId('device-rename-input'); + + fireEvent.change(input, { target: { value }}); + } + + it('renders device name', () => { + const { container } = render(getComponent()); + expect({ container }).toMatchSnapshot(); + }); + + it('renders device id as fallback when device has no display name ', () => { + const { container } = render(getComponent({ + device: { ...device, display_name: undefined }, + })); + expect({ container }).toMatchSnapshot(); + }); + + it('displays name edit form on rename button click', () => { + const { getByTestId, container } = render(getComponent()); + + fireEvent.click(getByTestId('device-heading-rename-cta')); + + expect({ container }).toMatchSnapshot(); + }); + + it('cancelling edit switches back to original display', () => { + const { getByTestId, container } = render(getComponent()); + + // start editing + fireEvent.click(getByTestId('device-heading-rename-cta')); + + // stop editing + fireEvent.click(getByTestId('device-rename-cancel-cta')); + + expect(container.getElementsByClassName('mx_DeviceDetailHeading').length).toBe(1); + }); + + it('disables form while device isLoading', () => { + const { getByTestId, rerender } = render(getComponent()); + + // start editing + fireEvent.click(getByTestId('device-heading-rename-cta')); + + // enter loading state + rerender(getComponent({ isLoading: true })); + + // buttons disabled + expect( + getByTestId('device-rename-cancel-cta').getAttribute('aria-disabled'), + ).toEqual("true"); + expect( + getByTestId('device-rename-submit-cta').getAttribute('aria-disabled'), + ).toEqual("true"); + }); + + it('clicking submit updates device name with edited value', () => { + const saveDeviceName = jest.fn(); + const { getByTestId } = render(getComponent({ saveDeviceName })); + + // start editing + fireEvent.click(getByTestId('device-heading-rename-cta')); + + setInputValue(getByTestId, 'new device name'); + + fireEvent.click(getByTestId('device-rename-submit-cta')); + + expect(saveDeviceName).toHaveBeenCalledWith('new device name'); }); }); diff --git a/test/components/views/settings/devices/__snapshots__/DeviceDetailHeading-test.tsx.snap b/test/components/views/settings/devices/__snapshots__/DeviceDetailHeading-test.tsx.snap new file mode 100644 index 00000000000..84b351814ff --- /dev/null +++ b/test/components/views/settings/devices/__snapshots__/DeviceDetailHeading-test.tsx.snap @@ -0,0 +1,91 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[` displays name edit form on rename button click 1`] = ` +Object { + "container":
+
+
+ +
+
+ Save +
+
+ Cancel +
+
+
, +} +`; + +exports[` renders device id as fallback when device has no display name 1`] = ` +Object { + "container":
+
+

+ 123 +

+
+ Rename +
+
+
, +} +`; + +exports[` renders device name 1`] = ` +Object { + "container":
+
+

+ My device +

+
+ Rename +
+
+
, +} +`; From 2bfef5d34d370b44ce546a071db39a5db34b6f81 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 14 Sep 2022 16:38:20 +0200 Subject: [PATCH 05/12] style device rename component --- .../devices/_DeviceDetailHeading.pcss | 18 ++++- .../settings/devices/DeviceDetailHeading.tsx | 67 ++++++++++++------ src/i18n/strings/en_EN.json | 2 + .../devices/DeviceDetailHeading-test.tsx | 8 +-- .../CurrentDeviceSection-test.tsx.snap | 8 +++ .../DeviceDetailHeading-test.tsx.snap | 70 ++++++++++++------- .../__snapshots__/DeviceDetails-test.tsx.snap | 24 +++++++ 7 files changed, 146 insertions(+), 51 deletions(-) diff --git a/res/css/components/views/settings/devices/_DeviceDetailHeading.pcss b/res/css/components/views/settings/devices/_DeviceDetailHeading.pcss index 841f764be34..06c2f3882bb 100644 --- a/res/css/components/views/settings/devices/_DeviceDetailHeading.pcss +++ b/res/css/components/views/settings/devices/_DeviceDetailHeading.pcss @@ -25,6 +25,22 @@ limitations under the License. flex-shrink: 0; } -.mx_DeviceDetailHeading_editor { +.mx_DeviceDetailHeading_renameForm { display: grid; + grid-gap: $spacing-16; + justify-content: left; + grid-template-columns: 100%; } + +.mx_DeviceDetailHeading_renameFormButtons { + gap: $spacing-8; +} + +.mx_DeviceDetailHeading_renameFormInput { + // override field styles + margin: 0 0 $spacing-4 0 !important; +} + +.mx_DeviceDetailHeading_renameFormHeading { + margin: 0; +} \ No newline at end of file diff --git a/src/components/views/settings/devices/DeviceDetailHeading.tsx b/src/components/views/settings/devices/DeviceDetailHeading.tsx index a3b7adadcfc..c1c959ad552 100644 --- a/src/components/views/settings/devices/DeviceDetailHeading.tsx +++ b/src/components/views/settings/devices/DeviceDetailHeading.tsx @@ -19,6 +19,8 @@ import React, { useEffect, useState } from 'react'; import { _t } from '../../../../languageHandler'; import AccessibleButton from '../../elements/AccessibleButton'; import Field from '../../elements/Field'; +import Spinner from '../../elements/Spinner'; +import { Caption } from '../../typography/Caption'; import Heading from '../../typography/Heading'; import { DeviceWithVerification } from './types'; @@ -41,32 +43,55 @@ const DeviceNameEditor: React.FC void }> = ({ setDeviceName(event.target.value); const onSubmit = () => saveDeviceName(deviceName); + const headingId = `device-rename-${device.device_id}`; + const descriptionId = `device-rename-description-${device.device_id}`; return
- - { _t('Save') } - { _t('Cancel') } +

+ { _t('Rename session') } +

+
+ + + { _t('Please be aware that session names are also visible to people you communicate with') } + +
+
+ + { _t('Save') } + + { isLoading && } + { _t('Cancel') } +
; }; diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index ab35c67f368..214db0000c3 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -1707,6 +1707,8 @@ "Sign out devices|other": "Sign out devices", "Sign out devices|one": "Sign out device", "Authentication": "Authentication", + "Rename session": "Rename session", + "Please be aware that session names are also visible to people you communicate with": "Please be aware that session names are also visible to people you communicate with", "Session ID": "Session ID", "Last activity": "Last activity", "Device": "Device", diff --git a/test/components/views/settings/devices/DeviceDetailHeading-test.tsx b/test/components/views/settings/devices/DeviceDetailHeading-test.tsx index 33e690ba7b2..64b00203cbd 100644 --- a/test/components/views/settings/devices/DeviceDetailHeading-test.tsx +++ b/test/components/views/settings/devices/DeviceDetailHeading-test.tsx @@ -17,7 +17,7 @@ limitations under the License. import { fireEvent, render, RenderResult } from '@testing-library/react'; import React from 'react'; -import {DeviceDetailHeading} from '../../../../../src/components/views/settings/devices/DeviceDetailHeading'; +import { DeviceDetailHeading } from '../../../../../src/components/views/settings/devices/DeviceDetailHeading'; describe('', () => { const device = { @@ -33,11 +33,11 @@ describe('', () => { const getComponent = (props = {}) => ; - const setInputValue = (getByTestId: RenderResult['getByTestId'] , value: string) => { + const setInputValue = (getByTestId: RenderResult['getByTestId'], value: string) => { const input = getByTestId('device-rename-input'); - fireEvent.change(input, { target: { value }}); - } + fireEvent.change(input, { target: { value } }); + }; it('renders device name', () => { const { container } = render(getComponent()); diff --git a/test/components/views/settings/devices/__snapshots__/CurrentDeviceSection-test.tsx.snap b/test/components/views/settings/devices/__snapshots__/CurrentDeviceSection-test.tsx.snap index b02d77d5984..f0aef065e2e 100644 --- a/test/components/views/settings/devices/__snapshots__/CurrentDeviceSection-test.tsx.snap +++ b/test/components/views/settings/devices/__snapshots__/CurrentDeviceSection-test.tsx.snap @@ -17,6 +17,14 @@ HTMLCollection [ > alices_device +
+ Rename +
-
- -
-
- Save + Rename session +

+
+
+ +
+ + Please be aware that session names are also visible to people you communicate with +
- Cancel +
+ Save +
+
+ Cancel +
, diff --git a/test/components/views/settings/devices/__snapshots__/DeviceDetails-test.tsx.snap b/test/components/views/settings/devices/__snapshots__/DeviceDetails-test.tsx.snap index 6d01237619d..f29e7b22de0 100644 --- a/test/components/views/settings/devices/__snapshots__/DeviceDetails-test.tsx.snap +++ b/test/components/views/settings/devices/__snapshots__/DeviceDetails-test.tsx.snap @@ -17,6 +17,14 @@ exports[` renders a verified device 1`] = ` > my-device +
+ Rename +
renders device with metadata 1`] = ` > My Device +
+ Rename +
renders device without metadata 1`] = ` > my-device +
+ Rename +
Date: Wed, 14 Sep 2022 17:39:27 +0200 Subject: [PATCH 06/12] add loading state --- .../devices/_DeviceDetailHeading.pcss | 7 +++ .../settings/devices/CurrentDeviceSection.tsx | 7 ++- .../settings/devices/DeviceDetailHeading.tsx | 25 +++++++--- .../views/settings/devices/DeviceDetails.tsx | 4 +- .../views/settings/devices/useOwnDevices.ts | 46 ++++++++++++++++--- .../settings/tabs/user/SessionManagerTab.tsx | 7 ++- .../DeviceDetailHeading-test.tsx.snap | 2 + 7 files changed, 80 insertions(+), 18 deletions(-) diff --git a/res/css/components/views/settings/devices/_DeviceDetailHeading.pcss b/res/css/components/views/settings/devices/_DeviceDetailHeading.pcss index 06c2f3882bb..107eefdb6e5 100644 --- 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 index e720b47ede9..b4cc39a8e3c 100644 --- 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; } const CurrentDeviceSection: React.FC = ({ @@ -39,6 +40,7 @@ const CurrentDeviceSection: React.FC = ({ isSigningOut, onVerifyCurrentDevice, onSignOutCurrentDevice, + onSaveDeviceName, }) => { const [isExpanded, setIsExpanded] = useState(false); @@ -46,7 +48,8 @@ const CurrentDeviceSection: React.FC = ({ heading={_t('Current session')} data-testid='current-session-section' > - { isLoading && } + { /* only show big spinner on first load */ } + { isLoading && !device && } { !!device && <> = ({ }
diff --git a/src/components/views/settings/devices/DeviceDetailHeading.tsx b/src/components/views/settings/devices/DeviceDetailHeading.tsx index c1c959ad552..2e6c88bb68e 100644 --- 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; } const DeviceNameEditor: React.FC void }> = ({ @@ -37,19 +37,26 @@ const DeviceNameEditor: React.FC void }> = ({ useEffect(() => { setDeviceName(device.display_name); - }, [device]); + }, [device.display_name]); const onInputChange = (event: React.ChangeEvent): void => setDeviceName(event.target.value); - const onSubmit = () => saveDeviceName(deviceName); + const onSubmit = async (event: FormEvent) => { + event.preventDefault(); + await saveDeviceName(deviceName); + stopEditing(); + }; + const headingId = `device-rename-${device.device_id}`; const descriptionId = `device-rename-description-${device.device_id}`; return
+ onSubmit={onSubmit} + method="post" + >

void }> = ({ aria-labelledby={headingId} aria-describedby={descriptionId} className="mx_DeviceDetailHeading_renameFormInput" + maxLength={100} /> void }> = ({ > { _t('Save') } - { isLoading && } { _t('Cancel') } + > + { _t('Cancel') } + + { isLoading && }

; }; @@ -99,6 +109,7 @@ export const DeviceDetailHeading: React.FC = ({ device, isLoading, saveDeviceName, }) => { const [isEditing, setIsEditing] = useState(false); + return isEditing ? void; onSignOutDevice: () => void; onSetDeviceName: (deviceName: string) => void; @@ -40,6 +41,7 @@ interface MetadataTable { const DeviceDetails: React.FC = ({ device, isSigningOut, + isLoading, onVerifyDevice, onSignOutDevice, onSaveDeviceName, @@ -65,7 +67,7 @@ const DeviceDetails: React.FC = ({
Promise; refreshDevices: () => Promise; + saveDeviceName: (deviceId: DeviceWithVerification['device_id'], deviceName: string) => Promise; error?: OwnDevicesError; }; export const useOwnDevices = (): DevicesState => { @@ -89,11 +93,14 @@ export const useOwnDevices = (): DevicesState => { const userId = matrixClient.getUserId(); const [devices, setDevices] = useState({}); - const [isLoading, setIsLoading] = useState(true); + const [isLoadingDeviceList, setIsLoadingDeviceList] = useState(true); + + // device ids with pending requests + const [pendingDeviceIds, setPendingDeviceIds] = useState([]); const [error, setError] = useState(); 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 => { + 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 index 0b2056b63dc..81684b9da6f 100644 --- 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(); const [expandedDeviceIds, setExpandedDeviceIds] = useState([]); @@ -167,8 +169,9 @@ const SessionManagerTab: React.FC = () => { /> 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 index 9f52f02f4c1..cae06d101a7 100644 --- 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 {

From 83c19517f133ef31e22001db006acaea23248815 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 14 Sep 2022 17:46:24 +0200 Subject: [PATCH 07/12] kind of handle missing current device in drilled props --- src/components/views/settings/tabs/user/SessionManagerTab.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/views/settings/tabs/user/SessionManagerTab.tsx b/src/components/views/settings/tabs/user/SessionManagerTab.tsx index 81684b9da6f..6255042c130 100644 --- a/src/components/views/settings/tabs/user/SessionManagerTab.tsx +++ b/src/components/views/settings/tabs/user/SessionManagerTab.tsx @@ -170,8 +170,8 @@ const SessionManagerTab: React.FC = () => { saveDeviceName(currentDevice.device_id, deviceName)} + isLoading={isLoadingDeviceList || pendingDeviceIds.includes(currentDevice?.device_id)} + onSaveDeviceName={(deviceName) => saveDeviceName(currentDevice?.device_id, deviceName)} onVerifyCurrentDevice={onVerifyCurrentDevice} onSignOutCurrentDevice={onSignOutCurrentDevice} /> From 7933cd670bcc332531cfae2722f2d96b084c18a6 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 15 Sep 2022 13:31:00 +0200 Subject: [PATCH 08/12] use local loading state, add basic error message --- .../devices/_DeviceDetailHeading.pcss | 8 ++- .../settings/devices/CurrentDeviceSection.tsx | 8 +-- .../settings/devices/DeviceDetailHeading.tsx | 26 ++++++-- .../views/settings/devices/DeviceDetails.tsx | 9 +-- .../settings/devices/FilteredDeviceList.tsx | 7 ++ .../views/settings/devices/useOwnDevices.ts | 9 +-- .../settings/tabs/user/SessionManagerTab.tsx | 6 +- .../devices/CurrentDeviceSection-test.tsx | 1 + .../devices/DeviceDetailHeading-test.tsx | 65 ++++++++++++++++--- .../settings/devices/DeviceDetails-test.tsx | 2 + .../devices/FilteredDeviceList-test.tsx | 1 + .../CurrentDeviceSection-test.tsx.snap | 13 ++++ .../DeviceDetailHeading-test.tsx.snap | 2 + .../__snapshots__/DeviceDetails-test.tsx.snap | 3 + 14 files changed, 121 insertions(+), 39 deletions(-) diff --git a/res/css/components/views/settings/devices/_DeviceDetailHeading.pcss b/res/css/components/views/settings/devices/_DeviceDetailHeading.pcss index 107eefdb6e5..b62cc531893 100644 --- a/res/css/components/views/settings/devices/_DeviceDetailHeading.pcss +++ b/res/css/components/views/settings/devices/_DeviceDetailHeading.pcss @@ -50,4 +50,10 @@ limitations under the License. .mx_DeviceDetailHeading_renameFormHeading { margin: 0; -} \ No newline at end of file +} + +.mx_DeviceDetailHeading_renameFormError { + color: $alert; + padding-right: $spacing-4; + display: block; +} diff --git a/src/components/views/settings/devices/CurrentDeviceSection.tsx b/src/components/views/settings/devices/CurrentDeviceSection.tsx index b4cc39a8e3c..023d33b083c 100644 --- a/src/components/views/settings/devices/CurrentDeviceSection.tsx +++ b/src/components/views/settings/devices/CurrentDeviceSection.tsx @@ -31,7 +31,7 @@ interface Props { isSigningOut: boolean; onVerifyCurrentDevice: () => void; onSignOutCurrentDevice: () => void; - onSaveDeviceName: (deviceName: string) => Promise; + saveDeviceName: (deviceName: string) => Promise; } const CurrentDeviceSection: React.FC = ({ @@ -40,7 +40,7 @@ const CurrentDeviceSection: React.FC = ({ isSigningOut, onVerifyCurrentDevice, onSignOutCurrentDevice, - onSaveDeviceName, + saveDeviceName, }) => { const [isExpanded, setIsExpanded] = useState(false); @@ -64,9 +64,9 @@ const CurrentDeviceSection: React.FC = ({ }
diff --git a/src/components/views/settings/devices/DeviceDetailHeading.tsx b/src/components/views/settings/devices/DeviceDetailHeading.tsx index 2e6c88bb68e..d294e3b2a10 100644 --- a/src/components/views/settings/devices/DeviceDetailHeading.tsx +++ b/src/components/views/settings/devices/DeviceDetailHeading.tsx @@ -26,14 +26,15 @@ import { DeviceWithVerification } from './types'; interface Props { device: DeviceWithVerification; - isLoading: boolean; saveDeviceName: (deviceName: string) => Promise; } const DeviceNameEditor: React.FC void }> = ({ - device, isLoading, saveDeviceName, stopEditing, + device, saveDeviceName, stopEditing, }) => { const [deviceName, setDeviceName] = useState(device.display_name || ''); + const [isLoading, setIsLoading] = useState(false); + const [error, setError] = useState(null); useEffect(() => { setDeviceName(device.display_name); @@ -43,9 +44,16 @@ const DeviceNameEditor: React.FC void }> = ({ setDeviceName(event.target.value); const onSubmit = async (event: FormEvent) => { + setIsLoading(true); + setError(null); event.preventDefault(); - await saveDeviceName(deviceName); - stopEditing(); + try { + await saveDeviceName(deviceName); + stopEditing(); + } catch (error) { + setError(_t('Failed to set display name')); + setIsLoading(false); + } }; const headingId = `device-rename-${device.device_id}`; @@ -81,6 +89,11 @@ const DeviceNameEditor: React.FC void }> = ({ id={descriptionId} > { _t('Please be aware that session names are also visible to people you communicate with') } + { !!error && + + { error } + + }

@@ -106,18 +119,17 @@ const DeviceNameEditor: React.FC void }> = ({ }; export const DeviceDetailHeading: React.FC = ({ - device, isLoading, saveDeviceName, + device, saveDeviceName, }) => { const [isEditing, setIsEditing] = useState(false); return isEditing ? setIsEditing(false)} /> - :
+ :
{ device.display_name ?? device.device_id } void; onSignOutDevice: () => void; - onSetDeviceName: (deviceName: string) => void; + saveDeviceName: (deviceName: string) => Promise; } interface MetadataTable { @@ -41,10 +40,9 @@ interface MetadataTable { const DeviceDetails: React.FC = ({ device, isSigningOut, - isLoading, onVerifyDevice, onSignOutDevice, - onSaveDeviceName, + saveDeviceName, }) => { const metadata: MetadataTable[] = [ { @@ -67,8 +65,7 @@ const DeviceDetails: React.FC = ({
void; onDeviceExpandToggle: (deviceId: DeviceWithVerification['device_id']) => void; onSignOutDevices: (deviceIds: DeviceWithVerification['device_id'][]) => void; + saveDeviceName: DevicesState['saveDeviceName']; onRequestDeviceVerification?: (deviceId: DeviceWithVerification['device_id']) => void; } @@ -137,6 +139,7 @@ const DeviceListItem: React.FC<{ isSigningOut: boolean; onDeviceExpandToggle: () => void; onSignOutDevice: () => void; + saveDeviceName: (deviceName: string) => Promise; onRequestDeviceVerification?: () => void; }> = ({ device, @@ -144,6 +147,7 @@ const DeviceListItem: React.FC<{ isSigningOut, onDeviceExpandToggle, onSignOutDevice, + saveDeviceName, onRequestDeviceVerification, }) =>
  • }
    • ; @@ -177,6 +182,7 @@ export const FilteredDeviceList = signingOutDeviceIds, onFilterChange, onDeviceExpandToggle, + saveDeviceName, onSignOutDevices, onRequestDeviceVerification, }: Props, ref: ForwardedRef) => { @@ -234,6 +240,7 @@ export const FilteredDeviceList = isSigningOut={signingOutDeviceIds.includes(device.device_id)} onDeviceExpandToggle={() => onDeviceExpandToggle(device.device_id)} onSignOutDevice={() => onSignOutDevices([device.device_id])} + saveDeviceName={(deviceName: string) => saveDeviceName(device.device_id, deviceName)} onRequestDeviceVerification={ onRequestDeviceVerification ? () => onRequestDeviceVerification(device.device_id) diff --git a/src/components/views/settings/devices/useOwnDevices.ts b/src/components/views/settings/devices/useOwnDevices.ts index 2b0762b1ba0..6dcf6fc16e7 100644 --- a/src/components/views/settings/devices/useOwnDevices.ts +++ b/src/components/views/settings/devices/useOwnDevices.ts @@ -78,8 +78,6 @@ export type DevicesState = { devices: DevicesDictionary; currentDeviceId: string; 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; refreshDevices: () => Promise; @@ -95,8 +93,6 @@ export const useOwnDevices = (): DevicesState => { const [devices, setDevices] = useState({}); const [isLoadingDeviceList, setIsLoadingDeviceList] = useState(true); - // device ids with pending requests - const [pendingDeviceIds, setPendingDeviceIds] = useState([]); const [error, setError] = useState(); const refreshDevices = useCallback(async () => { @@ -147,7 +143,6 @@ export const useOwnDevices = (): DevicesState => { } try { - setPendingDeviceIds(p => ([...p, deviceId])); await matrixClient.setDeviceDetails( deviceId, { display_name: deviceName }, @@ -157,15 +152,13 @@ export const useOwnDevices = (): DevicesState => { 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]); + }, [matrixClient, devices, refreshDevices]); return { devices, currentDeviceId, isLoadingDeviceList, error, - pendingDeviceIds, requestDeviceVerification, refreshDevices, saveDeviceName, diff --git a/src/components/views/settings/tabs/user/SessionManagerTab.tsx b/src/components/views/settings/tabs/user/SessionManagerTab.tsx index 6255042c130..bd26965451b 100644 --- a/src/components/views/settings/tabs/user/SessionManagerTab.tsx +++ b/src/components/views/settings/tabs/user/SessionManagerTab.tsx @@ -89,7 +89,6 @@ const SessionManagerTab: React.FC = () => { devices, currentDeviceId, isLoadingDeviceList, - pendingDeviceIds, requestDeviceVerification, refreshDevices, saveDeviceName, @@ -170,8 +169,8 @@ const SessionManagerTab: React.FC = () => { saveDeviceName(currentDevice?.device_id, deviceName)} + isLoading={isLoadingDeviceList} + saveDeviceName={(deviceName) => saveDeviceName(currentDevice?.device_id, deviceName)} onVerifyCurrentDevice={onVerifyCurrentDevice} onSignOutCurrentDevice={onSignOutCurrentDevice} /> @@ -194,6 +193,7 @@ const SessionManagerTab: React.FC = () => { onDeviceExpandToggle={onDeviceExpandToggle} onRequestDeviceVerification={requestDeviceVerification ? onTriggerDeviceVerification : undefined} onSignOutDevices={onSignOutOtherDevices} + saveDeviceName={saveDeviceName} ref={filteredDeviceListRef} /> diff --git a/test/components/views/settings/devices/CurrentDeviceSection-test.tsx b/test/components/views/settings/devices/CurrentDeviceSection-test.tsx index a63d96fa07c..22824fb1e71 100644 --- a/test/components/views/settings/devices/CurrentDeviceSection-test.tsx +++ b/test/components/views/settings/devices/CurrentDeviceSection-test.tsx @@ -36,6 +36,7 @@ describe('', () => { device: alicesVerifiedDevice, onVerifyCurrentDevice: jest.fn(), onSignOutCurrentDevice: jest.fn(), + saveDeviceName: jest.fn(), isLoading: false, isSigningOut: false, }; diff --git a/test/components/views/settings/devices/DeviceDetailHeading-test.tsx b/test/components/views/settings/devices/DeviceDetailHeading-test.tsx index 64b00203cbd..655fa359c9b 100644 --- a/test/components/views/settings/devices/DeviceDetailHeading-test.tsx +++ b/test/components/views/settings/devices/DeviceDetailHeading-test.tsx @@ -14,10 +14,13 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { fireEvent, render, RenderResult } from '@testing-library/react'; import React from 'react'; +import { fireEvent, render, RenderResult } from '@testing-library/react'; import { DeviceDetailHeading } from '../../../../../src/components/views/settings/devices/DeviceDetailHeading'; +import { flushPromisesWithFakeTimers } from '../../../../test-utils'; + +jest.useFakeTimers(); describe('', () => { const device = { @@ -27,7 +30,6 @@ describe('', () => { }; const defaultProps = { device, - isLoading: false, saveDeviceName: jest.fn(), }; const getComponent = (props = {}) => @@ -71,14 +73,29 @@ describe('', () => { expect(container.getElementsByClassName('mx_DeviceDetailHeading').length).toBe(1); }); - it('disables form while device isLoading', () => { - const { getByTestId, rerender } = render(getComponent()); + it('clicking submit updates device name with edited value', () => { + const saveDeviceName = jest.fn(); + const { getByTestId } = render(getComponent({ saveDeviceName })); // start editing fireEvent.click(getByTestId('device-heading-rename-cta')); - // enter loading state - rerender(getComponent({ isLoading: true })); + setInputValue(getByTestId, 'new device name'); + + fireEvent.click(getByTestId('device-rename-submit-cta')); + + expect(saveDeviceName).toHaveBeenCalledWith('new device name'); + }); + + it('disables form while device name is saving', () => { + const { getByTestId, container } = render(getComponent()); + + // start editing + fireEvent.click(getByTestId('device-heading-rename-cta')); + + setInputValue(getByTestId, 'new device name'); + + fireEvent.click(getByTestId('device-rename-submit-cta')); // buttons disabled expect( @@ -87,19 +104,47 @@ describe('', () => { expect( getByTestId('device-rename-submit-cta').getAttribute('aria-disabled'), ).toEqual("true"); + + expect(container.getElementsByClassName('mx_Spinner').length).toBeTruthy(); }); - it('clicking submit updates device name with edited value', () => { - const saveDeviceName = jest.fn(); - const { getByTestId } = render(getComponent({ saveDeviceName })); + it('toggles out of editing mode when device name is saved successfully', async () => { + const { getByTestId } = render(getComponent()); // start editing fireEvent.click(getByTestId('device-heading-rename-cta')); + setInputValue(getByTestId, 'new device name'); + fireEvent.click(getByTestId('device-rename-submit-cta')); + + await flushPromisesWithFakeTimers(); + // read mode displayed + expect(getByTestId('device-detail-heading')).toBeTruthy(); + }); + + it('displays error when device name fails to save', async () => { + const saveDeviceName = jest.fn().mockRejectedValueOnce('oups').mockResolvedValue({}); + const { getByTestId, queryByText, container } = render(getComponent({ saveDeviceName })); + + // start editing + fireEvent.click(getByTestId('device-heading-rename-cta')); setInputValue(getByTestId, 'new device name'); + fireEvent.click(getByTestId('device-rename-submit-cta')); + + // flush promise + await flushPromisesWithFakeTimers(); + // then tick for render + await flushPromisesWithFakeTimers(); + // error message displayed + expect(queryByText('Failed to set display name')).toBeTruthy(); + // spinner removed + expect(container.getElementsByClassName('mx_Spinner').length).toBeFalsy(); + + // try again fireEvent.click(getByTestId('device-rename-submit-cta')); - expect(saveDeviceName).toHaveBeenCalledWith('new device name'); + // error message cleared + expect(queryByText('Failed to set display name')).toBeFalsy(); }); }); diff --git a/test/components/views/settings/devices/DeviceDetails-test.tsx b/test/components/views/settings/devices/DeviceDetails-test.tsx index 272f781758c..dad0ce625be 100644 --- a/test/components/views/settings/devices/DeviceDetails-test.tsx +++ b/test/components/views/settings/devices/DeviceDetails-test.tsx @@ -27,7 +27,9 @@ describe('', () => { const defaultProps = { device: baseDevice, isSigningOut: false, + isLoading: false, onSignOutDevice: jest.fn(), + saveDeviceName: jest.fn(), }; const getComponent = (props = {}) => ; // 14.03.2022 16:15 diff --git a/test/components/views/settings/devices/FilteredDeviceList-test.tsx b/test/components/views/settings/devices/FilteredDeviceList-test.tsx index 02cff732223..64869d31b6a 100644 --- a/test/components/views/settings/devices/FilteredDeviceList-test.tsx +++ b/test/components/views/settings/devices/FilteredDeviceList-test.tsx @@ -44,6 +44,7 @@ describe('', () => { onFilterChange: jest.fn(), onDeviceExpandToggle: jest.fn(), onSignOutDevices: jest.fn(), + saveDeviceName: jest.fn(), expandedDeviceIds: [], signingOutDeviceIds: [], devices: { diff --git a/test/components/views/settings/devices/__snapshots__/CurrentDeviceSection-test.tsx.snap b/test/components/views/settings/devices/__snapshots__/CurrentDeviceSection-test.tsx.snap index f0aef065e2e..0f029423ab2 100644 --- a/test/components/views/settings/devices/__snapshots__/CurrentDeviceSection-test.tsx.snap +++ b/test/components/views/settings/devices/__snapshots__/CurrentDeviceSection-test.tsx.snap @@ -11,6 +11,7 @@ HTMLCollection [ >

    Verify or sign out from this session for best security and reliability.

    +
    +
    + Verify session +
    +

    diff --git a/test/components/views/settings/devices/__snapshots__/DeviceDetailHeading-test.tsx.snap b/test/components/views/settings/devices/__snapshots__/DeviceDetailHeading-test.tsx.snap index cae06d101a7..0842ffb43d5 100644 --- a/test/components/views/settings/devices/__snapshots__/DeviceDetailHeading-test.tsx.snap +++ b/test/components/views/settings/devices/__snapshots__/DeviceDetailHeading-test.tsx.snap @@ -69,6 +69,7 @@ Object { "container":

    renders a verified device 1`] = ` >

    renders device with metadata 1`] = ` >

    renders device without metadata 1`] = ` >

    Date: Thu, 15 Sep 2022 13:50:48 +0200 Subject: [PATCH 09/12] integration-ish test rename --- .../settings/devices/DeviceDetailHeading.tsx | 4 +- .../devices/DeviceDetailHeading-test.tsx | 4 +- .../DeviceDetailHeading-test.tsx.snap | 25 ---- .../tabs/user/SessionManagerTab-test.tsx | 110 +++++++++++++++++- .../SessionManagerTab-test.tsx.snap | 36 ++++-- 5 files changed, 136 insertions(+), 43 deletions(-) diff --git a/src/components/views/settings/devices/DeviceDetailHeading.tsx b/src/components/views/settings/devices/DeviceDetailHeading.tsx index d294e3b2a10..9453b764088 100644 --- a/src/components/views/settings/devices/DeviceDetailHeading.tsx +++ b/src/components/views/settings/devices/DeviceDetailHeading.tsx @@ -90,7 +90,9 @@ const DeviceNameEditor: React.FC void }> = ({ > { _t('Please be aware that session names are also visible to people you communicate with') } { !!error && - + { error } } diff --git a/test/components/views/settings/devices/DeviceDetailHeading-test.tsx b/test/components/views/settings/devices/DeviceDetailHeading-test.tsx index 655fa359c9b..1bda0a04f54 100644 --- a/test/components/views/settings/devices/DeviceDetailHeading-test.tsx +++ b/test/components/views/settings/devices/DeviceDetailHeading-test.tsx @@ -47,10 +47,10 @@ describe('', () => { }); it('renders device id as fallback when device has no display name ', () => { - const { container } = render(getComponent({ + const { getByText } = render(getComponent({ device: { ...device, display_name: undefined }, })); - expect({ container }).toMatchSnapshot(); + expect(getByText(device.device_id)).toBeTruthy(); }); it('displays name edit form on rename button click', () => { diff --git a/test/components/views/settings/devices/__snapshots__/DeviceDetailHeading-test.tsx.snap b/test/components/views/settings/devices/__snapshots__/DeviceDetailHeading-test.tsx.snap index 0842ffb43d5..c80da7be4c7 100644 --- a/test/components/views/settings/devices/__snapshots__/DeviceDetailHeading-test.tsx.snap +++ b/test/components/views/settings/devices/__snapshots__/DeviceDetailHeading-test.tsx.snap @@ -64,31 +64,6 @@ Object { } `; -exports[` renders device id as fallback when device has no display name 1`] = ` -Object { - "container":
    -
    -

    - 123 -

    -
    - Rename -
    -
    -
    , -} -`; - exports[` renders device name 1`] = ` Object { "container":
    diff --git a/test/components/views/settings/tabs/user/SessionManagerTab-test.tsx b/test/components/views/settings/tabs/user/SessionManagerTab-test.tsx index 4e0a556818f..a281708a409 100644 --- a/test/components/views/settings/tabs/user/SessionManagerTab-test.tsx +++ b/test/components/views/settings/tabs/user/SessionManagerTab-test.tsx @@ -15,13 +15,14 @@ limitations under the License. */ import React from 'react'; -import { fireEvent, render } from '@testing-library/react'; +import { fireEvent, render, RenderResult } from '@testing-library/react'; import { act } from 'react-dom/test-utils'; import { DeviceInfo } from 'matrix-js-sdk/src/crypto/deviceinfo'; import { logger } from 'matrix-js-sdk/src/logger'; import { DeviceTrustLevel } from 'matrix-js-sdk/src/crypto/CrossSigning'; import { VerificationRequest } from 'matrix-js-sdk/src/crypto/verification/request/VerificationRequest'; import { sleep } from 'matrix-js-sdk/src/utils'; +import { IMyDevice } from 'matrix-js-sdk/src/matrix'; import SessionManagerTab from '../../../../../../src/components/views/settings/tabs/user/SessionManagerTab'; import MatrixClientContext from '../../../../../../src/contexts/MatrixClientContext'; @@ -40,6 +41,7 @@ describe('', () => { const alicesDevice = { device_id: deviceId, + display_name: 'Alices device', }; const alicesMobileDevice = { device_id: 'alices_mobile_device', @@ -64,6 +66,7 @@ describe('', () => { requestVerification: jest.fn().mockResolvedValue(mockVerificationRequest), deleteMultipleDevices: jest.fn(), generateClientSecret: jest.fn(), + setDeviceDetails: jest.fn(), }); const defaultProps = {}; @@ -87,7 +90,6 @@ describe('', () => { beforeEach(() => { jest.clearAllMocks(); jest.spyOn(logger, 'error').mockRestore(); - mockClient.getDevices.mockResolvedValue({ devices: [] }); mockClient.getStoredDevice.mockImplementation((_userId, id) => { const device = [alicesDevice, alicesMobileDevice].find(device => device.device_id === id); return device ? new DeviceInfo(device.device_id) : null; @@ -98,7 +100,7 @@ describe('', () => { mockClient.getDevices .mockReset() - .mockResolvedValue({ devices: [alicesMobileDevice] }); + .mockResolvedValue({ devices: [alicesDevice, alicesMobileDevice] }); }); it('renders spinner while devices load', () => { @@ -561,4 +563,106 @@ describe('', () => { }); }); }); + + describe('Rename sessions', () => { + const updateDeviceName = async ( + getByTestId: RenderResult['getByTestId'], + device: IMyDevice, + newDeviceName: string, + ) => { + toggleDeviceDetails(getByTestId, device.device_id); + + // start editing + fireEvent.click(getByTestId('device-heading-rename-cta')); + + const input = getByTestId('device-rename-input'); + fireEvent.change(input, { target: { value: newDeviceName } }); + fireEvent.click(getByTestId('device-rename-submit-cta')); + + await flushPromisesWithFakeTimers(); + await flushPromisesWithFakeTimers(); + }; + + it('renames current session', async () => { + const { getByTestId } = render(getComponent()); + + await act(async () => { + await flushPromisesWithFakeTimers(); + }); + + const newDeviceName = 'new device name'; + await updateDeviceName(getByTestId, alicesDevice, newDeviceName); + + expect(mockClient.setDeviceDetails).toHaveBeenCalledWith( + alicesDevice.device_id, { display_name: newDeviceName }); + + // devices refreshed + expect(mockClient.getDevices).toHaveBeenCalledTimes(2); + }); + + it('renames other session', async () => { + const { getByTestId } = render(getComponent()); + + await act(async () => { + await flushPromisesWithFakeTimers(); + }); + + const newDeviceName = 'new device name'; + await updateDeviceName(getByTestId, alicesMobileDevice, newDeviceName); + + expect(mockClient.setDeviceDetails).toHaveBeenCalledWith( + alicesMobileDevice.device_id, { display_name: newDeviceName }); + + // devices refreshed + expect(mockClient.getDevices).toHaveBeenCalledTimes(2); + }); + + it('does not rename session or refresh devices is session name is unchanged', async () => { + const { getByTestId } = render(getComponent()); + + await act(async () => { + await flushPromisesWithFakeTimers(); + }); + + await updateDeviceName(getByTestId, alicesDevice, alicesDevice.display_name); + + expect(mockClient.setDeviceDetails).not.toHaveBeenCalled(); + // only called once on initial load + expect(mockClient.getDevices).toHaveBeenCalledTimes(1); + }); + + it('saves an empty session display name successfully', async () => { + const { getByTestId } = render(getComponent()); + + await act(async () => { + await flushPromisesWithFakeTimers(); + }); + + await updateDeviceName(getByTestId, alicesDevice, ''); + + expect(mockClient.setDeviceDetails).toHaveBeenCalledWith( + alicesDevice.device_id, { display_name: '' }); + }); + + it('displays an error when session display name fails to save', async () => { + const logSpy = jest.spyOn(logger, 'error'); + const error = new Error('oups'); + mockClient.setDeviceDetails.mockRejectedValue(error); + const { getByTestId } = render(getComponent()); + + await act(async () => { + await flushPromisesWithFakeTimers(); + }); + + const newDeviceName = 'new device name'; + await updateDeviceName(getByTestId, alicesDevice, newDeviceName); + + await flushPromisesWithFakeTimers(); + + expect(logSpy).toHaveBeenCalledWith("Error setting session display name", error); + + // error displayed + expect(getByTestId('device-rename-error')).toBeTruthy(); + }); + }); }); diff --git a/test/components/views/settings/tabs/user/__snapshots__/SessionManagerTab-test.tsx.snap b/test/components/views/settings/tabs/user/__snapshots__/SessionManagerTab-test.tsx.snap index a0b087ce4f4..2d290b20b5f 100644 --- a/test/components/views/settings/tabs/user/__snapshots__/SessionManagerTab-test.tsx.snap +++ b/test/components/views/settings/tabs/user/__snapshots__/SessionManagerTab-test.tsx.snap @@ -85,11 +85,15 @@ exports[` renders current session section with a verified s
    -

    - alices_device -

    +

    + Alices device +

    +
    @@ -181,11 +185,15 @@ exports[` renders current session section with an unverifie
    -

    - alices_device -

    +

    + Alices device +

    +
    @@ -277,11 +285,15 @@ exports[` sets device verification status correctly 1`] = `
    -

    - alices_device -

    +

    + Alices device +

    +
    From 5953e44522e10aa3f9eb17fed59d33667e868f58 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 15 Sep 2022 13:53:32 +0200 Subject: [PATCH 10/12] tidy --- src/components/views/settings/devices/DeviceDetailHeading.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/settings/devices/DeviceDetailHeading.tsx b/src/components/views/settings/devices/DeviceDetailHeading.tsx index 9453b764088..c65868f7d0e 100644 --- a/src/components/views/settings/devices/DeviceDetailHeading.tsx +++ b/src/components/views/settings/devices/DeviceDetailHeading.tsx @@ -132,7 +132,7 @@ export const DeviceDetailHeading: React.FC = ({ stopEditing={() => setIsEditing(false)} /> :
    - { device.display_name ?? device.device_id } + { device.display_name || device.device_id } setIsEditing(true)} From 1f6de25e71286422e175fd7d0dd48aac9d8903fa Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 15 Sep 2022 13:56:19 +0200 Subject: [PATCH 11/12] fussy import ordering --- src/components/views/settings/devices/useOwnDevices.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/settings/devices/useOwnDevices.ts b/src/components/views/settings/devices/useOwnDevices.ts index 6dcf6fc16e7..0f7d1044da6 100644 --- a/src/components/views/settings/devices/useOwnDevices.ts +++ b/src/components/views/settings/devices/useOwnDevices.ts @@ -22,8 +22,8 @@ import { MatrixError } from "matrix-js-sdk/src/http-api"; import { logger } from "matrix-js-sdk/src/logger"; import MatrixClientContext from "../../../../contexts/MatrixClientContext"; -import { DevicesDictionary, DeviceWithVerification } from "./types"; import { _t } from "../../../../languageHandler"; +import { DevicesDictionary, DeviceWithVerification } from "./types"; const isDeviceVerified = ( matrixClient: MatrixClient, From f491afcd128bebe34dba383ceb147dd9c9097e09 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 15 Sep 2022 15:09:50 +0200 Subject: [PATCH 12/12] strict errors --- src/components/views/settings/devices/DeviceDetailHeading.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/views/settings/devices/DeviceDetailHeading.tsx b/src/components/views/settings/devices/DeviceDetailHeading.tsx index c65868f7d0e..dea79d3b23f 100644 --- a/src/components/views/settings/devices/DeviceDetailHeading.tsx +++ b/src/components/views/settings/devices/DeviceDetailHeading.tsx @@ -34,10 +34,10 @@ const DeviceNameEditor: React.FC void }> = ({ }) => { const [deviceName, setDeviceName] = useState(device.display_name || ''); const [isLoading, setIsLoading] = useState(false); - const [error, setError] = useState(null); + const [error, setError] = useState(null); useEffect(() => { - setDeviceName(device.display_name); + setDeviceName(device.display_name || ''); }, [device.display_name]); const onInputChange = (event: React.ChangeEvent): void =>