From 30d8dc06fcad02b0a2012d114b5a6fb5ecbf498c Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 19 Jun 2020 15:43:05 -0600 Subject: [PATCH 1/6] Increase bold weight for unread rooms For https://github.com/vector-im/riot-web/issues/14084 --- res/css/views/rooms/_RoomTile2.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/res/css/views/rooms/_RoomTile2.scss b/res/css/views/rooms/_RoomTile2.scss index 001499fea5..a97d1fd5b9 100644 --- a/res/css/views/rooms/_RoomTile2.scss +++ b/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 { From eeb408a0810e96afbb178652acbace7b0b6b0841 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 19 Jun 2020 15:44:37 -0600 Subject: [PATCH 2/6] Update badge logic for new setting and behaviour For https://github.com/vector-im/riot-web/issues/14084 --- .../views/rooms/NotificationBadge.tsx | 50 ++++++++++++++++--- src/components/views/rooms/RoomSublist2.tsx | 2 +- src/components/views/rooms/RoomTile2.tsx | 8 ++- src/settings/Settings.js | 5 ++ 4 files changed, 57 insertions(+), 8 deletions(-) diff --git a/src/components/views/rooms/NotificationBadge.tsx b/src/components/views/rooms/NotificationBadge.tsx index b742f8e8e7..65ccc88c5f 100644 --- a/src/components/views/rooms/NotificationBadge.tsx +++ b/src/components/views/rooms/NotificationBadge.tsx @@ -32,13 +32,14 @@ import ActiveRoomObserver from "../../../ActiveRoomObserver"; import { EventEmitter } from "events"; import { arrayDiff } from "../../../utils/arrays"; import { IDestroyable } from "../../../utils/IDestroyable"; +import SettingsStore from "../../../settings/SettingsStore"; 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 +54,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 { + 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) { @@ -75,24 +103,34 @@ export default class NotificationBadge extends React.PureComponent { + 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; + 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, diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index 08a41570e3..562c307769 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/src/components/views/rooms/RoomSublist2.tsx @@ -267,7 +267,7 @@ export default class RoomSublist2 extends React.Component { // TODO: Collapsed state - const badge = ; + const badge = ; let addRoomButton = null; if (!!this.props.onAddRoom) { diff --git a/src/components/views/rooms/RoomTile2.tsx b/src/components/views/rooms/RoomTile2.tsx index 9f4870d437..7f91b5ee9d 100644 --- a/src/components/views/rooms/RoomTile2.tsx +++ b/src/components/views/rooms/RoomTile2.tsx @@ -248,7 +248,13 @@ export default class RoomTile2 extends React.Component { 'mx_RoomTile2_minimized': this.props.isMinimized, }); - const badge = ; + const badge = ( + + ); // TODO: the original RoomTile uses state for the room name. Do we need to? let name = this.props.room.name; diff --git a/src/settings/Settings.js b/src/settings/Settings.js index ca8647e067..7cf0e629ec 100644 --- a/src/settings/Settings.js +++ b/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: LEVELS_ROOM_OR_ACCOUNT, + default: false, + }, "useCompactLayout": { supportedLevels: LEVELS_ACCOUNT_SETTINGS, displayName: _td('Use compact timeline layout'), From 8201eed929025bc24ae341d74fef4a5e494883ed Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 19 Jun 2020 16:03:38 -0600 Subject: [PATCH 3/6] Encourage counts if the user has a mention (red state) --- src/components/views/rooms/NotificationBadge.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/rooms/NotificationBadge.tsx b/src/components/views/rooms/NotificationBadge.tsx index 65ccc88c5f..2ddf095b59 100644 --- a/src/components/views/rooms/NotificationBadge.tsx +++ b/src/components/views/rooms/NotificationBadge.tsx @@ -118,7 +118,7 @@ export default class NotificationBadge extends React.PureComponent= NotificationColor.Red; const hasCount = this.props.notification.color >= NotificationColor.Grey; const hasUnread = this.props.notification.color >= NotificationColor.Bold; - const couldBeEmpty = !this.state.showCounts || hasUnread; + const couldBeEmpty = (!this.state.showCounts || hasUnread) && !hasNotif; let isEmptyBadge = couldBeEmpty && (!this.state.showCounts || !hasCount); if (this.props.forceCount) { isEmptyBadge = false; From 63ad14ae1e7a06a456b6b43a0e11bf803d3c3049 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 22 Jun 2020 10:35:55 -0600 Subject: [PATCH 4/6] Clean up imports --- src/components/views/rooms/NotificationBadge.tsx | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/components/views/rooms/NotificationBadge.tsx b/src/components/views/rooms/NotificationBadge.tsx index 2ddf095b59..36c269beba 100644 --- a/src/components/views/rooms/NotificationBadge.tsx +++ b/src/components/views/rooms/NotificationBadge.tsx @@ -18,17 +18,11 @@ 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"; From 784e73831bb070237e63ba2e75d527ac5b071a01 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 22 Jun 2020 11:23:38 -0600 Subject: [PATCH 5/6] Move setting to account only (no per-room) --- src/settings/Settings.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/settings/Settings.js b/src/settings/Settings.js index 7cf0e629ec..028f355ab8 100644 --- a/src/settings/Settings.js +++ b/src/settings/Settings.js @@ -190,7 +190,7 @@ export const SETTINGS = { }, // TODO: Wire up appropriately to UI (FTUE notifications) "Notifications.alwaysShowBadgeCounts": { - supportedLevels: LEVELS_ROOM_OR_ACCOUNT, + supportedLevels: ['account'], default: false, }, "useCompactLayout": { From fb551781c20dc7f872e866b615126d1fd1b70a55 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 22 Jun 2020 14:52:17 -0600 Subject: [PATCH 6/6] Force DMs to always be red notifications This also passes the tagId to the sublist so it doesn't have to rip it out of the `layout`. It doesn't get a layout until later anyways, which causes some null issues. --- .../views/rooms/NotificationBadge.tsx | 40 +++++++++++++++++-- src/components/views/rooms/RoomList2.tsx | 1 + src/components/views/rooms/RoomSublist2.tsx | 20 +++++----- src/components/views/rooms/RoomTile2.tsx | 8 +++- 4 files changed, 54 insertions(+), 15 deletions(-) diff --git a/src/components/views/rooms/NotificationBadge.tsx b/src/components/views/rooms/NotificationBadge.tsx index 36c269beba..37b61714b9 100644 --- a/src/components/views/rooms/NotificationBadge.tsx +++ b/src/components/views/rooms/NotificationBadge.tsx @@ -27,6 +27,7 @@ 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"; @@ -139,7 +140,7 @@ export default class NotificationBadge extends React.PureComponent { components.push( { 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 { }; 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 { 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 { 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 isUnreadFirst = RoomListStore.instance.getListOrder(this.props.layout.tagId) === ListAlgorithm.Importance; + const isAlphabetical = RoomListStore.instance.getTagSorting(this.props.tagId) === SortAlgorithm.Alphabetic; + const isUnreadFirst = RoomListStore.instance.getListOrder(this.props.tagId) === ListAlgorithm.Importance; contextMenu = ( { this.onTagSortChanged(SortAlgorithm.Recent)} checked={!isAlphabetical} - name={`mx_${this.props.layout.tagId}_sortBy`} + name={`mx_${this.props.tagId}_sortBy`} > {_t("Activity")} this.onTagSortChanged(SortAlgorithm.Alphabetic)} checked={isAlphabetical} - name={`mx_${this.props.layout.tagId}_sortBy`} + name={`mx_${this.props.tagId}_sortBy`} > {_t("A-Z")} diff --git a/src/components/views/rooms/RoomTile2.tsx b/src/components/views/rooms/RoomTile2.tsx index 7f91b5ee9d..18b4ee8185 100644 --- a/src/components/views/rooms/RoomTile2.tsx +++ b/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 { 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, };