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
2022-08-31 14:34:22 +02:00 · 2022-08-31 14:34:22 +02:00 · 54a66bd242
parent 0d6a550c33
commit 54a66bd242
8 changed files with 201 additions and 91 deletions
--- 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<Props> = ({
-    devices,
-    filter,
-    expandedDeviceIds,
-    onFilterChange,
-    onDeviceExpandToggle,
-}) => {
-    const sortedDevices = getFilteredSortedDevices(devices, filter);
+export const FilteredDeviceList =
+    forwardRef(({
+        devices,
+        filter,
+        expandedDeviceIds,
+        onFilterChange,
+        onDeviceExpandToggle,
+    }: Props, ref: ForwardedRef<HTMLDivElement>) => {
+        const sortedDevices = getFilteredSortedDevices(devices, filter);

-    const options: FilterDropdownOption<DeviceFilterKey>[] = [
-        { 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 options: FilterDropdownOption<DeviceFilterKey>[] = [
+            { 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);
-    };
+        const onFilterOptionChange = (filterId: DeviceFilterKey) => {
+            onFilterChange(filterId === ALL_FILTER_ID ? undefined : filterId as DeviceSecurityVariation);
+        };

-    return <div className='mx_FilteredDeviceList'>
-        <div className='mx_FilteredDeviceList_header'>
-            <span className='mx_FilteredDeviceList_headerLabel'>
-                { _t('Sessions') }
-            </span>
-            <FilterDropdown<DeviceFilterKey>
-                id='device-list-filter'
-                label={_t('Filter devices')}
-                value={filter || ALL_FILTER_ID}
-                onOptionChange={onFilterOptionChange}
-                options={options}
-                selectedLabel={_t('Show')}
-            />
-        </div>
-        { !!sortedDevices.length
-            ? <FilterSecurityCard filter={filter} />
-            : <NoResults filter={filter} clearFilter={() => onFilterChange(undefined)} />
-        }
-        <ol className='mx_FilteredDeviceList_list'>
-            { sortedDevices.map((device) => <DeviceListItem
-                key={device.device_id}
-                device={device}
-                isExpanded={expandedDeviceIds.includes(device.device_id)}
-                onDeviceExpandToggle={() => onDeviceExpandToggle(device.device_id)}
-            />,
-            ) }
-        </ol>
-    </div>
-    ;
-};
+        return <div className='mx_FilteredDeviceList' ref={ref}>
+            <div className='mx_FilteredDeviceList_header'>
+                <span className='mx_FilteredDeviceList_headerLabel'>
+                    { _t('Sessions') }
+                </span>
+                <FilterDropdown<DeviceFilterKey>
+                    id='device-list-filter'
+                    label={_t('Filter devices')}
+                    value={filter || ALL_FILTER_ID}
+                    onOptionChange={onFilterOptionChange}
+                    options={options}
+                    selectedLabel={_t('Show')}
+                />
+            </div>
+            { !!sortedDevices.length
+                ? <FilterSecurityCard filter={filter} />
+                : <NoResults filter={filter} clearFilter={() => onFilterChange(undefined)} />
+            }
+            <ol className='mx_FilteredDeviceList_list'>
+                { sortedDevices.map((device) => <DeviceListItem
+                    key={device.device_id}
+                    device={device}
+                    isExpanded={expandedDeviceIds.includes(device.device_id)}
+                    onDeviceExpandToggle={() => onDeviceExpandToggle(device.device_id)}
+                />,
+                ) }
+            </ol>
+        </div>;
+    });

-export default FilteredDeviceList;
--- 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<Props> = ({ devices }) => {
+const SecurityRecommendations: React.FC<Props> = ({
+    devices,
+    goToFilteredList,
+}) => {
    const devicesArray = Object.values<DeviceWithVerification>(devices);

    const unverifiedDevicesCount = filterDevicesBySecurityRecommendation(
@ -49,9 +53,6 @@ const SecurityRecommendations: React.FC<Props> = ({ devices }) => {

    const inactiveAgeDays = INACTIVE_DEVICE_AGE_DAYS;

-    // TODO(kerrya) stubbed until PSG-640/652
-    const noop = () => {};
-
    return <SettingsSubsection
        heading={_t('Security recommendations')}
        description={_t('Improve your account security by following these recommendations')}
@ -69,7 +70,8 @@ const SecurityRecommendations: React.FC<Props> = ({ devices }) => {
            >
                <AccessibleButton
                    kind='link_inline'
-                    onClick={noop}
+                    onClick={() => goToFilteredList(DeviceSecurityVariation.Unverified)}
+                    data-testid='unverified-devices-cta'
                >
                    { _t('View all') + ` (${unverifiedDevicesCount})` }
                </AccessibleButton>
@ -90,7 +92,8 @@ const SecurityRecommendations: React.FC<Props> = ({ devices }) => {
                >
                    <AccessibleButton
                        kind='link_inline'
-                        onClick={noop}
+                        onClick={() => goToFilteredList(DeviceSecurityVariation.Inactive)}
+                        data-testid='inactive-devices-cta'
                    >
                        { _t('View all') + ` (${inactiveDevicesCount})` }
                    </AccessibleButton>
--- 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<DeviceSecurityVariation>();
-    const [expandedDeviceIds, setExpandedDeviceIds] = useState([]);
+    const [expandedDeviceIds, setExpandedDeviceIds] = useState<DeviceWithVerification['device_id'][]>([]);
+    const filteredDeviceListRef = useRef<HTMLDivElement>(null);
+    const scrollIntoViewTimeoutRef = useRef<ReturnType<typeof setTimeout>>();

    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 <SettingsTab heading={_t('Sessions')}>
-        <SecurityRecommendations devices={devices} />
+        <SecurityRecommendations devices={devices} goToFilteredList={onGoToFilteredList} />
        <CurrentDeviceSection
            device={currentDevice}
            isLoading={isLoading}
@ -63,6 +83,7 @@ const SessionManagerTab: React.FC = () => {
                    expandedDeviceIds={expandedDeviceIds}
                    onFilterChange={setFilter}
                    onDeviceExpandToggle={onDeviceExpandToggle}
+                    ref={filteredDeviceListRef}
                />
            </SettingsSubsection>
        }
--- 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';

--- 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('<SecurityRecommendations />', () => {
@ -32,6 +33,7 @@ describe('<SecurityRecommendations />', () => {

    const defaultProps = {
        devices: {},
+        goToFilteredList: jest.fn(),
    };
    const getComponent = (props = {}) =>
        (<SecurityRecommendations {...defaultProps} {...props} />);
@ -69,4 +71,36 @@ describe('<SecurityRecommendations />', () => {
        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);
+    });
 });
--- 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[`<SecurityRecommendations /> renders both cards when user has both unver
          >
            <div
              class="mx_AccessibleButton mx_AccessibleButton_hasKind mx_AccessibleButton_kind_link_inline"
+              data-testid="unverified-devices-cta"
              role="button"
              tabindex="0"
            >
@ -88,6 +89,7 @@ exports[`<SecurityRecommendations /> renders both cards when user has both unver
          >
            <div
              class="mx_AccessibleButton mx_AccessibleButton_hasKind mx_AccessibleButton_kind_link_inline"
+              data-testid="inactive-devices-cta"
              role="button"
              tabindex="0"
            >
@ -149,6 +151,7 @@ exports[`<SecurityRecommendations /> renders inactive devices section when user
          >
            <div
              class="mx_AccessibleButton mx_AccessibleButton_hasKind mx_AccessibleButton_kind_link_inline"
+              data-testid="unverified-devices-cta"
              role="button"
              tabindex="0"
            >
@ -189,6 +192,7 @@ exports[`<SecurityRecommendations /> renders inactive devices section when user
          >
            <div
              class="mx_AccessibleButton mx_AccessibleButton_hasKind mx_AccessibleButton_kind_link_inline"
+              data-testid="inactive-devices-cta"
              role="button"
              tabindex="0"
            >
@ -250,6 +254,7 @@ exports[`<SecurityRecommendations /> renders unverified devices section when use
          >
            <div
              class="mx_AccessibleButton mx_AccessibleButton_hasKind mx_AccessibleButton_kind_link_inline"
+              data-testid="unverified-devices-cta"
              role="button"
              tabindex="0"
            >
@ -290,6 +295,7 @@ exports[`<SecurityRecommendations /> renders unverified devices section when use
          >
            <div
              class="mx_AccessibleButton mx_AccessibleButton_hasKind mx_AccessibleButton_kind_link_inline"
+              data-testid="inactive-devices-cta"
              role="button"
              tabindex="0"
            >
--- 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('<SessionManagerTab />', () => {
        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('<SessionManagerTab />', () => {
                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();
--- 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[`<SessionManagerTab /> goes to filtered list from security recommendations 1`] = `
+<div
+  class="mx_FilteredDeviceList_header"
+>
+  <span
+    class="mx_FilteredDeviceList_headerLabel"
+  >
+    Sessions
+  </span>
+  <div
+    class="mx_Dropdown mx_FilterDropdown"
+  >
+    <div
+      aria-describedby="device-list-filter_value"
+      aria-expanded="false"
+      aria-haspopup="listbox"
+      aria-label="Filter devices"
+      aria-owns="device-list-filter_input"
+      class="mx_AccessibleButton mx_Dropdown_input mx_no_textinput"
+      role="button"
+      tabindex="0"
+    >
+      <div
+        class="mx_Dropdown_option"
+        id="device-list-filter_value"
+      >
+        Show: Unverified
+      </div>
+      <span
+        class="mx_Dropdown_arrow"
+      />
+    </div>
+  </div>
+</div>
+`;
+
 exports[`<SessionManagerTab /> renders current session section with a verified session 1`] = `
 <div
  class="mx_SettingsSubsection"