Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

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

Merged
merged 8 commits into from
Aug 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -48,7 +51,6 @@ limitations under the License.
color: $secondary-content;

width: 100%;
margin-top: $spacing-20;

border-spacing: 0;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion src/components/views/settings/devices/DeviceDetails.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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} />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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,
Expand Down
43 changes: 35 additions & 8 deletions src/components/views/settings/devices/FilteredDeviceList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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 = [
Expand Down Expand Up @@ -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>
Expand Down
13 changes: 12 additions & 1 deletion src/components/views/settings/tabs/user/SessionManagerTab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -51,7 +60,9 @@ const SessionManagerTab: React.FC = () => {
<FilteredDeviceList
devices={otherDevices}
filter={filter}
expandedDeviceIds={expandedDeviceIds}
onFilterChange={setFilter}
onDeviceExpandToggle={onDeviceExpandToggle}
/>
</SettingsSubsection>
}
Expand Down
1 change: 1 addition & 0 deletions src/i18n/strings/en_EN.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ describe('<FilteredDeviceList />', () => {
};
const defaultProps = {
onFilterChange: jest.fn(),
onDeviceExpandToggle: jest.fn(),
expandedDeviceIds: [],
devices: {
[unverifiedNoMetadata.device_id]: unverifiedNoMetadata,
[verifiedNoMetadata.device_id]: verifiedNoMetadata,
Expand Down Expand Up @@ -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);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down