Device manager - expandable session details in device list (PSG-644) (#…

…9188) * add expandable device details to session list * test device expansion in filtered list * test expanded device id management from sessionmanager tab * i18n * update snapshot * update snapshots * use css instead of br
matrix-org · Aug 17, 2022 · e5fedfc · e5fedfc
1 parent fecc032
commit e5fedfc
Show file tree

Hide file tree

Showing 13 changed files with 155 additions and 12 deletions.
diff --git a/res/css/components/views/settings/devices/_DeviceDetails.pcss b/res/css/components/views/settings/devices/_DeviceDetails.pcss
@@ -32,6 +32,9 @@ limitations under the License.
     margin-bottom: $spacing-16;
     border-bottom: 1px solid $quinary-content;
 
+    display: grid;
+    grid-gap: $spacing-16;
+
     &:last-child {
         padding-bottom: 0;
         border-bottom: 0;
@@ -48,7 +51,6 @@ limitations under the License.
     color: $secondary-content;
 
     width: 100%;
-    margin-top: $spacing-20;
 
     border-spacing: 0;
 

diff --git a/res/css/components/views/settings/devices/_FilteredDeviceList.pcss b/res/css/components/views/settings/devices/_FilteredDeviceList.pcss
@@ -48,6 +48,11 @@ limitations under the License.
     padding: 0 $spacing-8;
 }
 
+.mx_FilteredDeviceList_listItem {
+    display: flex;
+    flex-direction: column;
+}
+
 .mx_FilteredDeviceList_securityCard {
     margin-bottom: $spacing-32;
 }

diff --git a/src/components/views/settings/devices/DeviceDetails.tsx b/src/components/views/settings/devices/DeviceDetails.tsx
@@ -49,7 +49,7 @@ const DeviceDetails: React.FC<Props> = ({ device }) => {
             ],
         },
     ];
-    return <div className='mx_DeviceDetails'>
+    return <div className='mx_DeviceDetails' data-testid={`device-detail-${device.device_id}`}>
         <section className='mx_DeviceDetails_section'>
             <Heading size='h3'>{ device.display_name ?? device.device_id }</Heading>
             <DeviceVerificationStatusCard device={device} />

diff --git a/src/components/views/settings/devices/DeviceExpandDetailsButton.tsx b/src/components/views/settings/devices/DeviceExpandDetailsButton.tsx
@@ -18,6 +18,7 @@ import classNames from 'classnames';
 import React from 'react';
 
 import { Icon as CaretIcon } from '../../../../../res/img/feather-customised/dropdown-arrow.svg';
+import { _t } from '../../../../languageHandler';
 import AccessibleButton from '../../elements/AccessibleButton';
 
 interface Props {
@@ -28,6 +29,7 @@ interface Props {
 const DeviceExpandDetailsButton: React.FC<Props> = ({ isExpanded, onClick, ...rest }) => {
     return <AccessibleButton
         {...rest}
+        aria-label={_t('Toggle device details')}
         kind='icon'
         className={classNames('mx_DeviceExpandDetailsButton', {
             mx_DeviceExpandDetailsButton_expanded: isExpanded,

diff --git a/src/components/views/settings/devices/FilteredDeviceList.tsx b/src/components/views/settings/devices/FilteredDeviceList.tsx
@@ -19,6 +19,8 @@ import React from 'react';
 import { _t } from '../../../../languageHandler';
 import AccessibleButton from '../../elements/AccessibleButton';
 import Dropdown from '../../elements/Dropdown';
+import DeviceDetails from './DeviceDetails';
+import DeviceExpandDetailsButton from './DeviceExpandDetailsButton';
 import DeviceSecurityCard from './DeviceSecurityCard';
 import DeviceTile from './DeviceTile';
 import {
@@ -33,8 +35,10 @@ import {
 
 interface Props {
     devices: DevicesDictionary;
+    expandedDeviceIds: DeviceWithVerification['device_id'][];
     filter?: DeviceSecurityVariation;
     onFilterChange: (filter: DeviceSecurityVariation | undefined) => void;
+    onDeviceExpandToggle: (deviceId: DeviceWithVerification['device_id']) => void;
 }
 
 // devices without timestamp metadata should be sorted last
@@ -123,11 +127,35 @@ const NoResults: React.FC<NoResultsProps> = ({ filter, clearFilter }) =>
         }
     </div>;
 
+const DeviceListItem: React.FC<{
+    device: DeviceWithVerification;
+    isExpanded: boolean;
+    onDeviceExpandToggle: () => void;
+}> = ({
+    device, isExpanded, onDeviceExpandToggle,
+}) => <li className='mx_FilteredDeviceList_listItem'>
+    <DeviceTile
+        device={device}
+    >
+        <DeviceExpandDetailsButton
+            isExpanded={isExpanded}
+            onClick={onDeviceExpandToggle}
+        />
+    </DeviceTile>
+    { isExpanded && <DeviceDetails device={device} /> }
+</li>;
+
 /**
  * Filtered list of devices
  * Sorted by latest activity descending
  */
-const FilteredDeviceList: React.FC<Props> = ({ devices, filter, onFilterChange }) => {
+const FilteredDeviceList: React.FC<Props> = ({
+    devices,
+    filter,
+    expandedDeviceIds,
+    onFilterChange,
+    onDeviceExpandToggle,
+}) => {
     const sortedDevices = getFilteredSortedDevices(devices, filter);
 
     const options = [
@@ -177,13 +205,12 @@ const FilteredDeviceList: React.FC<Props> = ({ devices, filter, onFilterChange }
             : <NoResults filter={filter} clearFilter={() => onFilterChange(undefined)} />
         }
         <ol className='mx_FilteredDeviceList_list'>
-            { sortedDevices.map((device) =>
-                <li key={device.device_id}>
-                    <DeviceTile
-                        device={device}
-                    />
-                </li>,
-
+            { sortedDevices.map((device) => <DeviceListItem
+                key={device.device_id}
+                device={device}
+                isExpanded={expandedDeviceIds.includes(device.device_id)}
+                onDeviceExpandToggle={() => onDeviceExpandToggle(device.device_id)}
+            />,
             ) }
         </ol>
     </div>

diff --git a/src/components/views/settings/tabs/user/SessionManagerTab.tsx b/src/components/views/settings/tabs/user/SessionManagerTab.tsx
@@ -22,12 +22,21 @@ import SettingsSubsection from '../../shared/SettingsSubsection';
 import FilteredDeviceList from '../../devices/FilteredDeviceList';
 import CurrentDeviceSection from '../../devices/CurrentDeviceSection';
 import SecurityRecommendations from '../../devices/SecurityRecommendations';
-import { DeviceSecurityVariation } from '../../devices/types';
+import { DeviceSecurityVariation, DeviceWithVerification } from '../../devices/types';
 import SettingsTab from '../SettingsTab';
 
 const SessionManagerTab: React.FC = () => {
     const { devices, currentDeviceId, isLoading } = useOwnDevices();
     const [filter, setFilter] = useState<DeviceSecurityVariation>();
+    const [expandedDeviceIds, setExpandedDeviceIds] = useState([]);
+
+    const onDeviceExpandToggle = (deviceId: DeviceWithVerification['device_id']): void => {
+        if (expandedDeviceIds.includes(deviceId)) {
+            setExpandedDeviceIds(expandedDeviceIds.filter(id => id !== deviceId));
+        } else {
+            setExpandedDeviceIds([...expandedDeviceIds, deviceId]);
+        }
+    };
 
     const { [currentDeviceId]: currentDevice, ...otherDevices } = devices;
     const shouldShowOtherSessions = Object.keys(otherDevices).length > 0;
@@ -51,7 +60,9 @@ const SessionManagerTab: React.FC = () => {
                 <FilteredDeviceList
                     devices={otherDevices}
                     filter={filter}
+                    expandedDeviceIds={expandedDeviceIds}
                     onFilterChange={setFilter}
+                    onDeviceExpandToggle={onDeviceExpandToggle}
                 />
             </SettingsSubsection>
         }

diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json
@@ -1701,6 +1701,7 @@
     "Device": "Device",
     "IP address": "IP address",
     "Session details": "Session details",
+    "Toggle device details": "Toggle device details",
     "Inactive for %(inactiveAgeDays)s+ days": "Inactive for %(inactiveAgeDays)s+ days",
     "Verified": "Verified",
     "Unverified": "Unverified",

diff --git a/test/components/views/settings/devices/FilteredDeviceList-test.tsx b/test/components/views/settings/devices/FilteredDeviceList-test.tsx
@@ -42,6 +42,8 @@ describe('<FilteredDeviceList />', () => {
     };
     const defaultProps = {
         onFilterChange: jest.fn(),
+        onDeviceExpandToggle: jest.fn(),
+        expandedDeviceIds: [],
         devices: {
             [unverifiedNoMetadata.device_id]: unverifiedNoMetadata,
             [verifiedNoMetadata.device_id]: verifiedNoMetadata,
@@ -179,4 +181,27 @@ describe('<FilteredDeviceList />', () => {
             expect(onFilterChange).toHaveBeenCalledWith(undefined);
         });
     });
+
+    describe('device details', () => {
+        it('renders expanded devices with device details', () => {
+            const expandedDeviceIds = [newDevice.device_id, hundredDaysOld.device_id];
+            const { container, getByTestId } = render(getComponent({ expandedDeviceIds }));
+            expect(container.getElementsByClassName('mx_DeviceDetails').length).toBeTruthy();
+            expect(getByTestId(`device-detail-${newDevice.device_id}`)).toBeTruthy();
+            expect(getByTestId(`device-detail-${hundredDaysOld.device_id}`)).toBeTruthy();
+        });
+
+        it('clicking toggle calls onDeviceExpandToggle', () => {
+            const onDeviceExpandToggle = jest.fn();
+            const { getByTestId } = render(getComponent({ onDeviceExpandToggle }));
+
+            act(() => {
+                const tile = getByTestId(`device-tile-${hundredDaysOld.device_id}`);
+                const toggle = tile.querySelector('[aria-label="Toggle device details"]');
+                fireEvent.click(toggle);
+            });
+
+            expect(onDeviceExpandToggle).toHaveBeenCalledWith(hundredDaysOld.device_id);
+        });
+    });
 });
diff --git a/test/components/views/settings/devices/__snapshots__/CurrentDeviceSection-test.tsx.snap b/test/components/views/settings/devices/__snapshots__/CurrentDeviceSection-test.tsx.snap
@@ -4,6 +4,7 @@ exports[`<CurrentDeviceSection /> displays device details on toggle click 1`] =
 HTMLCollection [
   <div
     class="mx_DeviceDetails"
+    data-testid="device-detail-alices_device"
   >
     <section
       class="mx_DeviceDetails_section"
@@ -164,6 +165,7 @@ exports[`<CurrentDeviceSection /> renders device and correct security card when
           class="mx_DeviceTile_actions"
         >
           <div
+            aria-label="Toggle device details"
             class="mx_AccessibleButton mx_DeviceExpandDetailsButton mx_AccessibleButton_hasKind mx_AccessibleButton_kind_icon"
             data-testid="current-session-toggle-details"
             role="button"
@@ -249,6 +251,7 @@ exports[`<CurrentDeviceSection /> renders device and correct security card when
           class="mx_DeviceTile_actions"
         >
           <div
+            aria-label="Toggle device details"
             class="mx_AccessibleButton mx_DeviceExpandDetailsButton mx_AccessibleButton_hasKind mx_AccessibleButton_kind_icon"
             data-testid="current-session-toggle-details"
             role="button"

diff --git a/test/components/views/settings/devices/__snapshots__/DeviceDetails-test.tsx.snap b/test/components/views/settings/devices/__snapshots__/DeviceDetails-test.tsx.snap
@@ -4,6 +4,7 @@ exports[`<DeviceDetails /> renders a verified device 1`] = `
 <div>
   <div
     class="mx_DeviceDetails"
+    data-testid="device-detail-my-device"
   >
     <section
       class="mx_DeviceDetails_section"
@@ -108,6 +109,7 @@ exports[`<DeviceDetails /> renders device with metadata 1`] = `
 <div>
   <div
     class="mx_DeviceDetails"
+    data-testid="device-detail-my-device"
   >
     <section
       class="mx_DeviceDetails_section"
@@ -216,6 +218,7 @@ exports[`<DeviceDetails /> renders device without metadata 1`] = `
 <div>
   <div
     class="mx_DeviceDetails"
+    data-testid="device-detail-my-device"
   >
     <section
       class="mx_DeviceDetails_section"

diff --git a/test/components/views/settings/devices/__snapshots__/DeviceExpandDetailsButton-test.tsx.snap b/test/components/views/settings/devices/__snapshots__/DeviceExpandDetailsButton-test.tsx.snap
@@ -4,6 +4,7 @@ exports[`<DeviceExpandDetailsButton /> renders when expanded 1`] = `
 Object {
   "container": <div>
     <div
+      aria-label="Toggle device details"
       class="mx_AccessibleButton mx_DeviceExpandDetailsButton mx_DeviceExpandDetailsButton_expanded mx_AccessibleButton_hasKind mx_AccessibleButton_kind_icon"
       role="button"
       tabindex="0"
@@ -20,6 +21,7 @@ exports[`<DeviceExpandDetailsButton /> renders when not expanded 1`] = `
 Object {
   "container": <div>
     <div
+      aria-label="Toggle device details"
       class="mx_AccessibleButton mx_DeviceExpandDetailsButton mx_AccessibleButton_hasKind mx_AccessibleButton_kind_icon"
       role="button"
       tabindex="0"

diff --git a/test/components/views/settings/tabs/user/SessionManagerTab-test.tsx b/test/components/views/settings/tabs/user/SessionManagerTab-test.tsx
@@ -15,7 +15,7 @@ limitations under the License.
 */
 
 import React from 'react';
-import { render } from '@testing-library/react';
+import { fireEvent, render } 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';
@@ -192,4 +192,63 @@ describe('<SessionManagerTab />', () => {
 
         expect(getByTestId('other-sessions-section')).toBeTruthy();
     });
+
+    describe('device detail expansion', () => {
+        it('renders no devices expanded by default', async () => {
+            mockClient.getDevices.mockResolvedValue({
+                devices: [alicesDevice, alicesOlderMobileDevice, alicesMobileDevice],
+            });
+            const { getByTestId } = render(getComponent());
+
+            await act(async () => {
+                await flushPromisesWithFakeTimers();
+            });
+
+            const otherSessionsSection = getByTestId('other-sessions-section');
+
+            // no expanded device details
+            expect(otherSessionsSection.getElementsByClassName('mx_DeviceDetails').length).toBeFalsy();
+        });
+
+        it('toggles device expansion on click', async () => {
+            mockClient.getDevices.mockResolvedValue({
+                devices: [alicesDevice, alicesOlderMobileDevice, alicesMobileDevice],
+            });
+            const { getByTestId, queryByTestId } = render(getComponent());
+
+            await act(async () => {
+                await flushPromisesWithFakeTimers();
+            });
+
+            act(() => {
+                const tile = getByTestId(`device-tile-${alicesOlderMobileDevice.device_id}`);
+                const toggle = tile.querySelector('[aria-label="Toggle device details"]');
+                fireEvent.click(toggle);
+            });
+
+            // device details are expanded
+            expect(getByTestId(`device-detail-${alicesOlderMobileDevice.device_id}`)).toBeTruthy();
+
+            act(() => {
+                const tile = getByTestId(`device-tile-${alicesMobileDevice.device_id}`);
+                const toggle = tile.querySelector('[aria-label="Toggle device details"]');
+                fireEvent.click(toggle);
+            });
+
+            // both device details are expanded
+            expect(getByTestId(`device-detail-${alicesOlderMobileDevice.device_id}`)).toBeTruthy();
+            expect(getByTestId(`device-detail-${alicesMobileDevice.device_id}`)).toBeTruthy();
+
+            act(() => {
+                const tile = getByTestId(`device-tile-${alicesMobileDevice.device_id}`);
+                const toggle = tile.querySelector('[aria-label="Toggle device details"]');
+                fireEvent.click(toggle);
+            });
+
+            // alicesMobileDevice was toggled off
+            expect(queryByTestId(`device-detail-${alicesMobileDevice.device_id}`)).toBeFalsy();
+            // alicesOlderMobileDevice stayed open
+            expect(getByTestId(`device-detail-${alicesOlderMobileDevice.device_id}`)).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
@@ -41,6 +41,7 @@ exports[`<SessionManagerTab /> renders current session section with a verified s
         class="mx_DeviceTile_actions"
       >
         <div
+          aria-label="Toggle device details"
           class="mx_AccessibleButton mx_DeviceExpandDetailsButton mx_AccessibleButton_hasKind mx_AccessibleButton_kind_icon"
           data-testid="current-session-toggle-details"
           role="button"
@@ -124,6 +125,7 @@ exports[`<SessionManagerTab /> renders current session section with an unverifie
         class="mx_DeviceTile_actions"
       >
         <div
+          aria-label="Toggle device details"
           class="mx_AccessibleButton mx_DeviceExpandDetailsButton mx_AccessibleButton_hasKind mx_AccessibleButton_kind_icon"
           data-testid="current-session-toggle-details"
           role="button"
@@ -195,6 +197,7 @@ exports[`<SessionManagerTab /> sets device verification status correctly 1`] = `
     class="mx_DeviceTile_actions"
   >
     <div
+      aria-label="Toggle device details"
       class="mx_AccessibleButton mx_DeviceExpandDetailsButton mx_AccessibleButton_hasKind mx_AccessibleButton_kind_icon"
       data-testid="current-session-toggle-details"
       role="button"