From 54a66bd242d7c9d236f20dda18a6cb6d319a00c6 Mon Sep 17 00:00:00 2001 From: Kerry Date: Wed, 31 Aug 2022 14:34:22 +0200 Subject: [PATCH] Device manager - scroll to filtered list from security recommendations (PSG-640) (#9227) * scroll to filtered list from security recommendations * test sessionmanager scroll to * stable snapshot * fix strict errors * prtidy * use smooth scrolling --- .../settings/devices/FilteredDeviceList.tsx | 133 +++++++++--------- .../devices/SecurityRecommendations.tsx | 15 +- .../settings/tabs/user/SessionManagerTab.tsx | 29 +++- .../devices/FilteredDeviceList-test.tsx | 2 +- .../devices/SecurityRecommendations-test.tsx | 36 ++++- .../SecurityRecommendations-test.tsx.snap | 6 + .../tabs/user/SessionManagerTab-test.tsx | 41 ++++-- .../SessionManagerTab-test.tsx.snap | 36 +++++ 8 files changed, 204 insertions(+), 94 deletions(-) diff --git a/src/components/views/settings/devices/FilteredDeviceList.tsx b/src/components/views/settings/devices/FilteredDeviceList.tsx index 5687cdf0af1..0c88a507c6d 100644 --- a/src/components/views/settings/devices/FilteredDeviceList.tsx +++ b/src/components/views/settings/devices/FilteredDeviceList.tsx @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import React from 'react'; +import React, { ForwardedRef, forwardRef } from 'react'; import { _t } from '../../../../languageHandler'; import AccessibleButton from '../../elements/AccessibleButton'; @@ -150,70 +150,69 @@ const DeviceListItem: React.FC<{ * Filtered list of devices * Sorted by latest activity descending */ -const FilteredDeviceList: React.FC = ({ - devices, - filter, - expandedDeviceIds, - onFilterChange, - onDeviceExpandToggle, -}) => { - const sortedDevices = getFilteredSortedDevices(devices, filter); - - const options: FilterDropdownOption[] = [ - { id: ALL_FILTER_ID, label: _t('All') }, - { - id: DeviceSecurityVariation.Verified, - label: _t('Verified'), - description: _t('Ready for secure messaging'), - }, - { - id: DeviceSecurityVariation.Unverified, - label: _t('Unverified'), - description: _t('Not ready for secure messaging'), - }, - { - id: DeviceSecurityVariation.Inactive, - label: _t('Inactive'), - description: _t( - 'Inactive for %(inactiveAgeDays)s days or longer', - { inactiveAgeDays: INACTIVE_DEVICE_AGE_DAYS }, - ), - }, - ]; - - const onFilterOptionChange = (filterId: DeviceFilterKey) => { - onFilterChange(filterId === ALL_FILTER_ID ? undefined : filterId as DeviceSecurityVariation); - }; - - return
-
- - { _t('Sessions') } - - - id='device-list-filter' - label={_t('Filter devices')} - value={filter || ALL_FILTER_ID} - onOptionChange={onFilterOptionChange} - options={options} - selectedLabel={_t('Show')} - /> -
- { !!sortedDevices.length - ? - : onFilterChange(undefined)} /> - } -
    - { sortedDevices.map((device) => onDeviceExpandToggle(device.device_id)} - />, - ) } -
-
- ; -}; +export const FilteredDeviceList = + forwardRef(({ + devices, + filter, + expandedDeviceIds, + onFilterChange, + onDeviceExpandToggle, + }: Props, ref: ForwardedRef) => { + const sortedDevices = getFilteredSortedDevices(devices, filter); + + const options: FilterDropdownOption[] = [ + { id: ALL_FILTER_ID, label: _t('All') }, + { + id: DeviceSecurityVariation.Verified, + label: _t('Verified'), + description: _t('Ready for secure messaging'), + }, + { + id: DeviceSecurityVariation.Unverified, + label: _t('Unverified'), + description: _t('Not ready for secure messaging'), + }, + { + id: DeviceSecurityVariation.Inactive, + label: _t('Inactive'), + description: _t( + 'Inactive for %(inactiveAgeDays)s days or longer', + { inactiveAgeDays: INACTIVE_DEVICE_AGE_DAYS }, + ), + }, + ]; + + const onFilterOptionChange = (filterId: DeviceFilterKey) => { + onFilterChange(filterId === ALL_FILTER_ID ? undefined : filterId as DeviceSecurityVariation); + }; + + return
+
+ + { _t('Sessions') } + + + id='device-list-filter' + label={_t('Filter devices')} + value={filter || ALL_FILTER_ID} + onOptionChange={onFilterOptionChange} + options={options} + selectedLabel={_t('Show')} + /> +
+ { !!sortedDevices.length + ? + : onFilterChange(undefined)} /> + } +
    + { sortedDevices.map((device) => onDeviceExpandToggle(device.device_id)} + />, + ) } +
+
; + }); -export default FilteredDeviceList; diff --git a/src/components/views/settings/devices/SecurityRecommendations.tsx b/src/components/views/settings/devices/SecurityRecommendations.tsx index 00181f5674a..82eb49fd474 100644 --- a/src/components/views/settings/devices/SecurityRecommendations.tsx +++ b/src/components/views/settings/devices/SecurityRecommendations.tsx @@ -29,9 +29,13 @@ import { interface Props { devices: DevicesDictionary; + goToFilteredList: (filter: DeviceSecurityVariation) => void; } -const SecurityRecommendations: React.FC = ({ devices }) => { +const SecurityRecommendations: React.FC = ({ + devices, + goToFilteredList, +}) => { const devicesArray = Object.values(devices); const unverifiedDevicesCount = filterDevicesBySecurityRecommendation( @@ -49,9 +53,6 @@ const SecurityRecommendations: React.FC = ({ devices }) => { const inactiveAgeDays = INACTIVE_DEVICE_AGE_DAYS; - // TODO(kerrya) stubbed until PSG-640/652 - const noop = () => {}; - return = ({ devices }) => { > goToFilteredList(DeviceSecurityVariation.Unverified)} + data-testid='unverified-devices-cta' > { _t('View all') + ` (${unverifiedDevicesCount})` } @@ -90,7 +92,8 @@ const SecurityRecommendations: React.FC = ({ devices }) => { > goToFilteredList(DeviceSecurityVariation.Inactive)} + data-testid='inactive-devices-cta' > { _t('View all') + ` (${inactiveDevicesCount})` } diff --git a/src/components/views/settings/tabs/user/SessionManagerTab.tsx b/src/components/views/settings/tabs/user/SessionManagerTab.tsx index c4878dbb372..607549ec4da 100644 --- a/src/components/views/settings/tabs/user/SessionManagerTab.tsx +++ b/src/components/views/settings/tabs/user/SessionManagerTab.tsx @@ -14,12 +14,12 @@ See the License for the specific language governing permissions and limitations under the License. */ -import React, { useState } from 'react'; +import React, { useEffect, useRef, useState } from 'react'; import { _t } from "../../../../../languageHandler"; import { useOwnDevices } from '../../devices/useOwnDevices'; import SettingsSubsection from '../../shared/SettingsSubsection'; -import FilteredDeviceList from '../../devices/FilteredDeviceList'; +import { FilteredDeviceList } from '../../devices/FilteredDeviceList'; import CurrentDeviceSection from '../../devices/CurrentDeviceSection'; import SecurityRecommendations from '../../devices/SecurityRecommendations'; import { DeviceSecurityVariation, DeviceWithVerification } from '../../devices/types'; @@ -28,7 +28,9 @@ import SettingsTab from '../SettingsTab'; const SessionManagerTab: React.FC = () => { const { devices, currentDeviceId, isLoading } = useOwnDevices(); const [filter, setFilter] = useState(); - const [expandedDeviceIds, setExpandedDeviceIds] = useState([]); + const [expandedDeviceIds, setExpandedDeviceIds] = useState([]); + const filteredDeviceListRef = useRef(null); + const scrollIntoViewTimeoutRef = useRef>(); const onDeviceExpandToggle = (deviceId: DeviceWithVerification['device_id']): void => { if (expandedDeviceIds.includes(deviceId)) { @@ -38,11 +40,29 @@ const SessionManagerTab: React.FC = () => { } }; + const onGoToFilteredList = (filter: DeviceSecurityVariation) => { + setFilter(filter); + // @TODO(kerrya) clear selection when added in PSG-659 + clearTimeout(scrollIntoViewTimeoutRef.current); + // wait a tick for the filtered section to rerender with different height + scrollIntoViewTimeoutRef.current = + window.setTimeout(() => filteredDeviceListRef.current?.scrollIntoView({ + // align element to top of scrollbox + block: 'start', + inline: 'nearest', + behavior: 'smooth', + })); + }; + const { [currentDeviceId]: currentDevice, ...otherDevices } = devices; const shouldShowOtherSessions = Object.keys(otherDevices).length > 0; + useEffect(() => () => { + clearTimeout(scrollIntoViewTimeoutRef.current); + }, [scrollIntoViewTimeoutRef]); + return - + { expandedDeviceIds={expandedDeviceIds} onFilterChange={setFilter} onDeviceExpandToggle={onDeviceExpandToggle} + ref={filteredDeviceListRef} /> } diff --git a/test/components/views/settings/devices/FilteredDeviceList-test.tsx b/test/components/views/settings/devices/FilteredDeviceList-test.tsx index 68ef60e76b5..ef5ae5b18b4 100644 --- a/test/components/views/settings/devices/FilteredDeviceList-test.tsx +++ b/test/components/views/settings/devices/FilteredDeviceList-test.tsx @@ -17,7 +17,7 @@ limitations under the License. import React from 'react'; import { act, fireEvent, render } from '@testing-library/react'; -import FilteredDeviceList from '../../../../../src/components/views/settings/devices/FilteredDeviceList'; +import { FilteredDeviceList } from '../../../../../src/components/views/settings/devices/FilteredDeviceList'; import { DeviceSecurityVariation } from '../../../../../src/components/views/settings/devices/types'; import { flushPromises, mockPlatformPeg } from '../../../../test-utils'; diff --git a/test/components/views/settings/devices/SecurityRecommendations-test.tsx b/test/components/views/settings/devices/SecurityRecommendations-test.tsx index b54745545b7..e589a995c59 100644 --- a/test/components/views/settings/devices/SecurityRecommendations-test.tsx +++ b/test/components/views/settings/devices/SecurityRecommendations-test.tsx @@ -15,9 +15,10 @@ limitations under the License. */ import React from 'react'; -import { render } from '@testing-library/react'; +import { act, fireEvent, render } from '@testing-library/react'; import SecurityRecommendations from '../../../../../src/components/views/settings/devices/SecurityRecommendations'; +import { DeviceSecurityVariation } from '../../../../../src/components/views/settings/devices/types'; const MS_DAY = 24 * 60 * 60 * 1000; describe('', () => { @@ -32,6 +33,7 @@ describe('', () => { const defaultProps = { devices: {}, + goToFilteredList: jest.fn(), }; const getComponent = (props = {}) => (); @@ -69,4 +71,36 @@ describe('', () => { const { container } = render(getComponent({ devices })); expect(container).toMatchSnapshot(); }); + + it('clicking view all unverified devices button works', () => { + const goToFilteredList = jest.fn(); + const devices = { + [verifiedNoMetadata.device_id]: verifiedNoMetadata, + [hundredDaysOld.device_id]: hundredDaysOld, + [unverifiedNoMetadata.device_id]: unverifiedNoMetadata, + }; + const { getByTestId } = render(getComponent({ devices, goToFilteredList })); + + act(() => { + fireEvent.click(getByTestId('unverified-devices-cta')); + }); + + expect(goToFilteredList).toHaveBeenCalledWith(DeviceSecurityVariation.Unverified); + }); + + it('clicking view all inactive devices button works', () => { + const goToFilteredList = jest.fn(); + const devices = { + [verifiedNoMetadata.device_id]: verifiedNoMetadata, + [hundredDaysOld.device_id]: hundredDaysOld, + [unverifiedNoMetadata.device_id]: unverifiedNoMetadata, + }; + const { getByTestId } = render(getComponent({ devices, goToFilteredList })); + + act(() => { + fireEvent.click(getByTestId('inactive-devices-cta')); + }); + + expect(goToFilteredList).toHaveBeenCalledWith(DeviceSecurityVariation.Inactive); + }); }); diff --git a/test/components/views/settings/devices/__snapshots__/SecurityRecommendations-test.tsx.snap b/test/components/views/settings/devices/__snapshots__/SecurityRecommendations-test.tsx.snap index 0ee9870b780..b122d918262 100644 --- a/test/components/views/settings/devices/__snapshots__/SecurityRecommendations-test.tsx.snap +++ b/test/components/views/settings/devices/__snapshots__/SecurityRecommendations-test.tsx.snap @@ -48,6 +48,7 @@ exports[` renders both cards when user has both unver >
@@ -88,6 +89,7 @@ exports[` renders both cards when user has both unver >
@@ -149,6 +151,7 @@ exports[` renders inactive devices section when user >
@@ -189,6 +192,7 @@ exports[` renders inactive devices section when user >
@@ -250,6 +254,7 @@ exports[` renders unverified devices section when use >
@@ -290,6 +295,7 @@ exports[` renders unverified devices section when use >
diff --git a/test/components/views/settings/tabs/user/SessionManagerTab-test.tsx b/test/components/views/settings/tabs/user/SessionManagerTab-test.tsx index 53e4d408a96..aad6fbcd380 100644 --- a/test/components/views/settings/tabs/user/SessionManagerTab-test.tsx +++ b/test/components/views/settings/tabs/user/SessionManagerTab-test.tsx @@ -193,6 +193,23 @@ describe('', () => { expect(getByTestId('other-sessions-section')).toBeTruthy(); }); + it('goes to filtered list from security recommendations', async () => { + mockClient.getDevices.mockResolvedValue({ devices: [alicesDevice, alicesMobileDevice] }); + const { getByTestId, container } = render(getComponent()); + + await act(async () => { + await flushPromisesWithFakeTimers(); + }); + + fireEvent.click(getByTestId('unverified-devices-cta')); + + // our session manager waits a tick for rerender + await flushPromisesWithFakeTimers(); + + // unverified filter is set + expect(container.querySelector('.mx_FilteredDeviceList_header')).toMatchSnapshot(); + }); + describe('device detail expansion', () => { it('renders no devices expanded by default', async () => { mockClient.getDevices.mockResolvedValue({ @@ -220,30 +237,24 @@ describe('', () => { await flushPromisesWithFakeTimers(); }); - act(() => { - const tile = getByTestId(`device-tile-${alicesOlderMobileDevice.device_id}`); - const toggle = tile.querySelector('[aria-label="Toggle device details"]'); - fireEvent.click(toggle); - }); + const tile1 = getByTestId(`device-tile-${alicesOlderMobileDevice.device_id}`); + const toggle1 = tile1.querySelector('[aria-label="Toggle device details"]') as Element; + fireEvent.click(toggle1); // 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); - }); + const tile2 = getByTestId(`device-tile-${alicesMobileDevice.device_id}`); + const toggle2 = tile2.querySelector('[aria-label="Toggle device details"]') as Element; + fireEvent.click(toggle2); // 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); - }); + const tile3 = getByTestId(`device-tile-${alicesMobileDevice.device_id}`); + const toggle3 = tile3.querySelector('[aria-label="Toggle device details"]') as Element; + fireEvent.click(toggle3); // alicesMobileDevice was toggled off expect(queryByTestId(`device-detail-${alicesMobileDevice.device_id}`)).toBeFalsy(); 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 fd719cf1976..468556195fc 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 @@ -1,5 +1,41 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[` goes to filtered list from security recommendations 1`] = ` +
+ + Sessions + +
+ +
+
+`; + exports[` renders current session section with a verified session 1`] = `