Merge pull request #4805 from matrix-org/travis/room-list/unread-2

Improve unread/badge states in new room list (mk II)

res/css/views/rooms/_RoomTile2.scss

@@ -67,7 +67,7 @@ limitations under the License.
         }
 
         .mx_RoomTile2_name.mx_RoomTile2_nameHasUnreadEvents {
-            font-weight: 600;
+            font-weight: 700;
         }
 
         .mx_RoomTile2_messagePreview {

src/components/views/rooms/NotificationBadge.tsx

@@ -18,27 +18,23 @@ import React from "react";
 import classNames from "classnames";
 import { formatMinimalBadgeCount } from "../../../utils/FormattingUtils";
 import { Room } from "matrix-js-sdk/src/models/room";
-import { RovingTabIndexWrapper } from "../../../accessibility/RovingTabIndex";
-import AccessibleButton from "../../views/elements/AccessibleButton";
-import RoomAvatar from "../../views/avatars/RoomAvatar";
-import dis from '../../../dispatcher/dispatcher';
-import { Key } from "../../../Keyboard";
 import * as RoomNotifs from '../../../RoomNotifs';
 import { EffectiveMembership, getEffectiveMembership } from "../../../stores/room-list/membership";
 import * as Unread from '../../../Unread';
 import { MatrixClientPeg } from "../../../MatrixClientPeg";
 import { MatrixEvent } from "matrix-js-sdk/src/models/event";
-import ActiveRoomObserver from "../../../ActiveRoomObserver";
 import { EventEmitter } from "events";
 import { arrayDiff } from "../../../utils/arrays";
 import { IDestroyable } from "../../../utils/IDestroyable";
+import SettingsStore from "../../../settings/SettingsStore";
+import { DefaultTagID, TagID } from "../../../stores/room-list/models";
 
 export const NOTIFICATION_STATE_UPDATE = "update";
 
 export enum NotificationColor {
     // Inverted (None -> Red) because we do integer comparisons on this
     None, // nothing special
-    Bold, // no badge, show as unread
+    Bold, // no badge, show as unread // TODO: This goes away with new notification structures
     Grey, // unread notified messages
     Red,  // unread pings
 }
@@ -53,18 +49,45 @@ interface IProps {
     notification: INotificationState;
 
     /**
-     * If true, the badge will conditionally display a badge without count for the user.
+     * If true, the badge will show a count if at all possible. This is typically
+     * used to override the user's preference for things like room sublists.
      */
-    allowNoCount: boolean;
+    forceCount: boolean;
+
+    /**
+     * The room ID, if any, the badge represents.
+     */
+    roomId?: string;
 }
 
 interface IState {
+    showCounts: boolean; // whether or not to show counts. Independent of props.forceCount
 }
 
 export default class NotificationBadge extends React.PureComponent<IProps, IState> {
+    private countWatcherRef: string;
+
     constructor(props: IProps) {
         super(props);
         this.props.notification.on(NOTIFICATION_STATE_UPDATE, this.onNotificationUpdate);
+
+        this.state = {
+            showCounts: SettingsStore.getValue("Notifications.alwaysShowBadgeCounts", this.roomId),
+        };
+
+        this.countWatcherRef = SettingsStore.watchSetting(
+            "Notifications.alwaysShowBadgeCounts", this.roomId,
+            this.countPreferenceChanged,
+        );
+    }
+
+    private get roomId(): string {
+        // We should convert this to null for safety with the SettingsStore
+        return this.props.roomId || null;
+    }
+
+    public componentWillUnmount() {
+        SettingsStore.unwatchSetting(this.countWatcherRef);
     }
 
     public componentDidUpdate(prevProps: Readonly<IProps>) {
@@ -75,24 +98,34 @@ export default class NotificationBadge extends React.PureComponent<IProps, IStat
         this.props.notification.on(NOTIFICATION_STATE_UPDATE, this.onNotificationUpdate);
     }
 
+    private countPreferenceChanged = () => {
+        this.setState({showCounts: SettingsStore.getValue("Notifications.alwaysShowBadgeCounts", this.roomId)});
+    };
+
     private onNotificationUpdate = () => {
         this.forceUpdate(); // notification state changed - update
     };
 
     public render(): React.ReactElement {
         // Don't show a badge if we don't need to
-        if (this.props.notification.color <= NotificationColor.Bold) return null;
+        if (this.props.notification.color <= NotificationColor.None) return null;
 
         const hasNotif = this.props.notification.color >= NotificationColor.Red;
         const hasCount = this.props.notification.color >= NotificationColor.Grey;
-        const isEmptyBadge = this.props.allowNoCount && !localStorage.getItem("mx_rl_rt_badgeCount");
+        const hasUnread = this.props.notification.color >= NotificationColor.Bold;
+        const couldBeEmpty = (!this.state.showCounts || hasUnread) && !hasNotif;
+        let isEmptyBadge = couldBeEmpty && (!this.state.showCounts || !hasCount);
+        if (this.props.forceCount) {
+            isEmptyBadge = false;
+            if (!hasCount) return null; // Can't render a badge
+        }
 
         let symbol = this.props.notification.symbol || formatMinimalBadgeCount(this.props.notification.count);
         if (isEmptyBadge) symbol = "";
 
         const classes = classNames({
             'mx_NotificationBadge': true,
-            'mx_NotificationBadge_visible': hasCount,
+            'mx_NotificationBadge_visible': isEmptyBadge ? true : hasCount,
             'mx_NotificationBadge_highlighted': hasNotif,
             'mx_NotificationBadge_dot': isEmptyBadge,
             'mx_NotificationBadge_2char': symbol.length > 0 && symbol.length < 3,
@@ -107,7 +140,7 @@ export default class NotificationBadge extends React.PureComponent<IProps, IStat
     }
 }
 
-export class RoomNotificationState extends EventEmitter implements IDestroyable {
+export class RoomNotificationState extends EventEmitter implements IDestroyable, INotificationState {
     private _symbol: string;
     private _count: number;
     private _color: NotificationColor;
@@ -205,13 +238,38 @@ export class RoomNotificationState extends EventEmitter implements IDestroyable
     }
 }
 
-export class ListNotificationState extends EventEmitter implements IDestroyable {
+export class TagSpecificNotificationState extends RoomNotificationState {
+    private static TAG_TO_COLOR: {
+        // @ts-ignore - TS wants this to be a string key, but we know better
+        [tagId: TagID]: NotificationColor,
+    } = {
+        [DefaultTagID.DM]: NotificationColor.Red,
+    };
+
+    private readonly colorWhenNotIdle?: NotificationColor;
+
+    constructor(room: Room, tagId: TagID) {
+        super(room);
+
+        const specificColor = TagSpecificNotificationState.TAG_TO_COLOR[tagId];
+        if (specificColor) this.colorWhenNotIdle = specificColor;
+    }
+
+    public get color(): NotificationColor {
+        if (!this.colorWhenNotIdle) return super.color;
+
+        if (super.color !== NotificationColor.None) return this.colorWhenNotIdle;
+        return super.color;
+    }
+}
+
+export class ListNotificationState extends EventEmitter implements IDestroyable, INotificationState {
     private _count: number;
     private _color: NotificationColor;
     private rooms: Room[] = [];
     private states: { [roomId: string]: RoomNotificationState } = {};
 
-    constructor(private byTileCount = false) {
+    constructor(private byTileCount = false, private tagId: TagID) {
         super();
     }
 
@@ -246,7 +304,7 @@ export class ListNotificationState extends EventEmitter implements IDestroyable
             state.destroy();
         }
         for (const newRoom of diff.added) {
-            const state = new RoomNotificationState(newRoom);
+            const state = new TagSpecificNotificationState(newRoom, this.tagId);
             state.on(NOTIFICATION_STATE_UPDATE, this.onRoomNotificationStateUpdate);
             if (this.states[newRoom.roomId]) {
                 // "Should never happen" disclaimer.
@@ -259,6 +317,12 @@ export class ListNotificationState extends EventEmitter implements IDestroyable
         this.calculateTotalState();
     }
 
+    public getForRoom(room: Room) {
+        const state = this.states[room.roomId];
+        if (!state) throw new Error("Unknown room for notification state");
+        return state;
+    }
+
     public destroy() {
         for (const state of Object.values(this.states)) {
             state.destroy();

src/components/views/rooms/RoomList2.tsx

@@ -193,6 +193,7 @@ export default class RoomList2 extends React.Component<IProps, IState> {
             components.push(
                 <RoomSublist2
                     key={`sublist-${orderedTagId}`}
+                    tagId={orderedTagId}
                     forRooms={true}
                     rooms={orderedRooms}
                     startAsHidden={aesthetics.defaultHidden}

src/components/views/rooms/RoomSublist2.tsx

@@ -32,6 +32,7 @@ import StyledCheckbox from "../elements/StyledCheckbox";
 import StyledRadioButton from "../elements/StyledRadioButton";
 import RoomListStore from "../../../stores/room-list/RoomListStore2";
 import { ListAlgorithm, SortAlgorithm } from "../../../stores/room-list/algorithms/models";
+import { TagID } from "../../../stores/room-list/models";
 
 /*******************************************************************
  *   CAUTION                                                       *
@@ -56,6 +57,7 @@ interface IProps {
     isInvite: boolean;
     layout: ListLayout;
     isMinimized: boolean;
+    tagId: TagID;
 
     // TODO: Collapsed state
     // TODO: Group invites
@@ -78,7 +80,7 @@ export default class RoomSublist2 extends React.Component<IProps, IState> {
         super(props);
 
         this.state = {
-            notificationState: new ListNotificationState(this.props.isInvite),
+            notificationState: new ListNotificationState(this.props.isInvite, this.props.tagId),
             menuDisplayed: false,
         };
         this.state.notificationState.setRooms(this.props.rooms);
@@ -130,13 +132,13 @@ export default class RoomSublist2 extends React.Component<IProps, IState> {
     };
 
     private onUnreadFirstChanged = async () => {
-        const isUnreadFirst = RoomListStore.instance.getListOrder(this.props.layout.tagId) === ListAlgorithm.Importance;
+        const isUnreadFirst = RoomListStore.instance.getListOrder(this.props.tagId) === ListAlgorithm.Importance;
         const newAlgorithm = isUnreadFirst ? ListAlgorithm.Natural : ListAlgorithm.Importance;
-        await RoomListStore.instance.setListOrder(this.props.layout.tagId, newAlgorithm);
+        await RoomListStore.instance.setListOrder(this.props.tagId, newAlgorithm);
     };
 
     private onTagSortChanged = async (sort: SortAlgorithm) => {
-        await RoomListStore.instance.setTagSorting(this.props.layout.tagId, sort);
+        await RoomListStore.instance.setTagSorting(this.props.tagId, sort);
     };
 
     private onMessagePreviewChanged = () => {
@@ -176,7 +178,7 @@ export default class RoomSublist2 extends React.Component<IProps, IState> {
                         key={`room-${room.roomId}`}
                         showMessagePreview={this.props.layout.showPreviews}
                         isMinimized={this.props.isMinimized}
-                        tag={this.props.layout.tagId}
+                        tag={this.props.tagId}
                     />
                 );
             }
@@ -189,8 +191,8 @@ export default class RoomSublist2 extends React.Component<IProps, IState> {
         let contextMenu = null;
         if (this.state.menuDisplayed) {
             const elementRect = this.menuButtonRef.current.getBoundingClientRect();
-            const isAlphabetical = RoomListStore.instance.getTagSorting(this.props.layout.tagId) === SortAlgorithm.Alphabetic;
+            const isAlphabetical = RoomListStore.instance.getTagSorting(this.props.tagId) === SortAlgorithm.Alphabetic;
-            const isUnreadFirst = RoomListStore.instance.getListOrder(this.props.layout.tagId) === ListAlgorithm.Importance;
+            const isUnreadFirst = RoomListStore.instance.getListOrder(this.props.tagId) === ListAlgorithm.Importance;
             contextMenu = (
                 <ContextMenu
                     chevronFace="none"
@@ -204,14 +206,14 @@ export default class RoomSublist2 extends React.Component<IProps, IState> {
                             <StyledRadioButton
                                 onChange={() => this.onTagSortChanged(SortAlgorithm.Recent)}
                                 checked={!isAlphabetical}
-                                name={`mx_${this.props.layout.tagId}_sortBy`}
+                                name={`mx_${this.props.tagId}_sortBy`}
                             >
                                 {_t("Activity")}
                             </StyledRadioButton>
                             <StyledRadioButton
                                 onChange={() => this.onTagSortChanged(SortAlgorithm.Alphabetic)}
                                 checked={isAlphabetical}
-                                name={`mx_${this.props.layout.tagId}_sortBy`}
+                                name={`mx_${this.props.tagId}_sortBy`}
                             >
                                 {_t("A-Z")}
                             </StyledRadioButton>
@@ -267,7 +269,7 @@ export default class RoomSublist2 extends React.Component<IProps, IState> {
 
                     // TODO: Collapsed state
 
-                    const badge = <NotificationBadge allowNoCount={false} notification={this.state.notificationState}/>;
+                    const badge = <NotificationBadge forceCount={true} notification={this.state.notificationState}/>;
 
                     let addRoomButton = null;
                     if (!!this.props.onAddRoom) {

src/components/views/rooms/RoomTile2.tsx

@@ -26,7 +26,11 @@ import RoomAvatar from "../../views/avatars/RoomAvatar";
 import dis from '../../../dispatcher/dispatcher';
 import { Key } from "../../../Keyboard";
 import ActiveRoomObserver from "../../../ActiveRoomObserver";
-import NotificationBadge, { INotificationState, NotificationColor, RoomNotificationState } from "./NotificationBadge";
+import NotificationBadge, {
+    INotificationState,
+    NotificationColor,
+    TagSpecificNotificationState
+} from "./NotificationBadge";
 import { _t } from "../../../languageHandler";
 import { ContextMenu, ContextMenuButton } from "../../structures/ContextMenu";
 import { DefaultTagID, TagID } from "../../../stores/room-list/models";
@@ -79,7 +83,7 @@ export default class RoomTile2 extends React.Component<IProps, IState> {
 
         this.state = {
             hover: false,
-            notificationState: new RoomNotificationState(this.props.room),
+            notificationState: new TagSpecificNotificationState(this.props.room, this.props.tag),
             selected: ActiveRoomObserver.activeRoomId === this.props.room.roomId,
             generalMenuDisplayed: false,
         };
@@ -248,7 +252,13 @@ export default class RoomTile2 extends React.Component<IProps, IState> {
             'mx_RoomTile2_minimized': this.props.isMinimized,
         });
 
-        const badge = <NotificationBadge notification={this.state.notificationState} allowNoCount={true} />;
+        const badge = (
+            <NotificationBadge
+                notification={this.state.notificationState}
+                forceCount={false}
+                roomId={this.props.room.roomId}
+            />
+        );
 
         // TODO: the original RoomTile uses state for the room name. Do we need to?
         let name = this.props.room.name;

src/settings/Settings.js

@@ -188,6 +188,11 @@ export const SETTINGS = {
         default: true,
         invertedSettingName: 'MessageComposerInput.dontSuggestEmoji',
     },
+    // TODO: Wire up appropriately to UI (FTUE notifications)
+    "Notifications.alwaysShowBadgeCounts": {
+        supportedLevels: ['account'],
+        default: false,
+    },
     "useCompactLayout": {
         supportedLevels: LEVELS_ACCOUNT_SETTINGS,
         displayName: _td('Use compact timeline layout'),