Device manager - device tile main click target (#9409)

* change device tile click to toggle details instead of selection * lint * test current device section click * stuck cypress
2024-11-15 20:54:59 +08:00 · 2022-10-14 13:43:20 +02:00 · 2022-10-14 13:43:20 +02:00 · 17fce6ccb2
commit 17fce6ccb2
parent 77543b32d4
14 changed files with 69 additions and 22 deletions
--- a/cypress/e2e/settings/device-management.spec.ts
+++ b/cypress/e2e/settings/device-management.spec.ts
@ -74,8 +74,8 @@ describe("Device manager", () => {
        cy.get('.mx_FilteredDeviceList_list').find('.mx_FilteredDeviceList_listItem').should('have.length', 3);

        // select two sessions
-        cy.get('.mx_FilteredDeviceList_list .mx_FilteredDeviceList_listItem').first().click();
-        cy.get('.mx_FilteredDeviceList_list .mx_FilteredDeviceList_listItem').last().click();
+        cy.get('.mx_FilteredDeviceList_list .mx_FilteredDeviceList_listItem .mx_Checkbox').first().click();
+        cy.get('.mx_FilteredDeviceList_list .mx_FilteredDeviceList_listItem .mx_Checkbox').last().click();
        // sign out from list selection action buttons
        cy.get('[data-testid="sign-out-selection-cta"]').click();
        // list updated after sign out
@ -84,7 +84,7 @@ describe("Device manager", () => {
        cy.get('[data-testid="unverified-devices-cta"]').should('have.text', 'View all (1)');

        const sessionName = `Alice's device`;
-        // select the first session
+        // open the first session
        cy.get('.mx_FilteredDeviceList_list .mx_FilteredDeviceList_listItem').first().within(() => {
            cy.get('[aria-label="Toggle device details"]').click();

--- a/res/css/components/views/settings/devices/_DeviceTile.pcss
+++ b/res/css/components/views/settings/devices/_DeviceTile.pcss
@ -21,6 +21,10 @@ limitations under the License.
    width: 100%;
 }

+.mx_DeviceTile_interactive {
+    cursor: pointer;
+}
+
 .mx_DeviceTile_info {
    flex: 1 1 0;
 }
--- a/res/css/components/views/settings/devices/_SelectableDeviceTile.pcss
+++ b/res/css/components/views/settings/devices/_SelectableDeviceTile.pcss
@ -19,7 +19,6 @@ limitations under the License.
    flex-direction: row;
    align-items: center;
    width: 100%;
-    cursor: pointer;
 }

 .mx_SelectableDeviceTile_checkbox {
--- a/src/components/views/settings/DevicesPanelEntry.tsx
+++ b/src/components/views/settings/DevicesPanelEntry.tsx
@ -173,7 +173,7 @@ export default class DevicesPanelEntry extends React.Component<IProps, IState> {

        return (
            <div className="mx_DevicesPanel_device">
-                <SelectableDeviceTile device={extendedDevice} onClick={this.onDeviceToggled} isSelected={this.props.selected}>
+                <SelectableDeviceTile device={extendedDevice} onSelect={this.onDeviceToggled} isSelected={this.props.selected}>
                    { buttons }
                </SelectableDeviceTile>
            </div>
--- a/src/components/views/settings/devices/CurrentDeviceSection.tsx
+++ b/src/components/views/settings/devices/CurrentDeviceSection.tsx
@ -105,6 +105,7 @@ const CurrentDeviceSection: React.FC<Props> = ({
        { !!device && <>
            <DeviceTile
                device={device}
+                onClick={() => setIsExpanded(!isExpanded)}
            >
                <DeviceExpandDetailsButton
                    data-testid='current-session-toggle-details'
--- a/src/components/views/settings/devices/DeviceTile.tsx
+++ b/src/components/views/settings/devices/DeviceTile.tsx
@ -15,6 +15,7 @@ limitations under the License.
 */

 import React, { Fragment } from "react";
+import classNames from "classnames";

 import { Icon as InactiveIcon } from '../../../../../res/img/element-icons/settings/inactive.svg';
 import { _t } from "../../../../languageHandler";
@ -23,6 +24,7 @@ import Heading from "../../typography/Heading";
 import { INACTIVE_DEVICE_AGE_DAYS, isDeviceInactive } from "./filter";
 import { ExtendedDevice } from "./types";
 import { DeviceTypeIcon } from "./DeviceTypeIcon";
+import { preventDefaultWrapper } from "../../../../utils/NativeEventUtils";
 export interface DeviceTileProps {
    device: ExtendedDevice;
    isSelected?: boolean;
@ -88,13 +90,20 @@ const DeviceTile: React.FC<DeviceTileProps> = ({
            { id: 'deviceId', value: device.device_id },
        ];

-    return <div className="mx_DeviceTile" data-testid={`device-tile-${device.device_id}`}>
+    return <div
+        className={classNames(
+            "mx_DeviceTile",
+            { "mx_DeviceTile_interactive": !!onClick },
+        )}
+        data-testid={`device-tile-${device.device_id}`}
+        onClick={onClick}
+    >
        <DeviceTypeIcon
            isVerified={device.isVerified}
            isSelected={isSelected}
            deviceType={device.deviceType}
        />
-        <div className="mx_DeviceTile_info" onClick={onClick}>
+        <div className="mx_DeviceTile_info">
            <DeviceTileName device={device} />
            <div className="mx_DeviceTile_metadata">
                { metadata.map(({ id, value }, index) =>
@ -107,7 +116,7 @@ const DeviceTile: React.FC<DeviceTileProps> = ({
                ) }
            </div>
        </div>
-        <div className="mx_DeviceTile_actions">
+        <div className="mx_DeviceTile_actions" onClick={preventDefaultWrapper(() => {})}>
            { children }
        </div>
    </div>;
--- a/src/components/views/settings/devices/FilteredDeviceList.tsx
+++ b/src/components/views/settings/devices/FilteredDeviceList.tsx
@ -179,7 +179,8 @@ const DeviceListItem: React.FC<{
 }) => <li className='mx_FilteredDeviceList_listItem'>
    <SelectableDeviceTile
        isSelected={isSelected}
-        onClick={toggleSelected}
+        onSelect={toggleSelected}
+        onClick={onDeviceExpandToggle}
        device={device}
    >
        <DeviceExpandDetailsButton
--- a/src/components/views/settings/devices/SelectableDeviceTile.tsx
+++ b/src/components/views/settings/devices/SelectableDeviceTile.tsx
@ -21,15 +21,22 @@ import DeviceTile, { DeviceTileProps } from './DeviceTile';

 interface Props extends DeviceTileProps {
    isSelected: boolean;
-    onClick: () => void;
+    onSelect: () => void;
+    onClick?: () => void;
 }

-const SelectableDeviceTile: React.FC<Props> = ({ children, device, isSelected, onClick }) => {
+const SelectableDeviceTile: React.FC<Props> = ({
+    children,
+    device,
+    isSelected,
+    onSelect,
+    onClick,
+}) => {
    return <div className='mx_SelectableDeviceTile'>
        <StyledCheckbox
            kind={CheckboxStyle.Solid}
            checked={isSelected}
-            onChange={onClick}
+            onChange={onSelect}
            className='mx_SelectableDeviceTile_checkbox'
            id={`device-tile-checkbox-${device.device_id}`}
            data-testid={`device-tile-checkbox-${device.device_id}`}
--- a/test/components/views/settings/devices/CurrentDeviceSection-test.tsx
+++ b/test/components/views/settings/devices/CurrentDeviceSection-test.tsx
@ -67,6 +67,23 @@ describe('<CurrentDeviceSection />', () => {
        expect(container).toMatchSnapshot();
    });

+    it('displays device details on main tile click', () => {
+        const { getByTestId, container } = render(getComponent({ device: alicesUnverifiedDevice }));
+
+        act(() => {
+            fireEvent.click(getByTestId(`device-tile-${alicesUnverifiedDevice.device_id}`));
+        });
+
+        expect(container.getElementsByClassName('mx_DeviceDetails').length).toBeTruthy();
+
+        act(() => {
+            fireEvent.click(getByTestId(`device-tile-${alicesUnverifiedDevice.device_id}`));
+        });
+
+        // device details are hidden
+        expect(container.getElementsByClassName('mx_DeviceDetails').length).toBeFalsy();
+    });
+
    it('displays device details on toggle click', () => {
        const { container, getByTestId } = render(getComponent({ device: alicesUnverifiedDevice }));

--- a/test/components/views/settings/devices/DeviceTile-test.tsx
+++ b/test/components/views/settings/devices/DeviceTile-test.tsx
@ -46,6 +46,14 @@ describe('<DeviceTile />', () => {
        expect(container).toMatchSnapshot();
    });

+    it('applies interactive class when tile has click handler', () => {
+        const onClick = jest.fn();
+        const { getByTestId } = render(getComponent({ onClick }));
+        expect(
+            getByTestId('device-tile-123').className.includes('mx_DeviceTile_interactive'),
+        ).toBeTruthy();
+    });
+
    it('renders a verified device with no metadata', () => {
        const { container } = render(getComponent());
        expect(container).toMatchSnapshot();
--- a/test/components/views/settings/devices/SelectableDeviceTile-test.tsx
+++ b/test/components/views/settings/devices/SelectableDeviceTile-test.tsx
@ -30,6 +30,7 @@ describe('<SelectableDeviceTile />', () => {
        deviceType: DeviceType.Unknown,
    };
    const defaultProps = {
+        onSelect: jest.fn(),
        onClick: jest.fn(),
        device,
        children: <div>test</div>,
@ -48,15 +49,15 @@ describe('<SelectableDeviceTile />', () => {
        expect(container.querySelector(`#device-tile-checkbox-${device.device_id}`)).toMatchSnapshot();
    });

-    it('calls onClick on checkbox click', () => {
-        const onClick = jest.fn();
-        const { container } = render(getComponent({ onClick }));
+    it('calls onSelect on checkbox click', () => {
+        const onSelect = jest.fn();
+        const { container } = render(getComponent({ onSelect }));

        act(() => {
            fireEvent.click(container.querySelector(`#device-tile-checkbox-${device.device_id}`));
        });

-        expect(onClick).toHaveBeenCalled();
+        expect(onSelect).toHaveBeenCalled();
    });

    it('calls onClick on device tile info click', () => {
--- a/test/components/views/settings/devices/snapshots/CurrentDeviceSection-test.tsx.snap
+++ b/test/components/views/settings/devices/snapshots/CurrentDeviceSection-test.tsx.snap
@ -181,7 +181,7 @@ exports[`<CurrentDeviceSection /> renders device and correct security card when
      class="mx_SettingsSubsection_content"
    >
      <div
-        class="mx_DeviceTile"
+        class="mx_DeviceTile mx_DeviceTile_interactive"
        data-testid="device-tile-alices_device"
      >
        <div
@ -317,7 +317,7 @@ exports[`<CurrentDeviceSection /> renders device and correct security card when
      class="mx_SettingsSubsection_content"
    >
      <div
-        class="mx_DeviceTile"
+        class="mx_DeviceTile mx_DeviceTile_interactive"
        data-testid="device-tile-alices_device"
      >
        <div
--- a/test/components/views/settings/devices/snapshots/SelectableDeviceTile-test.tsx.snap
+++ b/test/components/views/settings/devices/snapshots/SelectableDeviceTile-test.tsx.snap
@ -35,7 +35,7 @@ exports[`<SelectableDeviceTile /> renders unselected device tile with checkbox 1
      </label>
    </span>
    <div
-      class="mx_DeviceTile"
+      class="mx_DeviceTile mx_DeviceTile_interactive"
      data-testid="device-tile-my-device"
    >
      <div
--- a/test/components/views/settings/tabs/user/snapshots/SessionManagerTab-test.tsx.snap
+++ b/test/components/views/settings/tabs/user/snapshots/SessionManagerTab-test.tsx.snap
@ -45,7 +45,7 @@ exports[`<SessionManagerTab /> current session section renders current session s
    class="mx_SettingsSubsection_content"
  >
    <div
-      class="mx_DeviceTile"
+      class="mx_DeviceTile mx_DeviceTile_interactive"
      data-testid="device-tile-alices_device"
    >
      <div
@ -167,7 +167,7 @@ exports[`<SessionManagerTab /> current session section renders current session s
    class="mx_SettingsSubsection_content"
  >
    <div
-      class="mx_DeviceTile"
+      class="mx_DeviceTile mx_DeviceTile_interactive"
      data-testid="device-tile-alices_device"
    >
      <div
@ -334,7 +334,7 @@ exports[`<SessionManagerTab /> goes to filtered list from security recommendatio

 exports[`<SessionManagerTab /> sets device verification status correctly 1`] = `
 <div
-  class="mx_DeviceTile"
+  class="mx_DeviceTile mx_DeviceTile_interactive"
  data-testid="device-tile-alices_device"
 >
  <div