Improve widget buttons behaviour and layout (matrix-org#8734)

* Improve widet buttons behaviour and layout Relates to element-hq/element-web#20506 See PSC-79 Signed-off-by: Michael Weimann <michaelw@matrix.org> * Add AppTile tests
JanBurp · Jun 14, 2022 · 32b100e · 32b100e
1 parent 774ea38
commit 32b100e
Show file tree

Hide file tree

Showing 4 changed files with 94 additions and 45 deletions.
diff --git a/res/css/views/rooms/_AppsDrawer.scss b/res/css/views/rooms/_AppsDrawer.scss
@@ -194,8 +194,8 @@ $MinWidth: 240px;
     align-items: center;
     justify-content: space-between;
     width: 100%;
-    padding-top: 2px;
-    padding-bottom: 8px;
+    padding-top: 3px;
+    padding-bottom: 6px;
 }
 
 .mx_AppTileMenuBarTitle {
@@ -221,39 +221,50 @@ $MinWidth: 240px;
 }
 
 .mx_AppTileMenuBar_iconButton {
-    width: 12px;
-    height: 12px;
-    mask-repeat: no-repeat;
-    mask-position: 0 center;
-    mask-size: auto 12px;
-    background-color: $topleftmenu-color;
-    margin: 0 5px;
+    height: 24px;
+    margin: 0 4px;
+    position: relative;
+    width: 24px;
+
+    &::before {
+        background-color: $muted-fg-color;
+        content: '';
+        height: 24px;
+        mask-position: center;
+        mask-repeat: no-repeat;
+        mask-size: 12px;
+        position: absolute;
+        width: 24px;
+    }
 
-    &.mx_AppTileMenuBar_iconButton_close {
-        mask-size: auto 10px;
-        mask-image: url("$(res)/img/element-icons/maximise-expand.svg");
-        background-color: $accent;
+    &:hover::after {
+        background-color: $panel-actions;
+        border-radius: 50%;
+        content: '';
+        height: 24px;
+        left: 0;
+        position: absolute;
+        top: 0;
+        width: 24px;
     }
 
-    &.mx_AppTileMenuBar_iconButton_maximise {
-        mask-size: auto 10px;
-        mask-image: url("$(res)/img/element-icons/maximise-expand.svg");
+    &.mx_AppTileMenuBar_iconButton_collapse::before {
+        mask-image: url("$(res)/img/element-icons/minimise-collapse.svg");
     }
 
-    &.mx_AppTileMenuBar_iconButton_unpin {
-        mask-image: url("$(res)/img/element-icons/room/pin-upright.svg");
-        background-color: $accent;
+    &.mx_AppTileMenuBar_iconButton_maximise::before {
+        mask-image: url("$(res)/img/element-icons/maximise-expand.svg");
     }
 
-    &.mx_AppTileMenuBar_iconButton_pin {
-        mask-image: url("$(res)/img/element-icons/room/pin-upright.svg");
+    &.mx_AppTileMenuBar_iconButton_minimise::before {
+        mask-image: url("$(res)/img/element-icons/minus-button.svg");
     }
 
-    &.mx_AppTileMenuBar_iconButton_popout {
+    &.mx_AppTileMenuBar_iconButton_popout::before {
         mask-image: url('$(res)/img/feather-customised/widget/external-link.svg');
     }
 
-    &.mx_AppTileMenuBar_iconButton_menu {
+    &.mx_AppTileMenuBar_iconButton_menu::before {
         mask-image: url('$(res)/img/element-icons/room/ellipsis.svg');
     }
 }

diff --git a/src/components/views/elements/AppTile.tsx b/src/components/views/elements/AppTile.tsx
@@ -512,18 +512,14 @@ export default class AppTile extends React.Component<IProps, IState> {
         if (!this.props.room) return; // ignore action - it shouldn't even be visible
         const targetContainer =
             WidgetLayoutStore.instance.isInContainer(this.props.room, this.props.app, Container.Center)
-                ? Container.Right
+                ? Container.Top
                 : Container.Center;
         WidgetLayoutStore.instance.moveToContainer(this.props.room, this.props.app, targetContainer);
     };
 
-    private onTogglePinnedClick = (): void => {
+    private onMinimiseClicked = (): void => {
         if (!this.props.room) return; // ignore action - it shouldn't even be visible
-        const targetContainer =
-            WidgetLayoutStore.instance.isInContainer(this.props.room, this.props.app, Container.Top)
-                ? Container.Right
-                : Container.Top;
-        WidgetLayoutStore.instance.moveToContainer(this.props.room, this.props.app, targetContainer);
+        WidgetLayoutStore.instance.moveToContainer(this.props.room, this.props.app, Container.Right);
     };
 
     private onContextMenuClick = (): void => {
@@ -668,32 +664,23 @@ export default class AppTile extends React.Component<IProps, IState> {
                 isInContainer(this.props.room, this.props.app, Container.Center);
             const maximisedClasses = classNames({
                 "mx_AppTileMenuBar_iconButton": true,
-                "mx_AppTileMenuBar_iconButton_close": isMaximised,
+                "mx_AppTileMenuBar_iconButton_collapse": isMaximised,
                 "mx_AppTileMenuBar_iconButton_maximise": !isMaximised,
             });
             layoutButtons.push(<AccessibleButton
                 key="toggleMaximised"
                 className={maximisedClasses}
                 title={
-                    isMaximised ? _t("Close") : _t("Maximise")
+                    isMaximised ? _t("Un-maximise") : _t("Maximise")
                 }
                 onClick={this.onToggleMaximisedClick}
             />);
 
-            const isPinned = WidgetLayoutStore.instance.
-                isInContainer(this.props.room, this.props.app, Container.Top);
-            const pinnedClasses = classNames({
-                "mx_AppTileMenuBar_iconButton": true,
-                "mx_AppTileMenuBar_iconButton_unpin": isPinned,
-                "mx_AppTileMenuBar_iconButton_pin": !isPinned,
-            });
             layoutButtons.push(<AccessibleButton
-                key="togglePinned"
-                className={pinnedClasses}
-                title={
-                    isPinned ? _t("Unpin") : _t("Pin")
-                }
-                onClick={this.onTogglePinnedClick}
+                key="minimise"
+                className="mx_AppTileMenuBar_iconButton mx_AppTileMenuBar_iconButton_minimise"
+                title={_t("Minimise")}
+                onClick={this.onMinimiseClicked}
             />);
         }
 

diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json
@@ -2251,6 +2251,8 @@
     "Loading...": "Loading...",
     "Error loading Widget": "Error loading Widget",
     "Error - Mixed content": "Error - Mixed content",
+    "Un-maximise": "Un-maximise",
+    "Minimise": "Minimise",
     "Popout widget": "Popout widget",
     "Copy": "Copy",
     "Share entire screen": "Share entire screen",

diff --git a/test/components/views/elements/AppTile-test.tsx b/test/components/views/elements/AppTile-test.tsx
@@ -19,6 +19,8 @@ import TestRenderer from "react-test-renderer";
 import { jest } from "@jest/globals";
 import { Room } from "matrix-js-sdk/src/models/room";
 import { MatrixWidgetType } from "matrix-widget-api";
+import { mount, ReactWrapper } from "enzyme";
+import { Optional } from "matrix-events-sdk";
 
 import RightPanel from "../../../../src/components/structures/RightPanel";
 import { MatrixClientPeg } from "../../../../src/MatrixClientPeg";
@@ -307,4 +309,51 @@ describe("AppTile", () => {
         await RightPanelStore.instance.onNotReady();
         jest.restoreAllMocks();
     });
+
+    describe("for a pinned widget", () => {
+        let wrapper: ReactWrapper;
+        let moveToContainerSpy;
+
+        beforeEach(() => {
+            wrapper = mount((
+                <MatrixClientContext.Provider value={cli}>
+                    <AppTile
+                        key={app1.id}
+                        app={app1}
+                        room={r1}
+                    />
+                </MatrixClientContext.Provider>
+            ));
+
+            moveToContainerSpy = jest.spyOn(WidgetLayoutStore.instance, 'moveToContainer');
+        });
+
+        it("clicking 'minimise' should send the widget to the right", () => {
+            const minimiseButton = wrapper.find('.mx_AppTileMenuBar_iconButton_minimise');
+            minimiseButton.first().simulate('click');
+            expect(moveToContainerSpy).toHaveBeenCalledWith(r1, app1, Container.Right);
+        });
+
+        it("clicking 'maximise' should send the widget to the center", () => {
+            const minimiseButton = wrapper.find('.mx_AppTileMenuBar_iconButton_maximise');
+            minimiseButton.first().simulate('click');
+            expect(moveToContainerSpy).toHaveBeenCalledWith(r1, app1, Container.Center);
+        });
+
+        describe("for a maximised (centered) widget", () => {
+            beforeEach(() => {
+                jest.spyOn(WidgetLayoutStore.instance, 'isInContainer').mockImplementation(
+                    (room: Optional<Room>, widget: IApp, container: Container) => {
+                        return room === r1 && widget === app1 && container === Container.Center;
+                    },
+                );
+            });
+
+            it("clicking 'un-maximise' should send the widget to the top", () => {
+                const unMaximiseButton = wrapper.find('.mx_AppTileMenuBar_iconButton_collapse');
+                unMaximiseButton.first().simulate('click');
+                expect(moveToContainerSpy).toHaveBeenCalledWith(r1, app1, Container.Top);
+            });
+        });
+    });
 });