Merge pull request #6778 from matrix-org/t3chguy/fix/18891

src/components/views/rooms/NotificationBadge.tsx

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import React from "react";
+import React, { MouseEvent } from "react";
 import classNames from "classnames";
 import { formatCount } from "../../../utils/FormattingUtils";
 import SettingsStore from "../../../settings/SettingsStore";
@@ -22,6 +22,9 @@ import AccessibleButton from "../elements/AccessibleButton";
 import { XOR } from "../../../@types/common";
 import { NOTIFICATION_STATE_UPDATE, NotificationState } from "../../../stores/notifications/NotificationState";
 import { replaceableComponent } from "../../../utils/replaceableComponent";
+import Tooltip from "../elements/Tooltip";
+import { _t } from "../../../languageHandler";
+import { NotificationColor } from "../../../stores/notifications/NotificationColor";
 
 interface IProps {
     notification: NotificationState;
@@ -39,6 +42,7 @@ interface IProps {
 }
 
 interface IClickableProps extends IProps, React.InputHTMLAttributes<Element> {
+    showUnsentTooltip?: boolean;
     /**
      * If specified will return an AccessibleButton instead of a div.
      */
@@ -47,6 +51,7 @@ interface IClickableProps extends IProps, React.InputHTMLAttributes<Element> {
 
 interface IState {
     showCounts: boolean; // whether or not to show counts. Independent of props.forceCount
+    showTooltip: boolean;
 }
 
 @replaceableComponent("views.rooms.NotificationBadge")
@@ -59,6 +64,7 @@ export default class NotificationBadge extends React.PureComponent<XOR<IProps, I
 
         this.state = {
             showCounts: SettingsStore.getValue("Notifications.alwaysShowBadgeCounts", this.roomId),
+            showTooltip: false,
         };
 
         this.countWatcherRef = SettingsStore.watchSetting(
@@ -93,9 +99,22 @@ export default class NotificationBadge extends React.PureComponent<XOR<IProps, I
         this.forceUpdate(); // notification state changed - update
     };
 
+    private onMouseOver = (e: MouseEvent) => {
+        e.stopPropagation();
+        this.setState({
+            showTooltip: true,
+        });
+    };
+
+    private onMouseLeave = () => {
+        this.setState({
+            showTooltip: false,
+        });
+    };
+
     public render(): React.ReactElement {
         /* eslint @typescript-eslint/no-unused-vars: ["error", { "ignoreRestSiblings": true }] */
-        const { notification, forceCount, roomId, onClick, ...props } = this.props;
+        const { notification, showUnsentTooltip, forceCount, roomId, onClick, ...props } = this.props;
 
         // Don't show a badge if we don't need to
         if (notification.isIdle) return null;
@@ -124,9 +143,24 @@ export default class NotificationBadge extends React.PureComponent<XOR<IProps, I
         });
 
         if (onClick) {
+            let label: string;
+            let tooltip: JSX.Element;
+            if (showUnsentTooltip && this.state.showTooltip && notification.color === NotificationColor.Unsent) {
+                label = _t("Message didn't send. Click for info.");
+                tooltip = <Tooltip className="mx_RoleButton_tooltip" label={label} />;
+            }
+
             return (
-                <AccessibleButton {...props} className={classes} onClick={onClick}>
+                <AccessibleButton
+                    aria-label={label}
+                    {...props}
+                    className={classes}
+                    onClick={onClick}
+                    onMouseOver={this.onMouseOver}
+                    onMouseLeave={this.onMouseLeave}
+                >
                     <span className="mx_NotificationBadge_count">{ symbol }</span>
+                    { tooltip }
                 </AccessibleButton>
             );
         }

src/components/views/rooms/RoomSublist.tsx

@@ -670,6 +670,7 @@ export default class RoomSublist extends React.Component<IProps, IState> {
                             onClick={this.onBadgeClick}
                             tabIndex={tabIndex}
                             aria-label={ariaLabel}
+                            showUnsentTooltip={true}
                         />
                     );
 

src/components/views/rooms/RoomTile.tsx

@@ -17,7 +17,6 @@ limitations under the License.
 
 import React, { createRef } from "react";
 import { Room } from "matrix-js-sdk/src/models/room";
-import { MatrixEvent } from "matrix-js-sdk/src/models/event";
 import classNames from "classnames";
 import { RovingTabIndexWrapper } from "../../../accessibility/RovingTabIndex";
 import AccessibleButton, { ButtonEvent } from "../../views/elements/AccessibleButton";
@@ -51,8 +50,6 @@ import IconizedContextMenu, {
 } from "../context_menus/IconizedContextMenu";
 import { CommunityPrototypeStore, IRoomProfile } from "../../../stores/CommunityPrototypeStore";
 import { replaceableComponent } from "../../../utils/replaceableComponent";
-import { getUnsentMessages } from "../../structures/RoomStatusBar";
-import { StaticNotificationState } from "../../../stores/notifications/StaticNotificationState";
 
 interface IProps {
     room: Room;
@@ -68,7 +65,6 @@ interface IState {
     notificationsMenuPosition: PartialDOMRect;
     generalMenuPosition: PartialDOMRect;
     messagePreview?: string;
-    hasUnsentEvents: boolean;
 }
 
 const messagePreviewId = (roomId: string) => `mx_RoomTile_messagePreview_${roomId}`;
@@ -95,7 +91,6 @@ export default class RoomTile extends React.PureComponent<IProps, IState> {
             selected: ActiveRoomObserver.activeRoomId === this.props.room.roomId,
             notificationsMenuPosition: null,
             generalMenuPosition: null,
-            hasUnsentEvents: this.countUnsentEvents() > 0,
 
             // generatePreview() will return nothing if the user has previews disabled
             messagePreview: "",
@@ -106,10 +101,6 @@ export default class RoomTile extends React.PureComponent<IProps, IState> {
         this.roomProps = EchoChamber.forRoom(this.props.room);
     }
 
-    private countUnsentEvents(): number {
-        return getUnsentMessages(this.props.room).length;
-    }
-
     private onRoomNameUpdate = (room) => {
         this.forceUpdate();
     };
@@ -118,11 +109,6 @@ export default class RoomTile extends React.PureComponent<IProps, IState> {
         this.forceUpdate(); // notification state changed - update
     };
 
-    private onLocalEchoUpdated = (ev: MatrixEvent, room: Room) => {
-        if (room?.roomId !== this.props.room.roomId) return;
-        this.setState({ hasUnsentEvents: this.countUnsentEvents() > 0 });
-    };
-
     private onRoomPropertyUpdate = (property: CachedRoomKey) => {
         if (property === CachedRoomKey.NotificationVolume) this.onNotificationUpdate();
         // else ignore - not important for this tile
@@ -183,7 +169,6 @@ export default class RoomTile extends React.PureComponent<IProps, IState> {
             CommunityPrototypeStore.getUpdateEventName(this.props.room.roomId),
             this.onCommunityUpdate,
         );
-        MatrixClientPeg.get().on("Room.localEchoUpdated", this.onLocalEchoUpdated);
     }
 
     public componentWillUnmount() {
@@ -208,7 +193,6 @@ export default class RoomTile extends React.PureComponent<IProps, IState> {
             CommunityPrototypeStore.getUpdateEventName(this.props.room.roomId),
             this.onCommunityUpdate,
         );
-        MatrixClientPeg.get()?.removeListener("Room.localEchoUpdated", this.onLocalEchoUpdated);
     }
 
     private onAction = (payload: ActionPayload) => {
@@ -587,30 +571,17 @@ export default class RoomTile extends React.PureComponent<IProps, IState> {
         />;
 
         let badge: React.ReactNode;
-        if (!this.props.isMinimized) {
+        if (!this.props.isMinimized && this.notificationState) {
             // aria-hidden because we summarise the unread count/highlight status in a manual aria-label below
-            if (this.state.hasUnsentEvents) {
+            badge = (
-                // hardcode the badge to a danger state when there's unsent messages
+                <div className="mx_RoomTile_badgeContainer" aria-hidden="true">
-                badge = (
+                    <NotificationBadge
-                    <div className="mx_RoomTile_badgeContainer" aria-hidden="true">
+                        notification={this.notificationState}
-                        <NotificationBadge
+                        forceCount={false}
-                            notification={StaticNotificationState.RED_EXCLAMATION}
+                        roomId={this.props.room.roomId}
-                            forceCount={false}
+                    />
-                            roomId={this.props.room.roomId}
+                </div>
-                        />
+            );
-                    </div>
-                );
-            } else if (this.notificationState) {
-                badge = (
-                    <div className="mx_RoomTile_badgeContainer" aria-hidden="true">
-                        <NotificationBadge
-                            notification={this.notificationState}
-                            forceCount={false}
-                            roomId={this.props.room.roomId}
-                        />
-                    </div>
-                );
-            }
         }
 
         let messagePreview = null;

src/components/views/spaces/SpaceTreeLevel.tsx

@@ -93,6 +93,7 @@ export const SpaceButton: React.FC<IButtonProps> = ({
                 notification={notificationState}
                 aria-label={ariaLabel}
                 tabIndex={tabIndex}
+                showUnsentTooltip={true}
             />
         </div>;
     }

src/i18n/strings/en_EN.json

@@ -1598,6 +1598,7 @@
     "Your private messages are normally encrypted, but this room isn't. Usually this is due to an unsupported device or method being used, like email invites.": "Your private messages are normally encrypted, but this room isn't. Usually this is due to an unsupported device or method being used, like email invites.",
     "Enable encryption in settings.": "Enable encryption in settings.",
     "End-to-end encryption isn't enabled": "End-to-end encryption isn't enabled",
+    "Message didn't send. Click for info.": "Message didn't send. Click for info.",
     "Unpin": "Unpin",
     "View message": "View message",
     "%(duration)ss": "%(duration)ss",

src/stores/notifications/ListNotificationState.ts

@@ -32,7 +32,7 @@ export class ListNotificationState extends NotificationState {
     }
 
     public get symbol(): string {
-        return null; // This notification state doesn't support symbols
+        return this._color === NotificationColor.Unsent ? "!" : null;
     }
 
     public setRooms(rooms: Room[]) {

src/stores/notifications/NotificationColor.ts

@@ -21,4 +21,5 @@ export enum NotificationColor {
     Bold, // no badge, show as unread
     Grey, // unread notified messages
     Red,  // unread pings
+    Unsent, // some messages failed to send
 }

src/stores/notifications/RoomNotificationState.ts

@@ -24,6 +24,7 @@ import { Room } from "matrix-js-sdk/src/models/room";
 import * as RoomNotifs from '../../RoomNotifs';
 import * as Unread from '../../Unread';
 import { NotificationState } from "./NotificationState";
+import { getUnsentMessages } from "../../components/structures/RoomStatusBar";
 
 export class RoomNotificationState extends NotificationState implements IDestroyable {
     constructor(public readonly room: Room) {
@@ -32,6 +33,7 @@ export class RoomNotificationState extends NotificationState implements IDestroy
         this.room.on("Room.timeline", this.handleRoomEventUpdate);
         this.room.on("Room.redaction", this.handleRoomEventUpdate);
         this.room.on("Room.myMembership", this.handleMembershipUpdate);
+        this.room.on("Room.localEchoUpdated", this.handleLocalEchoUpdated);
         MatrixClientPeg.get().on("Event.decrypted", this.handleRoomEventUpdate);
         MatrixClientPeg.get().on("accountData", this.handleAccountDataUpdate);
         this.updateNotificationState();
@@ -47,12 +49,17 @@ export class RoomNotificationState extends NotificationState implements IDestroy
         this.room.removeListener("Room.timeline", this.handleRoomEventUpdate);
         this.room.removeListener("Room.redaction", this.handleRoomEventUpdate);
         this.room.removeListener("Room.myMembership", this.handleMembershipUpdate);
+        this.room.removeListener("Room.localEchoUpdated", this.handleLocalEchoUpdated);
         if (MatrixClientPeg.get()) {
             MatrixClientPeg.get().removeListener("Event.decrypted", this.handleRoomEventUpdate);
             MatrixClientPeg.get().removeListener("accountData", this.handleAccountDataUpdate);
         }
     }
 
+    private handleLocalEchoUpdated = () => {
+        this.updateNotificationState();
+    };
+
     private handleReadReceipt = (event: MatrixEvent, room: Room) => {
         if (!readReceiptChangeIsFor(event, MatrixClientPeg.get())) return; // not our own - ignore
         if (room.roomId !== this.room.roomId) return; // not for us - ignore
@@ -79,7 +86,12 @@ export class RoomNotificationState extends NotificationState implements IDestroy
     private updateNotificationState() {
         const snapshot = this.snapshot();
 
-        if (RoomNotifs.getRoomNotifsState(this.room.roomId) === RoomNotifs.MUTE) {
+        if (getUnsentMessages(this.room).length > 0) {
+            // When there are unsent messages we show a red `!`
+            this._color = NotificationColor.Unsent;
+            this._symbol = "!";
+            this._count = 1; // not used, technically
+        } else if (RoomNotifs.getRoomNotifsState(this.room.roomId) === RoomNotifs.MUTE) {
             // When muted we suppress all notification states, even if we have context on them.
             this._color = NotificationColor.None;
             this._symbol = null;

src/stores/notifications/SpaceNotificationState.ts

@@ -31,7 +31,7 @@ export class SpaceNotificationState extends NotificationState {
     }
 
     public get symbol(): string {
-        return null; // This notification state doesn't support symbols
+        return this._color === NotificationColor.Unsent ? "!" : null;
     }
 
     public setRooms(rooms: Room[]) {
@@ -54,7 +54,7 @@ export class SpaceNotificationState extends NotificationState {
     }
 
     public getFirstRoomWithNotifications() {
-        return this.rooms.find((room) => room.getUnreadNotificationCount() > 0).roomId;
+        return Object.values(this.states).find(state => state.color >= this.color)?.room.roomId;
     }
 
     public destroy() {
@@ -83,4 +83,3 @@ export class SpaceNotificationState extends NotificationState {
         this.emitIfUpdated(snapshot);
     }
 }
-