Iterate on forward dialog design feedback

Signed-off-by: Robin Townsend <robin@robin.town>

res/css/views/dialogs/_ForwardDialog.scss

@@ -23,7 +23,15 @@ limitations under the License.
     min-height: 0;
     height: 80vh;
 
-    .mx_ForwardDialog_preview {
+    > h3 {
+        margin: 0 0 6px;
+        color: $secondary-fg-color;
+        font-size: $font-12px;
+        font-weight: $font-semi-bold;
+        line-height: $font-15px;
+    }
+
+    > .mx_ForwardDialog_preview {
         max-height: 30%;
         flex-shrink: 0;
         overflow: scroll;
@@ -43,7 +51,14 @@ limitations under the License.
         }
     }
 
-    .mx_ForwardList {
+    > hr {
+        width: 100%;
+        border: none;
+        border-top: 1px solid $input-border-color;
+        margin: 12px 0;
+    }
+
+    > .mx_ForwardList {
         display: contents;
 
         .mx_SearchBox {
@@ -54,8 +69,6 @@ limitations under the License.
 
         .mx_ForwardList_content {
             flex-grow: 1;
-            // Push the scrollbar into the gutter
-            margin-right: -6px;
         }
 
         .mx_ForwardList_noResults {
@@ -64,30 +77,24 @@ limitations under the License.
         }
 
         .mx_ForwardList_results {
-            margin-right: 6px;
-
             &:not(:first-child) {
                 margin-top: 24px;
             }
 
-            > h3 {
-                margin: 0;
-                color: $secondary-fg-color;
-                font-size: $font-12px;
-                font-weight: $font-semi-bold;
-                line-height: $font-15px;
-            }
-
             .mx_ForwardList_entry {
                 display: flex;
                 justify-content: space-between;
-                margin-top: 12px;
                 height: 32px;
+                padding: 6px;
+                border-radius: 8px;
+
+                &:hover {
+                    background-color: $groupFilterPanel-bg-color;
+                }
 
                 .mx_ForwardList_roomButton {
                     display: flex;
                     margin-right: 12px;
-                    flex-grow: 1;
                     min-width: 0;
 
                     .mx_DecoratedRoomAvatar {
@@ -105,26 +112,39 @@ limitations under the License.
                 }
 
                 .mx_ForwardList_sendButton {
-                    &.mx_ForwardList_sending, &.mx_ForwardList_sent {
-                        &::before {
-                            content: '';
-                            display: inline-block;
+                    position: relative;
+
+                    &:not(.mx_ForwardList_canSend) .mx_ForwardList_sendLabel {
+                        // Hide the "Send" label while preserving button size
+                        visibility: hidden;
+                    }
+
+                    .mx_ForwardList_sendIcon, .mx_NotificationBadge {
+                        position: absolute;
+                    }
+
+                    .mx_NotificationBadge {
+                        opacity: 0.4;
+                    }
+
+                    &.mx_ForwardList_sending .mx_ForwardList_sendIcon {
                         background-color: $button-primary-bg-color;
+                        mask-image: url('$(res)/img/element-icons/circle-sending.svg');
                         mask-position: center;
                         mask-repeat: no-repeat;
                         mask-size: 14px;
                         width: 14px;
                         height: 14px;
-                            margin-right: 5px;
-                        }
                     }
 
-                    &.mx_ForwardList_sending::before {
-                        mask-image: url('$(res)/img/element-icons/circle-sending.svg');
-                    }
-
-                    &.mx_ForwardList_sent::before {
+                    &.mx_ForwardList_sent .mx_ForwardList_sendIcon {
+                        background-color: $button-primary-bg-color;
                         mask-image: url('$(res)/img/element-icons/circle-sent.svg');
+                        mask-position: center;
+                        mask-repeat: no-repeat;
+                        mask-size: 14px;
+                        width: 14px;
+                        height: 14px;
                     }
                 }
             }

src/components/views/dialogs/ForwardDialog.tsx

@@ -31,9 +31,11 @@ import {avatarUrlForUser} from "../../../Avatar";
 import EventTile from "../rooms/EventTile";
 import SearchBox from "../../structures/SearchBox";
 import DecoratedRoomAvatar from "../avatars/DecoratedRoomAvatar";
-import AccessibleButton from "../elements/AccessibleButton";
+import {Alignment} from '../elements/Tooltip';
 import AccessibleTooltipButton from "../elements/AccessibleTooltipButton";
 import AutoHideScrollbar from "../../structures/AutoHideScrollbar";
+import {StaticNotificationState} from "../../../stores/notifications/StaticNotificationState";
+import NotificationBadge from "../rooms/NotificationBadge";
 import {RoomPermalinkCreator} from "../../../utils/permalinks/Permalinks";
 import {sortRooms} from "../../../stores/room-list/algorithms/tag-sorting/RecentAlgorithm";
 
@@ -82,52 +84,60 @@ const Entry: React.FC<IEntryProps> = ({ room, event, cli, onFinished }) => {
         }
     };
 
-    let button;
-    if (room.maySendMessage()) {
-        let label;
     let className;
+    let disabled = false;
+    let title;
+    let icon;
     if (sendState === SendState.CanSend) {
-            label = _t("Send");
         className = "mx_ForwardList_canSend";
-        } else if (sendState === SendState.Sending) {
-            label = _t("Sending…");
-            className = "mx_ForwardList_sending";
-        } else if (sendState === SendState.Sent) {
-            label = _t("Sent");
-            className = "mx_ForwardList_sent";
+        if (room.maySendMessage()) {
+            title = _t("Send");
         } else {
-            label = _t("Failed to send");
-            className = "mx_ForwardList_sendFailed";
+            disabled = true;
+            title = _t("You do not have permission to do this");
         }
-
-        button =
-            <AccessibleButton
-                kind={sendState === SendState.Failed ? "danger_outline" : "primary_outline"}
-                className={`mx_ForwardList_sendButton ${className}`}
-                onClick={send}
-                disabled={sendState !== SendState.CanSend}
-            >
-                { label }
-            </AccessibleButton>;
+    } else if (sendState === SendState.Sending) {
+        className = "mx_ForwardList_sending";
+        disabled = true;
+        title = _t("Sending…");
+        icon = <div className="mx_ForwardList_sendIcon"></div>;
+    } else if (sendState === SendState.Sent) {
+        className = "mx_ForwardList_sent";
+        disabled = true;
+        title = _t("Sent");
+        icon = <div className="mx_ForwardList_sendIcon"></div>;
     } else {
-        button =
-            <AccessibleTooltipButton
-                kind="primary_outline"
-                className="mx_ForwardList_sendButton mx_ForwardList_canSend"
-                onClick={() => {}}
-                disabled={true}
-                title={_t("You do not have permission to post to this room")}
-            >
-                { _t("Send") }
-            </AccessibleTooltipButton>;
+        className = "mx_ForwardList_sendFailed";
+        disabled = true;
+        title = _t("Failed to send");
+        icon = <NotificationBadge
+            notification={StaticNotificationState.RED_EXCLAMATION}
+        />;
     }
 
     return <div className="mx_ForwardList_entry">
-        <AccessibleButton className="mx_ForwardList_roomButton" onClick={jumpToRoom}>
+        <AccessibleTooltipButton
+            className="mx_ForwardList_roomButton"
+            onClick={jumpToRoom}
+            title={_t("Open link")}
+            yOffset={-20}
+            alignment={Alignment.Top}
+        >
             <DecoratedRoomAvatar room={room} avatarSize={32} />
             <span className="mx_ForwardList_entry_name">{ room.name }</span>
-        </AccessibleButton>
-        { button }
+        </AccessibleTooltipButton>
+        <AccessibleTooltipButton
+            kind={sendState === SendState.Failed ? "danger_outline" : "primary_outline"}
+            className={`mx_ForwardList_sendButton ${className}`}
+            onClick={send}
+            disabled={disabled}
+            title={title}
+            yOffset={-20}
+            alignment={Alignment.Top}
+        >
+            <div className="mx_ForwardList_sendLabel">{ _t("Send") }</div>
+            { icon }
+        </AccessibleTooltipButton>
     </div>;
 };
 
@@ -180,6 +190,7 @@ const ForwardDialog: React.FC<IProps> = ({ cli, event, permalinkCreator, onFinis
         onFinished={onFinished}
         fixedWidth={false}
     >
+        <h3>{ _t("Message preview") }</h3>
         <div className={classnames("mx_ForwardDialog_preview", {
             "mx_IRCLayout": previewLayout == Layout.IRC,
             "mx_GroupLayout": previewLayout == Layout.Group,
@@ -191,11 +202,11 @@ const ForwardDialog: React.FC<IProps> = ({ cli, event, permalinkCreator, onFinis
                 permalinkCreator={permalinkCreator}
             />
         </div>
+        <hr />
         <div className="mx_ForwardList">
-            <h2>{ _t("Forward to") }</h2>
             <SearchBox
                 className="mx_textinput_icon mx_textinput_search"
-                placeholder={ _t("Filter your rooms and DMs") }
+                placeholder={ _t("Search for rooms or people") }
                 onSearch={setQuery}
                 autoComplete={true}
                 autoFocus={true}

src/components/views/elements/AccessibleTooltipButton.tsx

@@ -19,7 +19,7 @@ import React from 'react';
 import classNames from 'classnames';
 
 import AccessibleButton from "./AccessibleButton";
-import Tooltip from './Tooltip';
+import Tooltip, {Alignment} from './Tooltip';
 import {replaceableComponent} from "../../../utils/replaceableComponent";
 
 interface ITooltipProps extends React.ComponentProps<typeof AccessibleButton> {
@@ -28,6 +28,7 @@ interface ITooltipProps extends React.ComponentProps<typeof AccessibleButton> {
     tooltipClassName?: string;
     forceHide?: boolean;
     yOffset?: number;
+    alignment?: Alignment;
 }
 
 interface IState {
@@ -66,13 +67,14 @@ export default class AccessibleTooltipButton extends React.PureComponent<IToolti
 
     render() {
         // eslint-disable-next-line @typescript-eslint/no-unused-vars
-        const {title, tooltip, children, tooltipClassName, forceHide, yOffset, ...props} = this.props;
+        const {title, tooltip, children, tooltipClassName, forceHide, yOffset, alignment, ...props} = this.props;
 
         const tip = this.state.hover ? <Tooltip
             className="mx_AccessibleTooltipButton_container"
             tooltipClassName={classNames("mx_AccessibleTooltipButton_tooltip", tooltipClassName)}
             label={tooltip || title}
             yOffset={yOffset}
+            alignment={alignment}
         /> : <div />;
         return (
             <AccessibleButton

src/i18n/strings/en_EN.json

@@ -2202,11 +2202,13 @@
     "PRO TIP: If you start a bug, please submit <debugLogsLink>debug logs</debugLogsLink> to help us track down the problem.": "PRO TIP: If you start a bug, please submit <debugLogsLink>debug logs</debugLogsLink> to help us track down the problem.",
     "Report a bug": "Report a bug",
     "Please view <existingIssuesLink>existing bugs on Github</existingIssuesLink> first. No match? <newIssueLink>Start a new one</newIssueLink>.": "Please view <existingIssuesLink>existing bugs on Github</existingIssuesLink> first. No match? <newIssueLink>Start a new one</newIssueLink>.",
+    "You do not have permission to do this": "You do not have permission to do this",
     "Sending…": "Sending…",
     "Sent": "Sent",
+    "Open link": "Open link",
     "Forward message": "Forward message",
-    "Forward to": "Forward to",
-    "Filter your rooms and DMs": "Filter your rooms and DMs",
+    "Message preview": "Message preview",
+    "Search for rooms or people": "Search for rooms or people",
     "Confirm abort of host creation": "Confirm abort of host creation",
     "Are you sure you wish to abort creation of the host? The process cannot be continued.": "Are you sure you wish to abort creation of the host? The process cannot be continued.",
     "Abort": "Abort",

test/components/views/dialogs/ForwardDialog-test.js

@@ -101,30 +101,30 @@ describe("ForwardDialog", () => {
         }));
 
         const firstButton = wrapper.find("AccessibleButton.mx_ForwardList_sendButton").first();
-        expect(firstButton.text()).toBe("Send");
+        expect(firstButton.render().is(".mx_ForwardList_canSend")).toBe(true);
 
         act(() => { firstButton.simulate("click"); });
-        expect(firstButton.text()).toBe("Sending…");
+        expect(firstButton.render().is(".mx_ForwardList_sending")).toBe(true);
 
         await act(async () => {
             cancelSend();
             // Wait one tick for the button to realize the send failed
             await new Promise(resolve => setImmediate(resolve));
         });
-        expect(firstButton.text()).toBe("Failed to send");
+        expect(firstButton.render().is(".mx_ForwardList_sendFailed")).toBe(true);
 
         const secondButton = wrapper.find("AccessibleButton.mx_ForwardList_sendButton").at(1);
-        expect(secondButton.render().text()).toBe("Send");
+        expect(secondButton.render().is(".mx_ForwardList_canSend")).toBe(true);
 
         act(() => { secondButton.simulate("click"); });
-        expect(secondButton.text()).toBe("Sending…");
+        expect(secondButton.render().is(".mx_ForwardList_sending")).toBe(true);
 
         await act(async () => {
             finishSend();
             // Wait one tick for the button to realize the send succeeded
             await new Promise(resolve => setImmediate(resolve));
         });
-        expect(secondButton.text()).toBe("Sent");
+        expect(secondButton.render().is(".mx_ForwardList_sent")).toBe(true);
     });
 
     it("can render replies", async () => {