From 329bc8a89e409e342364ff2bc9f3dc52797b7732 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 9 Sep 2021 13:14:05 +0100 Subject: [PATCH 1/5] Move unsent event badge handling into RoomNotificationState --- src/components/views/rooms/RoomTile.tsx | 49 ++++--------------- .../notifications/RoomNotificationState.ts | 14 +++++- 2 files changed, 23 insertions(+), 40 deletions(-) diff --git a/src/components/views/rooms/RoomTile.tsx b/src/components/views/rooms/RoomTile.tsx index 4d6de10e1f..8ed2ca85e9 100644 --- a/src/components/views/rooms/RoomTile.tsx +++ b/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 { 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 { 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 { 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 { 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 { 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 { />; 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) { - // hardcode the badge to a danger state when there's unsent messages - badge = ( - - ); - } else if (this.notificationState) { - badge = ( - - ); - } + badge = ( + + ); } let messagePreview = null; diff --git a/src/stores/notifications/RoomNotificationState.ts b/src/stores/notifications/RoomNotificationState.ts index 3fadbe7d7a..0a2e801620 100644 --- a/src/stores/notifications/RoomNotificationState.ts +++ b/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.Red; + 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; From bbe714257ee57207b05aa592a025edcf098a030b Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 10 Sep 2021 09:15:54 +0100 Subject: [PATCH 2/5] Show unsent message warning on Space panel button --- .../views/rooms/NotificationBadge.tsx | 39 +++++++++++++++++-- .../views/spaces/SpaceTreeLevel.tsx | 1 + src/i18n/strings/en_EN.json | 1 + src/stores/notifications/NotificationColor.ts | 1 + .../notifications/RoomNotificationState.ts | 2 +- .../notifications/SpaceNotificationState.ts | 8 ++-- 6 files changed, 43 insertions(+), 9 deletions(-) diff --git a/src/components/views/rooms/NotificationBadge.tsx b/src/components/views/rooms/NotificationBadge.tsx index 8329de7391..a70ff93bd2 100644 --- a/src/components/views/rooms/NotificationBadge.tsx +++ b/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 { + showUnsentTooltip?: boolean; /** * If specified will return an AccessibleButton instead of a div. */ @@ -47,6 +51,7 @@ interface IClickableProps extends IProps, React.InputHTMLAttributes { 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 { + 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,23 @@ export default class NotificationBadge extends React.PureComponent + ); + } + return ( - + { symbol } + { tooltip } ); } diff --git a/src/components/views/spaces/SpaceTreeLevel.tsx b/src/components/views/spaces/SpaceTreeLevel.tsx index dda58ae944..f9d901a298 100644 --- a/src/components/views/spaces/SpaceTreeLevel.tsx +++ b/src/components/views/spaces/SpaceTreeLevel.tsx @@ -93,6 +93,7 @@ export const SpaceButton: React.FC = ({ notification={notificationState} aria-label={ariaLabel} tabIndex={tabIndex} + showUnsentTooltip={true} /> ; } diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index deb854868f..ff5eb29f99 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -1587,6 +1587,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", diff --git a/src/stores/notifications/NotificationColor.ts b/src/stores/notifications/NotificationColor.ts index b12f2b7c00..fadd5ac67e 100644 --- a/src/stores/notifications/NotificationColor.ts +++ b/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 } diff --git a/src/stores/notifications/RoomNotificationState.ts b/src/stores/notifications/RoomNotificationState.ts index 0a2e801620..d0479200bd 100644 --- a/src/stores/notifications/RoomNotificationState.ts +++ b/src/stores/notifications/RoomNotificationState.ts @@ -88,7 +88,7 @@ export class RoomNotificationState extends NotificationState implements IDestroy if (getUnsentMessages(this.room).length > 0) { // When there are unsent messages we show a red `!` - this._color = NotificationColor.Red; + this._color = NotificationColor.Unsent; this._symbol = "!"; this._count = 1; // not used, technically } else if (RoomNotifs.getRoomNotifsState(this.room.roomId) === RoomNotifs.MUTE) { diff --git a/src/stores/notifications/SpaceNotificationState.ts b/src/stores/notifications/SpaceNotificationState.ts index f8eb07251b..c414a01fc2 100644 --- a/src/stores/notifications/SpaceNotificationState.ts +++ b/src/stores/notifications/SpaceNotificationState.ts @@ -30,10 +30,6 @@ export class SpaceNotificationState extends NotificationState { super(); } - public get symbol(): string { - return null; // This notification state doesn't support symbols - } - public setRooms(rooms: Room[]) { const oldRooms = this.rooms; const diff = arrayDiff(oldRooms, rooms); @@ -54,7 +50,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() { @@ -79,6 +75,8 @@ export class SpaceNotificationState extends NotificationState { this._color = Math.max(this.color, state.color); } + this._symbol = this._color === NotificationColor.Unsent ? "!" : null; + // finally, publish an update if needed this.emitIfUpdated(snapshot); } From aff9be6120a0714532f6675b1aa7b5afc63cbebf Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 10 Sep 2021 10:58:13 +0100 Subject: [PATCH 3/5] Surface unsent messages on the sublist notification badge too --- src/components/views/rooms/NotificationBadge.tsx | 9 +++++---- src/components/views/rooms/RoomSublist.tsx | 1 + src/stores/notifications/ListNotificationState.ts | 9 +++++---- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/components/views/rooms/NotificationBadge.tsx b/src/components/views/rooms/NotificationBadge.tsx index a70ff93bd2..a97d51fc90 100644 --- a/src/components/views/rooms/NotificationBadge.tsx +++ b/src/components/views/rooms/NotificationBadge.tsx @@ -143,15 +143,16 @@ export default class NotificationBadge extends React.PureComponent - ); + label = _t("Message didn't send. Click for info."); + tooltip = ; } return ( { onClick={this.onBadgeClick} tabIndex={tabIndex} aria-label={ariaLabel} + showUnsentTooltip={true} /> ); diff --git a/src/stores/notifications/ListNotificationState.ts b/src/stores/notifications/ListNotificationState.ts index 6c67dbdd08..4168fe80b6 100644 --- a/src/stores/notifications/ListNotificationState.ts +++ b/src/stores/notifications/ListNotificationState.ts @@ -31,10 +31,6 @@ export class ListNotificationState extends NotificationState { super(); } - public get symbol(): string { - return null; // This notification state doesn't support symbols - } - public setRooms(rooms: Room[]) { // If we're only concerned about the tile count, don't bother setting up listeners. if (this.byTileCount) { @@ -82,6 +78,7 @@ export class ListNotificationState extends NotificationState { private calculateTotalState() { const snapshot = this.snapshot(); + this._symbol = null; if (this.byTileCount) { this._color = NotificationColor.Red; this._count = this.rooms.length; @@ -92,6 +89,10 @@ export class ListNotificationState extends NotificationState { this._count += state.count; this._color = Math.max(this.color, state.color); } + + if (this._color === NotificationColor.Unsent) { + this._symbol = "!"; + } } // finally, publish an update if needed From 9a8a45382744f26ed42e43873464ee4e0805e8b1 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 10 Sep 2021 11:09:46 +0100 Subject: [PATCH 4/5] tidy --- src/stores/notifications/ListNotificationState.ts | 9 ++++----- src/stores/notifications/SpaceNotificationState.ts | 7 ++++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/stores/notifications/ListNotificationState.ts b/src/stores/notifications/ListNotificationState.ts index 4168fe80b6..97ba2bd80b 100644 --- a/src/stores/notifications/ListNotificationState.ts +++ b/src/stores/notifications/ListNotificationState.ts @@ -31,6 +31,10 @@ export class ListNotificationState extends NotificationState { super(); } + public get symbol(): string { + return this._color === NotificationColor.Unsent ? "!" : null; + } + public setRooms(rooms: Room[]) { // If we're only concerned about the tile count, don't bother setting up listeners. if (this.byTileCount) { @@ -78,7 +82,6 @@ export class ListNotificationState extends NotificationState { private calculateTotalState() { const snapshot = this.snapshot(); - this._symbol = null; if (this.byTileCount) { this._color = NotificationColor.Red; this._count = this.rooms.length; @@ -89,10 +92,6 @@ export class ListNotificationState extends NotificationState { this._count += state.count; this._color = Math.max(this.color, state.color); } - - if (this._color === NotificationColor.Unsent) { - this._symbol = "!"; - } } // finally, publish an update if needed diff --git a/src/stores/notifications/SpaceNotificationState.ts b/src/stores/notifications/SpaceNotificationState.ts index c414a01fc2..137b2ca0f2 100644 --- a/src/stores/notifications/SpaceNotificationState.ts +++ b/src/stores/notifications/SpaceNotificationState.ts @@ -30,6 +30,10 @@ export class SpaceNotificationState extends NotificationState { super(); } + public get symbol(): string { + return this._color === NotificationColor.Unsent ? "!" : null; + } + public setRooms(rooms: Room[]) { const oldRooms = this.rooms; const diff = arrayDiff(oldRooms, rooms); @@ -75,10 +79,7 @@ export class SpaceNotificationState extends NotificationState { this._color = Math.max(this.color, state.color); } - this._symbol = this._color === NotificationColor.Unsent ? "!" : null; - // finally, publish an update if needed this.emitIfUpdated(snapshot); } } - From 6fcd930d0b49e565c506f556b12e0f5d5d04c79b Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 10 Sep 2021 11:54:25 +0100 Subject: [PATCH 5/5] fix unrelated issues --- .../views/settings/tabs/room/SecurityRoomSettingsTab.tsx | 2 +- src/components/views/spaces/SpaceSettingsVisibilityTab.tsx | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/components/views/settings/tabs/room/SecurityRoomSettingsTab.tsx b/src/components/views/settings/tabs/room/SecurityRoomSettingsTab.tsx index b467dc7680..d1c5bc8448 100644 --- a/src/components/views/settings/tabs/room/SecurityRoomSettingsTab.tsx +++ b/src/components/views/settings/tabs/room/SecurityRoomSettingsTab.tsx @@ -425,7 +425,7 @@ export default class SecurityRoomSettingsTab extends React.Component state.getJoinRule()); const [guestAccessEnabled, setGuestAccessEnabled] = useLocalEcho( () => space.currentState.getStateEvents(EventType.RoomGuestAccess, "") ?.getContent()?.guest_access === GuestAccess.CanJoin, @@ -64,7 +66,7 @@ const SpaceSettingsVisibilityTab = ({ matrixClient: cli, space, closeSettingsFn const canonicalAliasEv = space.currentState.getStateEvents(EventType.RoomCanonicalAlias, ""); let advancedSection; - if (visibility === SpaceVisibility.Unlisted) { + if (joinRule === JoinRule.Public) { if (showAdvancedSection) { advancedSection = <>