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
matrix-org · Oct 14, 2022 · 17fce6c · 17fce6c
1 parent 77543b3
commit 17fce6c
Show file tree

Hide file tree

Showing 14 changed files with 69 additions and 22 deletions.
diff --git 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();
 

diff --git 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;
 }

diff --git 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 {

diff --git 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>

diff --git 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'

diff --git 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>;

diff --git 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

diff --git 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}`}

diff --git 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 }));
 

diff --git 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();

diff --git 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', () => {

diff --git 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

diff --git 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

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
@@ -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