From 5a9c2e530a9c0ff59742aef0ad3a571c4944336d Mon Sep 17 00:00:00 2001
From: Kerry <kerrya@element.io>
Date: Tue, 9 Aug 2022 15:07:25 +0200
Subject: [PATCH] Device manager - selectable device tile wrapper (PSG-637)
 (#9153)

* add selectabledevicetile wrapper

* set pointer cursor

* line up own device icon with new checkboxes
---
 res/css/_components.pcss                      |  1 +
 .../devices/_SelectableDeviceTile.pcss        | 28 ++++++
 res/css/views/settings/_DevicesPanel.pcss     |  6 +-
 .../views/elements/StyledCheckbox.tsx         |  8 +-
 .../views/settings/DevicesPanelEntry.tsx      | 26 +++---
 .../views/settings/devices/DeviceTile.tsx     |  9 +-
 .../settings/devices/SelectableDeviceTile.tsx | 42 +++++++++
 .../LabelledCheckbox-test.tsx.snap            |  2 -
 .../devices/SelectableDeviceTile-test.tsx     | 85 +++++++++++++++++++
 .../SelectableDeviceTile-test.tsx.snap        | 71 ++++++++++++++++
 10 files changed, 255 insertions(+), 23 deletions(-)
 create mode 100644 res/css/components/views/settings/devices/_SelectableDeviceTile.pcss
 create mode 100644 src/components/views/settings/devices/SelectableDeviceTile.tsx
 create mode 100644 test/components/views/settings/devices/SelectableDeviceTile-test.tsx
 create mode 100644 test/components/views/settings/devices/__snapshots__/SelectableDeviceTile-test.tsx.snap

diff --git a/res/css/_components.pcss b/res/css/_components.pcss
index d4d7ecc316..d6445f0143 100644
--- a/res/css/_components.pcss
+++ b/res/css/_components.pcss
@@ -28,6 +28,7 @@
 @import "./components/views/messages/_MBeaconBody.pcss";
 @import "./components/views/messages/shared/_MediaProcessingError.pcss";
 @import "./components/views/settings/devices/_DeviceTile.pcss";
+@import "./components/views/settings/devices/_SelectableDeviceTile.pcss";
 @import "./components/views/settings/shared/_SettingsSubsection.pcss";
 @import "./components/views/spaces/_QuickThemeSwitcher.pcss";
 @import "./structures/_AutoHideScrollbar.pcss";
diff --git a/res/css/components/views/settings/devices/_SelectableDeviceTile.pcss b/res/css/components/views/settings/devices/_SelectableDeviceTile.pcss
new file mode 100644
index 0000000000..5d6a497e02
--- /dev/null
+++ b/res/css/components/views/settings/devices/_SelectableDeviceTile.pcss
@@ -0,0 +1,28 @@
+/*
+Copyright 2022 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+.mx_SelectableDeviceTile {
+    display: flex;
+    flex-direction: row;
+    align-items: center;
+    width: 100%;
+    cursor: pointer;
+}
+
+.mx_SelectableDeviceTile_checkbox {
+    flex: 0 0;
+    margin-right: $spacing-16;
+}
diff --git a/res/css/views/settings/_DevicesPanel.pcss b/res/css/views/settings/_DevicesPanel.pcss
index 9cbdb6a2a1..8581225cee 100644
--- a/res/css/views/settings/_DevicesPanel.pcss
+++ b/res/css/views/settings/_DevicesPanel.pcss
@@ -56,10 +56,12 @@ limitations under the License.
     align-items: flex-start;
     margin-block: 10px;
     min-height: 35px;
+    padding: 0 $spacing-8;
 }
 
-.mx_DevicesPanel_icon, .mx_DevicesPanel_checkbox {
-    margin-left: 9px;
+.mx_DevicesPanel_icon {
+    margin-left: 0px;
+    margin-right: $spacing-16;
     margin-top: 2px;
 }
 
diff --git a/src/components/views/elements/StyledCheckbox.tsx b/src/components/views/elements/StyledCheckbox.tsx
index 333fbb8adb..3542236682 100644
--- a/src/components/views/elements/StyledCheckbox.tsx
+++ b/src/components/views/elements/StyledCheckbox.tsx
@@ -70,9 +70,11 @@ export default class StyledCheckbox extends React.PureComponent<IProps, IState>
                 <div className="mx_Checkbox_background">
                     <div className="mx_Checkbox_checkmark" />
                 </div>
-                <div>
-                    { this.props.children }
-                </div>
+                { !!this.props.children &&
+                    <div>
+                        { this.props.children }
+                    </div>
+                }
             </label>
         </span>;
     }
diff --git a/src/components/views/settings/DevicesPanelEntry.tsx b/src/components/views/settings/DevicesPanelEntry.tsx
index 5a5330fd3e..b0301214b9 100644
--- a/src/components/views/settings/DevicesPanelEntry.tsx
+++ b/src/components/views/settings/DevicesPanelEntry.tsx
@@ -20,7 +20,6 @@ import { logger } from "matrix-js-sdk/src/logger";
 
 import { _t } from '../../../languageHandler';
 import { MatrixClientPeg } from '../../../MatrixClientPeg';
-import StyledCheckbox, { CheckboxStyle } from '../elements/StyledCheckbox';
 import AccessibleButton from "../elements/AccessibleButton";
 import Field from "../elements/Field";
 import Modal from "../../../Modal";
@@ -28,6 +27,7 @@ import SetupEncryptionDialog from '../dialogs/security/SetupEncryptionDialog';
 import VerificationRequestDialog from '../../views/dialogs/VerificationRequestDialog';
 import LogoutDialog from '../dialogs/LogoutDialog';
 import DeviceTile from './devices/DeviceTile';
+import SelectableDeviceTile from './devices/SelectableDeviceTile';
 
 interface IProps {
     device: IMyDevice;
@@ -133,14 +133,6 @@ export default class DevicesPanelEntry extends React.Component<IProps, IState> {
             </AccessibleButton>;
         }
 
-        const left = this.props.isOwnDevice ?
-            <div className="mx_DevicesPanel_deviceTrust">
-                <span className={"mx_DevicesPanel_icon mx_E2EIcon " + iconClass} />
-            </div> :
-            <div className="mx_DevicesPanel_checkbox">
-                <StyledCheckbox kind={CheckboxStyle.Outline} onChange={this.onDeviceToggled} checked={this.props.selected} />
-            </div>;
-
         const buttons = this.state.renaming ?
             <form className="mx_DevicesPanel_renameForm" onSubmit={this.onRenameSubmit}>
                 <Field
@@ -162,12 +154,22 @@ export default class DevicesPanelEntry extends React.Component<IProps, IState> {
                 </AccessibleButton>
             </React.Fragment>;
 
-        return (
-            <div className={"mx_DevicesPanel_device" + myDeviceClass}>
-                { left }
+        if (this.props.isOwnDevice) {
+            return <div className={"mx_DevicesPanel_device" + myDeviceClass}>
+                <div className="mx_DevicesPanel_deviceTrust">
+                    <span className={"mx_DevicesPanel_icon mx_E2EIcon " + iconClass} />
+                </div>
                 <DeviceTile device={this.props.device}>
                     { buttons }
                 </DeviceTile>
+            </div>;
+        }
+
+        return (
+            <div className={"mx_DevicesPanel_device" + myDeviceClass}>
+                <SelectableDeviceTile device={this.props.device} onClick={this.onDeviceToggled} isSelected={this.props.selected}>
+                    { buttons }
+                </SelectableDeviceTile>
             </div>
         );
     }
diff --git a/src/components/views/settings/devices/DeviceTile.tsx b/src/components/views/settings/devices/DeviceTile.tsx
index 03d952fbb1..33f9fc40a8 100644
--- a/src/components/views/settings/devices/DeviceTile.tsx
+++ b/src/components/views/settings/devices/DeviceTile.tsx
@@ -23,15 +23,16 @@ import TooltipTarget from "../../elements/TooltipTarget";
 import { Alignment } from "../../elements/Tooltip";
 import Heading from "../../typography/Heading";
 
-interface Props {
+export interface DeviceTileProps {
     device: IMyDevice;
     children?: React.ReactNode;
+    onClick?: () => void;
 }
 
 const DeviceTileName: React.FC<{ device: IMyDevice }> = ({ device }) => {
     if (device.display_name) {
         return <TooltipTarget
-            alignment={Alignment.Left}
+            alignment={Alignment.Top}
             label={`${device.display_name} (${device.device_id})`}
         >
             <Heading size='h4'>
@@ -59,7 +60,7 @@ const DeviceMetadata: React.FC<{ value: string, id: string }> = ({ value, id })
     value ? <span data-testid={`device-metadata-${id}`}>{ value }</span> : null
 );
 
-const DeviceTile: React.FC<Props> = ({ device, children }) => {
+const DeviceTile: React.FC<DeviceTileProps> = ({ device, children, onClick }) => {
     const lastActivity = device.last_seen_ts && `${_t('Last activity')} ${formatLastActivity(device.last_seen_ts)}`;
     const metadata = [
         { id: 'lastActivity', value: lastActivity },
@@ -67,7 +68,7 @@ const DeviceTile: React.FC<Props> = ({ device, children }) => {
     ];
 
     return <div className="mx_DeviceTile">
-        <div className="mx_DeviceTile_info">
+        <div className="mx_DeviceTile_info" onClick={onClick}>
             <DeviceTileName device={device} />
             <div className="mx_DeviceTile_metadata">
                 { metadata.map(({ id, value }, index) =>
diff --git a/src/components/views/settings/devices/SelectableDeviceTile.tsx b/src/components/views/settings/devices/SelectableDeviceTile.tsx
new file mode 100644
index 0000000000..e232e5ff50
--- /dev/null
+++ b/src/components/views/settings/devices/SelectableDeviceTile.tsx
@@ -0,0 +1,42 @@
+/*
+Copyright 2022 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import React from 'react';
+
+import StyledCheckbox, { CheckboxStyle } from '../../elements/StyledCheckbox';
+import DeviceTile, { DeviceTileProps } from './DeviceTile';
+
+interface Props extends DeviceTileProps {
+    isSelected: boolean;
+    onClick: () => void;
+}
+
+const SelectableDeviceTile: React.FC<Props> = ({ children, device, isSelected, onClick }) => {
+    return <div className='mx_SelectableDeviceTile'>
+        <StyledCheckbox
+            kind={CheckboxStyle.Solid}
+            checked={isSelected}
+            onChange={onClick}
+            className='mx_SelectableDeviceTile_checkbox'
+            id={`device-tile-checkbox-${device.device_id}`}
+        />
+        <DeviceTile device={device} onClick={onClick}>
+            { children }
+        </DeviceTile>
+    </div>;
+};
+
+export default SelectableDeviceTile;
diff --git a/test/components/views/elements/__snapshots__/LabelledCheckbox-test.tsx.snap b/test/components/views/elements/__snapshots__/LabelledCheckbox-test.tsx.snap
index 34cdbe59be..286c69a8d0 100644
--- a/test/components/views/elements/__snapshots__/LabelledCheckbox-test.tsx.snap
+++ b/test/components/views/elements/__snapshots__/LabelledCheckbox-test.tsx.snap
@@ -34,7 +34,6 @@ exports[`<LabelledCheckbox /> should render with byline of "this is a byline" 1`
               className="mx_Checkbox_checkmark"
             />
           </div>
-          <div />
         </label>
       </span>
     </StyledCheckbox>
@@ -90,7 +89,6 @@ exports[`<LabelledCheckbox /> should render with byline of null 1`] = `
               className="mx_Checkbox_checkmark"
             />
           </div>
-          <div />
         </label>
       </span>
     </StyledCheckbox>
diff --git a/test/components/views/settings/devices/SelectableDeviceTile-test.tsx b/test/components/views/settings/devices/SelectableDeviceTile-test.tsx
new file mode 100644
index 0000000000..77dad3e138
--- /dev/null
+++ b/test/components/views/settings/devices/SelectableDeviceTile-test.tsx
@@ -0,0 +1,85 @@
+/*
+Copyright 2022 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import { fireEvent, render } from '@testing-library/react';
+import React from 'react';
+import { act } from 'react-dom/test-utils';
+
+import SelectableDeviceTile from '../../../../../src/components/views/settings/devices/SelectableDeviceTile';
+
+describe('<SelectableDeviceTile />', () => {
+    const device = {
+        display_name: 'My Device',
+        device_id: 'my-device',
+        last_seen_ip: '123.456.789',
+    };
+    const defaultProps = {
+        onClick: jest.fn(),
+        device,
+        children: <div>test</div>,
+        isSelected: false,
+    };
+    const getComponent = (props = {}) =>
+        (<SelectableDeviceTile {...defaultProps} {...props} />);
+
+    it('renders unselected device tile with checkbox', () => {
+        const { container } = render(getComponent());
+        expect(container).toMatchSnapshot();
+    });
+
+    it('renders selected tile', () => {
+        const { container } = render(getComponent({ isSelected: true }));
+        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 }));
+
+        act(() => {
+            fireEvent.click(container.querySelector(`#device-tile-checkbox-${device.device_id}`));
+        });
+
+        expect(onClick).toHaveBeenCalled();
+    });
+
+    it('calls onClick on device tile info click', () => {
+        const onClick = jest.fn();
+        const { getByText } = render(getComponent({ onClick }));
+
+        act(() => {
+            fireEvent.click(getByText(device.display_name));
+        });
+
+        expect(onClick).toHaveBeenCalled();
+    });
+
+    it('does not call onClick when clicking device tiles actions', () => {
+        const onClick = jest.fn();
+        const onDeviceActionClick = jest.fn();
+        const children = <button onClick={onDeviceActionClick} data-testid='device-action-button'>test</button>;
+        const { getByTestId } = render(getComponent({ onClick, children }));
+
+        act(() => {
+            fireEvent.click(getByTestId('device-action-button'));
+        });
+
+        // action click handler called
+        expect(onDeviceActionClick).toHaveBeenCalled();
+        // main click handler not called
+        expect(onClick).not.toHaveBeenCalled();
+    });
+});
diff --git a/test/components/views/settings/devices/__snapshots__/SelectableDeviceTile-test.tsx.snap b/test/components/views/settings/devices/__snapshots__/SelectableDeviceTile-test.tsx.snap
new file mode 100644
index 0000000000..09b81870a1
--- /dev/null
+++ b/test/components/views/settings/devices/__snapshots__/SelectableDeviceTile-test.tsx.snap
@@ -0,0 +1,71 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`<SelectableDeviceTile /> renders selected tile 1`] = `
+<input
+  checked=""
+  id="device-tile-checkbox-my-device"
+  type="checkbox"
+/>
+`;
+
+exports[`<SelectableDeviceTile /> renders unselected device tile with checkbox 1`] = `
+<div>
+  <div
+    class="mx_SelectableDeviceTile"
+  >
+    <span
+      class="mx_Checkbox mx_SelectableDeviceTile_checkbox mx_Checkbox_hasKind mx_Checkbox_kind_solid"
+    >
+      <input
+        id="device-tile-checkbox-my-device"
+        type="checkbox"
+      />
+      <label
+        for="device-tile-checkbox-my-device"
+      >
+        <div
+          class="mx_Checkbox_background"
+        >
+          <div
+            class="mx_Checkbox_checkmark"
+          />
+        </div>
+      </label>
+    </span>
+    <div
+      class="mx_DeviceTile"
+    >
+      <div
+        class="mx_DeviceTile_info"
+      >
+        <div
+          tabindex="0"
+        >
+          <h4
+            class="mx_Heading_h4"
+          >
+            My Device
+          </h4>
+        </div>
+        <div
+          class="mx_DeviceTile_metadata"
+        >
+           · 
+          <span
+            data-testid="device-metadata-lastSeenIp"
+          >
+            123.456.789
+          </span>
+        </div>
+      </div>
+      <div
+        class="mx_DeviceTile_actions"
+      >
+        <div>
+          test
+        </div>
+      </div>
+    </div>
+  </div>
+</div>
+`;