From d237d02c03d8b7a29240acabade733693aeec96e Mon Sep 17 00:00:00 2001 From: Mike Pennisi Date: Fri, 3 Jul 2020 14:27:55 -0400 Subject: [PATCH 01/16] Highlight "Jump to Bottom" badge when appropriate This resolves https://github.com/vector-im/riot-web/issues/13135 Signed-off-by: Mike Pennisi --- res/css/views/rooms/_JumpToBottomButton.scss | 5 +++++ src/components/structures/RoomView.js | 1 + src/components/views/rooms/JumpToBottomButton.js | 7 ++++++- 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/res/css/views/rooms/_JumpToBottomButton.scss b/res/css/views/rooms/_JumpToBottomButton.scss index 63cf574596..23018df8da 100644 --- a/res/css/views/rooms/_JumpToBottomButton.scss +++ b/res/css/views/rooms/_JumpToBottomButton.scss @@ -41,6 +41,11 @@ limitations under the License. // with text-align in parent display: inline-block; padding: 0 4px; + color: $roomtile-badge-fg-color; + background-color: $roomtile-name-color; +} + +.mx_JumpToBottomButton_highlight .mx_JumpToBottomButton_badge { color: $secondary-accent-color; background-color: $warning-color; } diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index 519c4c1f8e..a9f75ce632 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -2044,6 +2044,7 @@ export default createReactClass({ if (!this.state.atEndOfLiveTimeline && !this.state.searchResults) { const JumpToBottomButton = sdk.getComponent('rooms.JumpToBottomButton'); jumpToBottom = ( 0} numUnreadMessages={this.state.numUnreadMessages} onScrollToBottomClick={this.jumpToLiveTimeline} />); diff --git a/src/components/views/rooms/JumpToBottomButton.js b/src/components/views/rooms/JumpToBottomButton.js index d3305f498a..b6cefc1231 100644 --- a/src/components/views/rooms/JumpToBottomButton.js +++ b/src/components/views/rooms/JumpToBottomButton.js @@ -16,13 +16,18 @@ limitations under the License. import { _t } from '../../../languageHandler'; import AccessibleButton from '../elements/AccessibleButton'; +import classNames from 'classnames'; export default (props) => { + const className = classNames({ + 'mx_JumpToBottomButton': true, + 'mx_JumpToBottomButton_highlight': props.highlight, + }); let badge; if (props.numUnreadMessages) { badge = (
{props.numUnreadMessages}
); } - return (
+ return (
From fe2bb355ab106a0d04648decc4beea38c7cb3e72 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Wed, 8 Jul 2020 18:02:20 +0100 Subject: [PATCH 02/16] Hide archive button --- src/components/structures/UserMenu.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/structures/UserMenu.tsx b/src/components/structures/UserMenu.tsx index 0cc312d653..1a0fac985b 100644 --- a/src/components/structures/UserMenu.tsx +++ b/src/components/structures/UserMenu.tsx @@ -280,11 +280,11 @@ export default class UserMenu extends React.Component { label={_t("All settings")} onClick={(e) => this.onSettingsOpen(e, null)} /> - + /> */} Date: Wed, 8 Jul 2020 18:07:01 +0100 Subject: [PATCH 03/16] i18n --- src/i18n/strings/en_EN.json | 1 - 1 file changed, 1 deletion(-) diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 6c4ccd6685..a38d87d08e 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -2137,7 +2137,6 @@ "Switch theme": "Switch theme", "Security & privacy": "Security & privacy", "All settings": "All settings", - "Archived rooms": "Archived rooms", "Feedback": "Feedback", "User menu": "User menu", "Could not load user profile": "Could not load user profile", From 9b48130f4f04da2fdaaf9b7de7dbe05763a61965 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 8 Jul 2020 11:48:34 -0600 Subject: [PATCH 04/16] Protect rooms from getting lost due to complex transitions Fixes https://github.com/vector-im/riot-web/issues/14378 Rooms transitioning between multiple states are often at risk of going missing due to the sticky room handling. We now protect that transition by bluntly ensuring the room can't go missing, and by always ensuring we have an updated reference to the room. --- src/stores/room-list/algorithms/Algorithm.ts | 33 +++++++++++++++----- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/src/stores/room-list/algorithms/Algorithm.ts b/src/stores/room-list/algorithms/Algorithm.ts index 8c7bbc8615..eee8e60b86 100644 --- a/src/stores/room-list/algorithms/Algorithm.ts +++ b/src/stores/room-list/algorithms/Algorithm.ts @@ -655,18 +655,35 @@ export class Algorithm extends EventEmitter { cause = RoomUpdateCause.PossibleTagChange; } - // If we have tags for a room and don't have the room referenced, the room reference - // probably changed. We need to swap out the problematic reference. - if (hasTags && !this.rooms.includes(room) && !isSticky) { - console.warn(`${room.roomId} is missing from room array but is known - trying to find duplicate`); + // Check to see if the room is known first + let knownRoomRef = this.rooms.includes(room); + if (hasTags && !knownRoomRef) { + console.warn(`${room.roomId} might be a reference change - attempting to update reference`); this.rooms = this.rooms.map(r => r.roomId === room.roomId ? room : r); - - // Sanity check - if (!this.rooms.includes(room)) { - throw new Error(`Failed to replace ${room.roomId} with an updated reference`); + knownRoomRef = this.rooms.includes(room); + if (!knownRoomRef) { + console.warn(`${room.roomId} is still not referenced. It may be sticky.`); } } + if (hasTags && isForLastSticky && !knownRoomRef) { + // we have a fairly good chance at losing a room right now. Under some circumstances, + // we can end up with a room which transitions references and tag changes, then gets + // lost when the sticky room changes. To counter this, we try and add the room to the + // list manually as the condition below to update the reference will fail. + // + // Other conditions *should* result in the room being sorted into the right place. + console.warn(`${room.roomId} was about to be lost - inserting at end of room list`); + this.rooms.push(room); + knownRoomRef = true; + } + + // If we have tags for a room and don't have the room referenced, something went horribly + // wrong - the reference should have been updated above. + if (hasTags && !knownRoomRef && !isSticky) { + throw new Error(`${room.roomId} is missing from room array but is known - trying to find duplicate`); + } + // Like above, update the reference to the sticky room if we need to if (hasTags && isSticky) { // Go directly in and set the sticky room's new reference, being careful not From e2d65222a24674a1206138bc1f54e4a41baaae72 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 8 Jul 2020 18:59:27 +0100 Subject: [PATCH 05/16] Don't render the context menu within its trigger otherwise unhandled clicks will re-trigger Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/structures/UserMenu.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/structures/UserMenu.tsx b/src/components/structures/UserMenu.tsx index 0cc312d653..9f69ad6eca 100644 --- a/src/components/structures/UserMenu.tsx +++ b/src/components/structures/UserMenu.tsx @@ -347,8 +347,8 @@ export default class UserMenu extends React.Component { {name} {buttons}
- {this.renderContextMenu()} + {this.renderContextMenu()} ); } From 6e208505674e4070f2bb1ffc1c0676dc5dee11de Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 8 Jul 2020 12:17:51 -0600 Subject: [PATCH 06/16] Remove sanity check from requestAnimationFrame It should be in all major browsers as of years ago, and we use it unguarded elsewhere in the app. The performance benefits of it appear to be worthwhile enough to keep it, though it's not a perfect solution. --- src/components/structures/LeftPanel2.tsx | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/components/structures/LeftPanel2.tsx b/src/components/structures/LeftPanel2.tsx index e92b0fa9ae..0875914b78 100644 --- a/src/components/structures/LeftPanel2.tsx +++ b/src/components/structures/LeftPanel2.tsx @@ -115,20 +115,12 @@ export default class LeftPanel2 extends React.Component { }; private handleStickyHeaders(list: HTMLDivElement) { - // TODO: Evaluate if this has any performance benefit or detriment. - // See https://github.com/vector-im/riot-web/issues/14035 - if (this.isDoingStickyHeaders) return; this.isDoingStickyHeaders = true; - if (window.requestAnimationFrame) { - window.requestAnimationFrame(() => { - this.doStickyHeaders(list); - this.isDoingStickyHeaders = false; - }); - } else { + window.requestAnimationFrame(() => { this.doStickyHeaders(list); this.isDoingStickyHeaders = false; - } + }); } private doStickyHeaders(list: HTMLDivElement) { From f9aca7c05e381022598f73953761b22d89546e2e Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 8 Jul 2020 14:36:55 -0600 Subject: [PATCH 07/16] Avoid bounding box usage in sticky headers & improve reliability We now use offsets and scroll information to determine where the headers should be stuck to, still supporting the transparent background. Some scroll jumps were originally introduced as part of the change in numbering, so they have been fixed here. By proxy, some additional scroll jump/instability should be fixed as well. This has a lingering problem of still causing a huge number of no-op UI updates though, which will be dealt with in a future commit. --- res/css/structures/_LeftPanel2.scss | 7 ++- res/css/views/rooms/_RoomSublist2.scss | 12 ++--- src/components/structures/LeftPanel2.tsx | 62 +++++++++++------------- 3 files changed, 39 insertions(+), 42 deletions(-) diff --git a/res/css/structures/_LeftPanel2.scss b/res/css/structures/_LeftPanel2.scss index eaa22a3efa..b3f7fcc8ee 100644 --- a/res/css/structures/_LeftPanel2.scss +++ b/res/css/structures/_LeftPanel2.scss @@ -122,16 +122,19 @@ $tagPanelWidth: 70px; // only applies in this file, used for calculations } .mx_LeftPanel2_roomListWrapper { + // Create a flexbox to ensure the containing items cause appropriate overflow. display: flex; + flex-grow: 1; overflow: hidden; min-height: 0; + margin-top: 12px; // so we're not up against the search/filter - &.stickyBottom { + &.mx_LeftPanel2_roomListWrapper_stickyBottom { padding-bottom: 32px; } - &.stickyTop { + &.mx_LeftPanel2_roomListWrapper_stickyTop { padding-top: 32px; } } diff --git a/res/css/views/rooms/_RoomSublist2.scss b/res/css/views/rooms/_RoomSublist2.scss index 30389a4ec5..1d13f25b8f 100644 --- a/res/css/views/rooms/_RoomSublist2.scss +++ b/res/css/views/rooms/_RoomSublist2.scss @@ -24,10 +24,6 @@ limitations under the License. margin-left: 8px; width: 100%; - &:first-child { - margin-top: 12px; // so we're not up against the search/filter - } - .mx_RoomSublist2_headerContainer { // Create a flexbox to make alignment easy display: flex; @@ -49,10 +45,15 @@ limitations under the License. padding-bottom: 8px; height: 24px; + // Hide the header container if the contained element is stickied. + // We don't use display:none as that causes the header to go away too. + &.mx_RoomSublist2_headerContainer_hasSticky { + height: 0; + } + .mx_RoomSublist2_stickable { flex: 1; max-width: 100%; - z-index: 2; // Prioritize headers in the visible list over sticky ones // Create a flexbox to make ordering easy display: flex; @@ -64,7 +65,6 @@ limitations under the License. // when sticky scrolls instead of collapses the list. &.mx_RoomSublist2_headerContainer_sticky { position: fixed; - z-index: 1; // over top of other elements, but still under the ones in the visible list height: 32px; // to match the header container // width set by JS } diff --git a/src/components/structures/LeftPanel2.tsx b/src/components/structures/LeftPanel2.tsx index 0875914b78..316b590aea 100644 --- a/src/components/structures/LeftPanel2.tsx +++ b/src/components/structures/LeftPanel2.tsx @@ -124,42 +124,42 @@ export default class LeftPanel2 extends React.Component { } private doStickyHeaders(list: HTMLDivElement) { - const rlRect = list.getBoundingClientRect(); - const bottom = rlRect.bottom; - const top = rlRect.top; + const topEdge = list.scrollTop; + const bottomEdge = list.offsetHeight + list.scrollTop; const sublists = list.querySelectorAll(".mx_RoomSublist2"); - const headerRightMargin = 24; // calculated from margins and widths to align with non-sticky tiles - const headerStickyWidth = rlRect.width - headerRightMargin; + const headerRightMargin = 16; // calculated from margins and widths to align with non-sticky tiles + const headerStickyWidth = list.clientWidth - headerRightMargin; - let gotBottom = false; let lastTopHeader; + let firstBottomHeader; for (const sublist of sublists) { - const slRect = sublist.getBoundingClientRect(); - const header = sublist.querySelector(".mx_RoomSublist2_stickable"); + const headerContainer = header.parentElement; // .mx_RoomSublist2_headerContainer header.style.removeProperty("display"); // always clear display:none first + headerContainer.classList.remove("mx_RoomSublist2_headerContainer_hasSticky"); - if (slRect.top + HEADER_HEIGHT > bottom && !gotBottom) { - header.classList.add("mx_RoomSublist2_headerContainer_sticky"); - header.classList.add("mx_RoomSublist2_headerContainer_stickyBottom"); - header.style.width = `${headerStickyWidth}px`; - header.style.removeProperty("top"); - gotBottom = true; - } else if (((slRect.top - (HEADER_HEIGHT * 0.6) + HEADER_HEIGHT) < top) || sublist === sublists[0]) { - // the header should become sticky once it is 60% or less out of view at the top. - // We also add HEADER_HEIGHT because the sticky header is put above the scrollable area, - // into the padding of .mx_LeftPanel2_roomListWrapper, - // by subtracting HEADER_HEIGHT from the top below. - // We also always try to make the first sublist header sticky. + // When an element is <=40% off screen, make it take over + const offScreenFactor = 0.4; + const isOffTop = (sublist.offsetTop + (offScreenFactor * HEADER_HEIGHT)) <= topEdge; + const isOffBottom = (sublist.offsetTop + (offScreenFactor * HEADER_HEIGHT)) >= bottomEdge; + + if (isOffTop || sublist === sublists[0]) { header.classList.add("mx_RoomSublist2_headerContainer_sticky"); header.classList.add("mx_RoomSublist2_headerContainer_stickyTop"); + headerContainer.classList.add("mx_RoomSublist2_headerContainer_hasSticky"); header.style.width = `${headerStickyWidth}px`; - header.style.top = `${rlRect.top - HEADER_HEIGHT}px`; + header.style.top = `${list.parentElement.offsetTop}px`; if (lastTopHeader) { lastTopHeader.style.display = "none"; } lastTopHeader = header; + } else if (isOffBottom && !firstBottomHeader) { + header.classList.add("mx_RoomSublist2_headerContainer_sticky"); + header.classList.add("mx_RoomSublist2_headerContainer_stickyBottom"); + headerContainer.classList.add("mx_RoomSublist2_headerContainer_hasSticky"); + header.style.width = `${headerStickyWidth}px`; + firstBottomHeader = header; } else { header.classList.remove("mx_RoomSublist2_headerContainer_sticky"); header.classList.remove("mx_RoomSublist2_headerContainer_stickyTop"); @@ -171,22 +171,16 @@ export default class LeftPanel2 extends React.Component { // add appropriate sticky classes to wrapper so it has // the necessary top/bottom padding to put the sticky header in - const listWrapper = list.parentElement; - if (gotBottom) { - listWrapper.classList.add("stickyBottom"); - } else { - listWrapper.classList.remove("stickyBottom"); - } + const listWrapper = list.parentElement; // .mx_LeftPanel2_roomListWrapper if (lastTopHeader) { - listWrapper.classList.add("stickyTop"); + listWrapper.classList.add("mx_LeftPanel2_roomListWrapper_stickyTop"); } else { - listWrapper.classList.remove("stickyTop"); + listWrapper.classList.remove("mx_LeftPanel2_roomListWrapper_stickyTop"); } - - // ensure scroll doesn't go above the gap left by the header of - // the first sublist always being sticky if no other header is sticky - if (list.scrollTop < HEADER_HEIGHT) { - list.scrollTop = HEADER_HEIGHT; + if (firstBottomHeader) { + listWrapper.classList.add("mx_LeftPanel2_roomListWrapper_stickyBottom"); + } else { + listWrapper.classList.remove("mx_LeftPanel2_roomListWrapper_stickyBottom"); } } From 74ca0618ac8e500fc95ef88615b8e652bc487450 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 8 Jul 2020 14:53:52 -0600 Subject: [PATCH 08/16] Improve scrolling performance for sticky headers The layout updates are anecdotal based on devtools flagging the values which are "changing" even if they aren't. The scrolling feels better with this as well, though this might be placebo. --- src/components/structures/LeftPanel2.tsx | 90 +++++++++++++++++++----- 1 file changed, 74 insertions(+), 16 deletions(-) diff --git a/src/components/structures/LeftPanel2.tsx b/src/components/structures/LeftPanel2.tsx index 316b590aea..f1f1ffd01f 100644 --- a/src/components/structures/LeftPanel2.tsx +++ b/src/components/structures/LeftPanel2.tsx @@ -131,13 +131,19 @@ export default class LeftPanel2 extends React.Component { const headerRightMargin = 16; // calculated from margins and widths to align with non-sticky tiles const headerStickyWidth = list.clientWidth - headerRightMargin; + // We track which styles we want on a target before making the changes to avoid + // excessive layout updates. + const targetStyles = new Map(); + let lastTopHeader; let firstBottomHeader; for (const sublist of sublists) { const header = sublist.querySelector(".mx_RoomSublist2_stickable"); - const headerContainer = header.parentElement; // .mx_RoomSublist2_headerContainer header.style.removeProperty("display"); // always clear display:none first - headerContainer.classList.remove("mx_RoomSublist2_headerContainer_hasSticky"); // When an element is <=40% off screen, make it take over const offScreenFactor = 0.4; @@ -145,27 +151,79 @@ export default class LeftPanel2 extends React.Component { const isOffBottom = (sublist.offsetTop + (offScreenFactor * HEADER_HEIGHT)) >= bottomEdge; if (isOffTop || sublist === sublists[0]) { - header.classList.add("mx_RoomSublist2_headerContainer_sticky"); - header.classList.add("mx_RoomSublist2_headerContainer_stickyTop"); - headerContainer.classList.add("mx_RoomSublist2_headerContainer_hasSticky"); - header.style.width = `${headerStickyWidth}px`; - header.style.top = `${list.parentElement.offsetTop}px`; + targetStyles.set(header, { stickyTop: true }); if (lastTopHeader) { lastTopHeader.style.display = "none"; + targetStyles.set(lastTopHeader, { makeInvisible: true }); } lastTopHeader = header; } else if (isOffBottom && !firstBottomHeader) { - header.classList.add("mx_RoomSublist2_headerContainer_sticky"); - header.classList.add("mx_RoomSublist2_headerContainer_stickyBottom"); - headerContainer.classList.add("mx_RoomSublist2_headerContainer_hasSticky"); - header.style.width = `${headerStickyWidth}px`; + targetStyles.set(header, { stickyBottom: true }); firstBottomHeader = header; } else { - header.classList.remove("mx_RoomSublist2_headerContainer_sticky"); - header.classList.remove("mx_RoomSublist2_headerContainer_stickyTop"); - header.classList.remove("mx_RoomSublist2_headerContainer_stickyBottom"); - header.style.removeProperty("width"); - header.style.removeProperty("top"); + targetStyles.set(header, {}); // nothing == clear + } + } + + // Run over the style changes and make them reality. We check to see if we're about to + // cause a no-op update, as adding/removing properties that are/aren't there cause + // layout updates. + for (const header of targetStyles.keys()) { + const style = targetStyles.get(header); + const headerContainer = header.parentElement; // .mx_RoomSublist2_headerContainer + + if (style.makeInvisible) { + // we will have already removed the 'display: none', so add it back. + header.style.display = "none"; + continue; // nothing else to do, even if sticky somehow + } + + if (style.stickyTop) { + if (!header.classList.contains("mx_RoomSublist2_headerContainer_stickyTop")) { + header.classList.add("mx_RoomSublist2_headerContainer_stickyTop"); + } + + const newTop = `${list.parentElement.offsetTop}px`; + if (header.style.top !== newTop) { + header.style.top = newTop; + } + } else if (style.stickyBottom) { + if (!header.classList.contains("mx_RoomSublist2_headerContainer_stickyBottom")) { + header.classList.add("mx_RoomSublist2_headerContainer_stickyBottom"); + } + } + + if (style.stickyTop || style.stickyBottom) { + if (!header.classList.contains("mx_RoomSublist2_headerContainer_sticky")) { + header.classList.add("mx_RoomSublist2_headerContainer_sticky"); + } + if (!headerContainer.classList.contains("mx_RoomSublist2_headerContainer_hasSticky")) { + headerContainer.classList.add("mx_RoomSublist2_headerContainer_hasSticky"); + } + + const newWidth = `${headerStickyWidth}px`; + if (header.style.width !== newWidth) { + header.style.width = newWidth; + } + } else if (!style.stickyTop && !style.stickyBottom) { + if (header.classList.contains("mx_RoomSublist2_headerContainer_sticky")) { + header.classList.remove("mx_RoomSublist2_headerContainer_sticky"); + } + if (header.classList.contains("mx_RoomSublist2_headerContainer_stickyTop")) { + header.classList.remove("mx_RoomSublist2_headerContainer_stickyTop"); + } + if (header.classList.contains("mx_RoomSublist2_headerContainer_stickyBottom")) { + header.classList.remove("mx_RoomSublist2_headerContainer_stickyBottom"); + } + if (headerContainer.classList.contains("mx_RoomSublist2_headerContainer_hasSticky")) { + headerContainer.classList.remove("mx_RoomSublist2_headerContainer_hasSticky"); + } + if (header.style.width) { + header.style.removeProperty('width'); + } + if (header.style.top) { + header.style.removeProperty('top'); + } } } From 8972cf937864280e5f918aea42f470842cdd428f Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 8 Jul 2020 16:09:45 -0600 Subject: [PATCH 09/16] Potential solution to supporting transparent 'show more' buttons In this demonstration, we remove the cutting line (as it collides with the tile in a weird spot) and instead replace the tile with a placeholder when the text is about to collide with the avatar in the tile. We use a `round()` for this because through some amazing coincidence the collision happens at 0.47, which is close enough to 0.5 for people not to notice. --- res/css/views/rooms/_RoomSublist2.scss | 20 +++++--------------- src/components/views/rooms/RoomSublist2.tsx | 10 ++++++++-- src/stores/room-list/ListLayout.ts | 4 ---- 3 files changed, 13 insertions(+), 21 deletions(-) diff --git a/res/css/views/rooms/_RoomSublist2.scss b/res/css/views/rooms/_RoomSublist2.scss index 30389a4ec5..f64e3c1842 100644 --- a/res/css/views/rooms/_RoomSublist2.scss +++ b/res/css/views/rooms/_RoomSublist2.scss @@ -187,16 +187,16 @@ limitations under the License. flex-direction: column; overflow: hidden; + .mx_RoomSublist2_placeholder { + height: 44px; // Height of a room tile plus margins + } + .mx_RoomSublist2_showNButton { cursor: pointer; font-size: $font-13px; line-height: $font-18px; color: $roomtile2-preview-color; - // This is the same color as the left panel background because it needs - // to occlude the lastmost tile in the list. - background-color: $roomlist2-bg-color; - // Update the render() function for RoomSublist2 if these change // Update the ListLayout class for minVisibleTiles if these change. // @@ -209,7 +209,7 @@ limitations under the License. // We force this to the bottom so it will overlap rooms as needed. // We account for the space it takes up (24px) in the code through padding. position: absolute; - bottom: 0; // the height of the resize handle + bottom: 0; left: 0; right: 0; @@ -236,16 +236,6 @@ limitations under the License. .mx_RoomSublist2_showLessButtonChevron { mask-image: url('$(res)/img/feather-customised/chevron-up.svg'); } - - &.mx_RoomSublist2_isCutting::before { - content: ''; - position: absolute; - top: 0; - left: 0; - right: 0; - height: 4px; - box-shadow: 0px -2px 3px rgba(46, 47, 50, 0.08); - } } // Class name comes from the ResizableBox component diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index 732585d3ac..d6ff77b400 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/src/components/views/rooms/RoomSublist2.tsx @@ -560,10 +560,8 @@ export default class RoomSublist2 extends React.Component { if (visibleTiles.length > 0) { const layout = this.props.layout; // to shorten calls - const maxTilesFactored = layout.tilesWithResizerBoxFactor(this.numTiles); const showMoreBtnClasses = classNames({ 'mx_RoomSublist2_showNButton': true, - 'mx_RoomSublist2_isCutting': this.state.isResizing && layout.visibleTiles < maxTilesFactored, }); // If we're hiding rooms, show a 'show more' button to the user. This button @@ -642,6 +640,14 @@ export default class RoomSublist2 extends React.Component { const tilesWithoutPadding = Math.min(relativeTiles, layout.visibleTiles); const tilesPx = layout.calculateTilesToPixelsMin(relativeTiles, tilesWithoutPadding, padding); + // Now that we know our padding constraints, let's find out if we need to chop off the + // last rendered visible tile so it doesn't collide with the 'show more' button + let visibleUnpaddedTiles = Math.round(layout.visibleTiles - layout.pixelsToTiles(padding)); + if (visibleUnpaddedTiles === visibleTiles.length - 1) { + const placeholder =
; + visibleTiles.splice(visibleUnpaddedTiles, 1, placeholder); + } + const dimensions = { height: tilesPx, }; diff --git a/src/stores/room-list/ListLayout.ts b/src/stores/room-list/ListLayout.ts index 99674fe74f..5169c5e4e5 100644 --- a/src/stores/room-list/ListLayout.ts +++ b/src/stores/room-list/ListLayout.ts @@ -109,10 +109,6 @@ export class ListLayout { return this.tilesToPixels(Math.min(maxTiles, n)) + padding; } - public tilesWithResizerBoxFactor(n: number): number { - return n + RESIZER_BOX_FACTOR; - } - public tilesWithPadding(n: number, paddingPx: number): number { return this.pixelsToTiles(this.tilesToPixelsWithPadding(n, paddingPx)); } From 016710af6a10cf6547668567ed6c029f7f1b4d02 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Thu, 9 Jul 2020 00:43:49 +0100 Subject: [PATCH 10/16] Noop first breadcrumb --- src/components/views/rooms/RoomBreadcrumbs2.tsx | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/components/views/rooms/RoomBreadcrumbs2.tsx b/src/components/views/rooms/RoomBreadcrumbs2.tsx index 687f4dd73e..f08adebcd2 100644 --- a/src/components/views/rooms/RoomBreadcrumbs2.tsx +++ b/src/components/views/rooms/RoomBreadcrumbs2.tsx @@ -16,7 +16,6 @@ limitations under the License. import React from "react"; import { BreadcrumbsStore } from "../../../stores/BreadcrumbsStore"; -import AccessibleButton from "../elements/AccessibleButton"; import DecoratedRoomAvatar from "../avatars/DecoratedRoomAvatar"; import { _t } from "../../../languageHandler"; import { Room } from "matrix-js-sdk/src/models/room"; @@ -88,16 +87,19 @@ export default class RoomBreadcrumbs2 extends React.PureComponent { Analytics.trackEvent("Breadcrumbs", "click_node", index); - defaultDispatcher.dispatch({action: "view_room", room_id: room.roomId}); + // If we're rendering the first breadcrumb and this is it no-op + if (!this.state.skipFirst && index === 0) { + return; + } else { + defaultDispatcher.dispatch({action: "view_room", room_id: room.roomId}); + } }; public render(): React.ReactElement { - // TODO: Decorate crumbs with icons: https://github.com/vector-im/riot-web/issues/14040 - // TODO: Scrolling: https://github.com/vector-im/riot-web/issues/14040 - // TODO: Tooltips: https://github.com/vector-im/riot-web/issues/14040 const tiles = BreadcrumbsStore.instance.rooms.map((r, i) => { const roomTags = RoomListStore.instance.getTagsForRoom(r); const roomTag = roomTags.includes(DefaultTagID.DM) ? DefaultTagID.DM : roomTags[0]; + const anon = () => this.viewRoom(r, i); return ( Date: Wed, 8 Jul 2020 18:27:50 -0600 Subject: [PATCH 11/16] Move list layout management to its own store This is more general maintenance than performance as the RoomList doesn't need to be generating layouts for the sublists, and it certainly doesn't need to be creating a bunch of extra ones. The sublists are perfectly capable of getting their own layout instance and using it, and we are perfectly able to limit the number of these things we create through the session's lifespan. --- src/@types/global.d.ts | 2 + src/components/views/rooms/RoomList2.tsx | 11 +---- src/components/views/rooms/RoomSublist2.tsx | 44 +++++++++-------- src/stores/room-list/RoomListLayoutStore.ts | 54 +++++++++++++++++++++ src/stores/room-list/RoomListStore2.ts | 13 ++--- 5 files changed, 84 insertions(+), 40 deletions(-) create mode 100644 src/stores/room-list/RoomListLayoutStore.ts diff --git a/src/@types/global.d.ts b/src/@types/global.d.ts index 2c2fec759c..fc52296d8b 100644 --- a/src/@types/global.d.ts +++ b/src/@types/global.d.ts @@ -21,6 +21,7 @@ import ToastStore from "../stores/ToastStore"; import DeviceListener from "../DeviceListener"; import { RoomListStore2 } from "../stores/room-list/RoomListStore2"; import { PlatformPeg } from "../PlatformPeg"; +import RoomListLayoutStore from "../stores/room-list/RoomListLayoutStore"; declare global { interface Window { @@ -34,6 +35,7 @@ declare global { mx_ToastStore: ToastStore; mx_DeviceListener: DeviceListener; mx_RoomListStore2: RoomListStore2; + mx_RoomListLayoutStore: RoomListLayoutStore; mxPlatformPeg: PlatformPeg; } diff --git a/src/components/views/rooms/RoomList2.tsx b/src/components/views/rooms/RoomList2.tsx index fb0136fb29..348c424927 100644 --- a/src/components/views/rooms/RoomList2.tsx +++ b/src/components/views/rooms/RoomList2.tsx @@ -32,7 +32,6 @@ import defaultDispatcher from "../../../dispatcher/dispatcher"; import RoomSublist2 from "./RoomSublist2"; import { ActionPayload } from "../../../dispatcher/payloads"; import { NameFilterCondition } from "../../../stores/room-list/filters/NameFilterCondition"; -import { ListLayout } from "../../../stores/room-list/ListLayout"; import { MatrixClientPeg } from "../../../MatrixClientPeg"; import GroupAvatar from "../avatars/GroupAvatar"; import TemporaryTile from "./TemporaryTile"; @@ -66,7 +65,6 @@ interface IProps { interface IState { sublists: ITagMap; - layouts: Map; } const TAG_ORDER: TagID[] = [ @@ -151,7 +149,6 @@ export default class RoomList2 extends React.Component { this.state = { sublists: {}, - layouts: new Map(), }; this.dispatcherRef = defaultDispatcher.register(this.onAction); @@ -227,12 +224,7 @@ export default class RoomList2 extends React.Component { const newLists = RoomListStore.instance.orderedLists; console.log("new lists", newLists); - const layoutMap = new Map(); - for (const tagId of Object.keys(newLists)) { - layoutMap.set(tagId, new ListLayout(tagId)); - } - - this.setState({sublists: newLists, layouts: layoutMap}, () => { + this.setState({sublists: newLists}, () => { this.props.onResize(); }); }; @@ -302,7 +294,6 @@ export default class RoomList2 extends React.Component { onAddRoom={onAddRoomFn} addRoomLabel={aesthetics.addRoomLabel} isInvite={aesthetics.isInvite} - layout={this.state.layouts.get(orderedTagId)} isMinimized={this.props.isMinimized} onResize={this.props.onResize} extraBadTilesThatShouldntExist={extraTiles} diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index 732585d3ac..3eadc9a313 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/src/components/views/rooms/RoomSublist2.tsx @@ -45,6 +45,7 @@ import {ActionPayload} from "../../../dispatcher/payloads"; import { Enable, Resizable } from "re-resizable"; import { Direction } from "re-resizable/lib/resizer"; import { polyfillTouchEvent } from "../../../@types/polyfill"; +import RoomListLayoutStore from "../../../stores/room-list/RoomListLayoutStore"; // TODO: Remove banner on launch: https://github.com/vector-im/riot-web/issues/14231 // TODO: Rename on launch: https://github.com/vector-im/riot-web/issues/14231 @@ -74,7 +75,6 @@ interface IProps { onAddRoom?: () => void; addRoomLabel: string; isInvite: boolean; - layout?: ListLayout; isMinimized: boolean; tagId: TagID; onResize: () => void; @@ -98,10 +98,13 @@ export default class RoomSublist2 extends React.Component { private headerButton = createRef(); private sublistRef = createRef(); private dispatcherRef: string; + private layout: ListLayout; constructor(props: IProps) { super(props); + this.layout = RoomListLayoutStore.instance.getLayoutFor(this.props.tagId); + this.state = { notificationState: new ListNotificationState(this.props.isInvite, this.props.tagId), contextMenuPosition: null, @@ -116,8 +119,7 @@ export default class RoomSublist2 extends React.Component { } private get numVisibleTiles(): number { - if (!this.props.layout) return 0; - const nVisible = Math.floor(this.props.layout.visibleTiles); + const nVisible = Math.floor(this.layout.visibleTiles); return Math.min(nVisible, this.numTiles); } @@ -135,7 +137,7 @@ export default class RoomSublist2 extends React.Component { // XXX: we have to do this a tick later because we have incorrect intermediate props during a room change // where we lose the room we are changing from temporarily and then it comes back in an update right after. setImmediate(() => { - const isCollapsed = this.props.layout.isCollapsed; + const isCollapsed = this.layout.isCollapsed; const roomIndex = this.props.rooms.findIndex((r) => r.roomId === payload.room_id); if (isCollapsed && roomIndex > -1) { @@ -143,7 +145,7 @@ export default class RoomSublist2 extends React.Component { } // extend the visible section to include the room if it is entirely invisible if (roomIndex >= this.numVisibleTiles) { - this.props.layout.visibleTiles = this.props.layout.tilesWithPadding(roomIndex + 1, MAX_PADDING_HEIGHT); + this.layout.visibleTiles = this.layout.tilesWithPadding(roomIndex + 1, MAX_PADDING_HEIGHT); this.forceUpdate(); // because the layout doesn't trigger a re-render } }); @@ -170,10 +172,10 @@ export default class RoomSublist2 extends React.Component { // resizing started*, meaning it is fairly useless for us. This is why we just use // the client height and run with it. - const heightBefore = this.props.layout.visibleTiles; - const heightInTiles = this.props.layout.pixelsToTiles(refToElement.clientHeight); - this.props.layout.setVisibleTilesWithin(heightInTiles, this.numTiles); - if (heightBefore === this.props.layout.visibleTiles) return; // no-op + const heightBefore = this.layout.visibleTiles; + const heightInTiles = this.layout.pixelsToTiles(refToElement.clientHeight); + this.layout.setVisibleTilesWithin(heightInTiles, this.numTiles); + if (heightBefore === this.layout.visibleTiles) return; // no-op this.forceUpdate(); // because the layout doesn't trigger a re-render }; @@ -187,13 +189,13 @@ export default class RoomSublist2 extends React.Component { private onShowAllClick = () => { const numVisibleTiles = this.numVisibleTiles; - this.props.layout.visibleTiles = this.props.layout.tilesWithPadding(this.numTiles, MAX_PADDING_HEIGHT); + this.layout.visibleTiles = this.layout.tilesWithPadding(this.numTiles, MAX_PADDING_HEIGHT); this.forceUpdate(); // because the layout doesn't trigger a re-render setImmediate(this.focusRoomTile, numVisibleTiles); // focus the tile after the current bottom one }; private onShowLessClick = () => { - this.props.layout.visibleTiles = this.props.layout.defaultVisibleTiles; + this.layout.visibleTiles = this.layout.defaultVisibleTiles; this.forceUpdate(); // because the layout doesn't trigger a re-render // focus will flow to the show more button here }; @@ -241,7 +243,7 @@ export default class RoomSublist2 extends React.Component { }; private onMessagePreviewChanged = () => { - this.props.layout.showPreviews = !this.props.layout.showPreviews; + this.layout.showPreviews = !this.layout.showPreviews; this.forceUpdate(); // because the layout doesn't trigger a re-render }; @@ -293,13 +295,13 @@ export default class RoomSublist2 extends React.Component { }; private toggleCollapsed = () => { - this.props.layout.isCollapsed = !this.props.layout.isCollapsed; + this.layout.isCollapsed = !this.layout.isCollapsed; this.forceUpdate(); // because the layout doesn't trigger an update setImmediate(() => this.props.onResize()); // needs to happen when the DOM is updated }; private onHeaderKeyDown = (ev: React.KeyboardEvent) => { - const isCollapsed = this.props.layout && this.props.layout.isCollapsed; + const isCollapsed = this.layout && this.layout.isCollapsed; switch (ev.key) { case Key.ARROW_LEFT: ev.stopPropagation(); @@ -339,7 +341,7 @@ export default class RoomSublist2 extends React.Component { }; private renderVisibleTiles(): React.ReactElement[] { - if (this.props.layout && this.props.layout.isCollapsed) { + if (this.layout && this.layout.isCollapsed) { // don't waste time on rendering return []; } @@ -353,7 +355,7 @@ export default class RoomSublist2 extends React.Component { @@ -404,7 +406,7 @@ export default class RoomSublist2 extends React.Component { {_t("Message preview")} @@ -496,7 +498,7 @@ export default class RoomSublist2 extends React.Component { const collapseClasses = classNames({ 'mx_RoomSublist2_collapseBtn': true, - 'mx_RoomSublist2_collapseBtn_collapsed': this.props.layout && this.props.layout.isCollapsed, + 'mx_RoomSublist2_collapseBtn_collapsed': this.layout && this.layout.isCollapsed, }); const classes = classNames({ @@ -524,7 +526,7 @@ export default class RoomSublist2 extends React.Component { tabIndex={tabIndex} className="mx_RoomSublist2_headerText" role="treeitem" - aria-expanded={!this.props.layout || !this.props.layout.isCollapsed} + aria-expanded={!this.layout.isCollapsed} aria-level={1} onClick={this.onHeaderClick} onContextMenu={this.onContextMenu} @@ -558,7 +560,7 @@ export default class RoomSublist2 extends React.Component { let content = null; if (visibleTiles.length > 0) { - const layout = this.props.layout; // to shorten calls + const layout = this.layout; // to shorten calls const maxTilesFactored = layout.tilesWithResizerBoxFactor(this.numTiles); const showMoreBtnClasses = classNames({ @@ -587,7 +589,7 @@ export default class RoomSublist2 extends React.Component { {showMoreText} ); - } else if (this.numTiles <= visibleTiles.length && this.numTiles > this.props.layout.defaultVisibleTiles) { + } else if (this.numTiles <= visibleTiles.length && this.numTiles > this.layout.defaultVisibleTiles) { // we have all tiles visible - add a button to show less let showLessText = ( diff --git a/src/stores/room-list/RoomListLayoutStore.ts b/src/stores/room-list/RoomListLayoutStore.ts new file mode 100644 index 0000000000..dd4364f5fc --- /dev/null +++ b/src/stores/room-list/RoomListLayoutStore.ts @@ -0,0 +1,54 @@ +/* +Copyright 2020 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import { TagID } from "./models"; +import { ListLayout } from "./ListLayout"; + +export default class RoomListLayoutStore { + private static internalInstance: RoomListLayoutStore; + + private readonly layoutMap = new Map(); + + public ensureLayoutExists(tagId: TagID) { + if (!this.layoutMap.has(tagId)) { + this.layoutMap.set(tagId, new ListLayout(tagId)); + } + } + + public getLayoutFor(tagId: TagID): ListLayout { + if (!this.layoutMap.has(tagId)) { + this.layoutMap.set(tagId, new ListLayout(tagId)); + } + return this.layoutMap.get(tagId); + } + + // Note: this primarily exists for debugging, and isn't really intended to be used by anything. + public async resetLayouts() { + console.warn("Resetting layouts for room list"); + for (const layout of this.layoutMap.values()) { + layout.reset(); + } + } + + public static get instance(): RoomListLayoutStore { + if (!RoomListLayoutStore.internalInstance) { + RoomListLayoutStore.internalInstance = new RoomListLayoutStore(); + } + return RoomListLayoutStore.internalInstance; + } +} + +window.mx_RoomListLayoutStore = RoomListLayoutStore.instance; diff --git a/src/stores/room-list/RoomListStore2.ts b/src/stores/room-list/RoomListStore2.ts index 82c419d79d..6020e46a12 100644 --- a/src/stores/room-list/RoomListStore2.ts +++ b/src/stores/room-list/RoomListStore2.ts @@ -32,6 +32,7 @@ import { Algorithm, LIST_UPDATED_EVENT } from "./algorithms/Algorithm"; import { EffectiveMembership, getEffectiveMembership } from "./membership"; import { ListLayout } from "./ListLayout"; import { isNullOrUndefined } from "matrix-js-sdk/src/utils"; +import RoomListLayoutStore from "./RoomListLayoutStore"; interface IState { tagsEnabled?: boolean; @@ -50,6 +51,7 @@ export class RoomListStore2 extends AsyncStore { private algorithm = new Algorithm(); private filterConditions: IFilterCondition[] = []; private tagWatcher = new TagWatcher(this); + private layoutMap: Map = new Map(); private readonly watchedSettings = [ 'feature_custom_tags', @@ -416,6 +418,8 @@ export class RoomListStore2 extends AsyncStore { for (const tagId of OrderedDefaultTagIDs) { sorts[tagId] = this.calculateTagSorting(tagId); orders[tagId] = this.calculateListOrder(tagId); + + RoomListLayoutStore.instance.ensureLayoutExists(tagId); } if (this.state.tagsEnabled) { @@ -434,15 +438,6 @@ export class RoomListStore2 extends AsyncStore { this.emit(LISTS_UPDATE_EVENT, this); } - // Note: this primarily exists for debugging, and isn't really intended to be used by anything. - public async resetLayouts() { - console.warn("Resetting layouts for room list"); - for (const tagId of Object.keys(this.orderedLists)) { - new ListLayout(tagId).reset(); - } - await this.regenerateAllLists(); - } - public addFilter(filter: IFilterCondition): void { // TODO: Remove debug: https://github.com/vector-im/riot-web/issues/14035 console.log("Adding filter condition:", filter); From 2baa78d26baf80c0208c1ec50022d023ad8c4141 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Thu, 9 Jul 2020 01:31:44 +0100 Subject: [PATCH 12/16] Move no-op to breadcrumb store --- .../views/rooms/RoomBreadcrumbs2.tsx | 8 +---- src/stores/BreadcrumbsStore.ts | 35 +++++++++++++------ 2 files changed, 25 insertions(+), 18 deletions(-) diff --git a/src/components/views/rooms/RoomBreadcrumbs2.tsx b/src/components/views/rooms/RoomBreadcrumbs2.tsx index f08adebcd2..602c697d8a 100644 --- a/src/components/views/rooms/RoomBreadcrumbs2.tsx +++ b/src/components/views/rooms/RoomBreadcrumbs2.tsx @@ -87,19 +87,13 @@ export default class RoomBreadcrumbs2 extends React.PureComponent { Analytics.trackEvent("Breadcrumbs", "click_node", index); - // If we're rendering the first breadcrumb and this is it no-op - if (!this.state.skipFirst && index === 0) { - return; - } else { - defaultDispatcher.dispatch({action: "view_room", room_id: room.roomId}); - } + defaultDispatcher.dispatch({action: "view_room", room_id: room.roomId}); }; public render(): React.ReactElement { const tiles = BreadcrumbsStore.instance.rooms.map((r, i) => { const roomTags = RoomListStore.instance.getTagsForRoom(r); const roomTag = roomTags.includes(DefaultTagID.DM) ? DefaultTagID.DM : roomTags[0]; - const anon = () => this.viewRoom(r, i); return ( { } private async appendRoom(room: Room) { + let updated = false; const rooms = (this.state.rooms || []).slice(); // cheap clone // If the room is upgraded, use that room instead. We'll also splice out @@ -136,30 +137,42 @@ export class BreadcrumbsStore extends AsyncStoreWithClient { // Take out any room that isn't the most recent room for (let i = 0; i < history.length - 1; i++) { const idx = rooms.findIndex(r => r.roomId === history[i].roomId); - if (idx !== -1) rooms.splice(idx, 1); + if (idx !== -1) { + rooms.splice(idx, 1); + updated = true; + } } } // Remove the existing room, if it is present const existingIdx = rooms.findIndex(r => r.roomId === room.roomId); - if (existingIdx !== -1) { - rooms.splice(existingIdx, 1); - } - // Splice the room to the start of the list - rooms.splice(0, 0, room); + // If we're focusing on the first room no-op + if (existingIdx !== 0) { + if (existingIdx !== -1) { + rooms.splice(existingIdx, 1); + } + + // Splice the room to the start of the list + rooms.splice(0, 0, room); + updated = true; + } if (rooms.length > MAX_ROOMS) { // This looks weird, but it's saying to start at the MAX_ROOMS point in the // list and delete everything after it. rooms.splice(MAX_ROOMS, rooms.length - MAX_ROOMS); + updated = true; } - // Update the breadcrumbs - await this.updateState({rooms}); - const roomIds = rooms.map(r => r.roomId); - if (roomIds.length > 0) { - await SettingsStore.setValue("breadcrumb_rooms", null, SettingLevel.ACCOUNT, roomIds); + + if (updated) { + // Update the breadcrumbs + await this.updateState({rooms}); + const roomIds = rooms.map(r => r.roomId); + if (roomIds.length > 0) { + await SettingsStore.setValue("breadcrumb_rooms", null, SettingLevel.ACCOUNT, roomIds); + } } } From c8f90be81d5a1d5df687066192b2af649bef77d5 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 8 Jul 2020 18:32:12 -0600 Subject: [PATCH 13/16] Ensure the map gets cleared upon logout --- src/stores/room-list/RoomListLayoutStore.ts | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/stores/room-list/RoomListLayoutStore.ts b/src/stores/room-list/RoomListLayoutStore.ts index dd4364f5fc..4d8c98a4f8 100644 --- a/src/stores/room-list/RoomListLayoutStore.ts +++ b/src/stores/room-list/RoomListLayoutStore.ts @@ -16,12 +16,21 @@ limitations under the License. import { TagID } from "./models"; import { ListLayout } from "./ListLayout"; +import { AsyncStoreWithClient } from "../AsyncStoreWithClient"; +import defaultDispatcher from "../../dispatcher/dispatcher"; +import { ActionPayload } from "../../dispatcher/payloads"; -export default class RoomListLayoutStore { +interface IState {} + +export default class RoomListLayoutStore extends AsyncStoreWithClient { private static internalInstance: RoomListLayoutStore; private readonly layoutMap = new Map(); + constructor() { + super(defaultDispatcher); + } + public ensureLayoutExists(tagId: TagID) { if (!this.layoutMap.has(tagId)) { this.layoutMap.set(tagId, new ListLayout(tagId)); @@ -49,6 +58,16 @@ export default class RoomListLayoutStore { } return RoomListLayoutStore.internalInstance; } + + protected async onNotReady(): Promise { + // On logout, clear the map. + this.layoutMap.clear(); + } + + // We don't need this function, but our contract says we do + protected async onAction(payload: ActionPayload): Promise { + return Promise.resolve(); + } } window.mx_RoomListLayoutStore = RoomListLayoutStore.instance; From 62b4596c049225ab767f7ef5a91c71cc2749d9fa Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 8 Jul 2020 18:36:56 -0600 Subject: [PATCH 14/16] Be consistent with other stores --- src/stores/room-list/RoomListLayoutStore.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/stores/room-list/RoomListLayoutStore.ts b/src/stores/room-list/RoomListLayoutStore.ts index 4d8c98a4f8..fbc7d7719d 100644 --- a/src/stores/room-list/RoomListLayoutStore.ts +++ b/src/stores/room-list/RoomListLayoutStore.ts @@ -31,6 +31,13 @@ export default class RoomListLayoutStore extends AsyncStoreWithClient { super(defaultDispatcher); } + public static get instance(): RoomListLayoutStore { + if (!RoomListLayoutStore.internalInstance) { + RoomListLayoutStore.internalInstance = new RoomListLayoutStore(); + } + return RoomListLayoutStore.internalInstance; + } + public ensureLayoutExists(tagId: TagID) { if (!this.layoutMap.has(tagId)) { this.layoutMap.set(tagId, new ListLayout(tagId)); @@ -52,13 +59,6 @@ export default class RoomListLayoutStore extends AsyncStoreWithClient { } } - public static get instance(): RoomListLayoutStore { - if (!RoomListLayoutStore.internalInstance) { - RoomListLayoutStore.internalInstance = new RoomListLayoutStore(); - } - return RoomListLayoutStore.internalInstance; - } - protected async onNotReady(): Promise { // On logout, clear the map. this.layoutMap.clear(); From 47380306c27b56ac26350695e29d57b1a30f8c3f Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 8 Jul 2020 19:26:25 -0600 Subject: [PATCH 15/16] Move and improve notification state handling Previously we were creating a notification state whenever we needed one, which was leading to hundreds of listeners even on a small account. To ease the burden, and reduce the load of having to wake so many listeners, we now record a single listener for each tag ID and room combination. This commit also introduces a number of utilities to make future notification work a bit of an easier transition, such as the `isX` and `hasX` getters on the new NotificationState abstract class. Similarly, "snapshots" have been added to reduce code duplication between different kinds of states checking for updates. The ListNotificationState is now heavily tied into the store which offers it to help reuse the cache of room notification states. Fixes https://github.com/vector-im/riot-web/issues/14370 --- .../views/avatars/DecoratedRoomAvatar.tsx | 8 +- .../views/rooms/NotificationBadge.tsx | 17 ++- src/components/views/rooms/RoomList2.tsx | 12 +-- src/components/views/rooms/RoomSublist2.tsx | 4 +- src/components/views/rooms/RoomTile2.tsx | 18 ++-- src/components/views/rooms/TemporaryTile.tsx | 7 +- .../notifications/INotificationState.ts | 26 ----- .../notifications/ListNotificationState.ts | 34 ++---- src/stores/notifications/NotificationState.ts | 87 +++++++++++++++ .../notifications/RoomNotificationState.ts | 29 +---- .../RoomNotificationStateStore.ts | 101 ++++++++++++++++++ .../notifications/StaticNotificationState.ts | 10 +- 12 files changed, 237 insertions(+), 116 deletions(-) delete mode 100644 src/stores/notifications/INotificationState.ts create mode 100644 src/stores/notifications/NotificationState.ts create mode 100644 src/stores/notifications/RoomNotificationStateStore.ts diff --git a/src/components/views/avatars/DecoratedRoomAvatar.tsx b/src/components/views/avatars/DecoratedRoomAvatar.tsx index e0ad3202b8..40ba15af33 100644 --- a/src/components/views/avatars/DecoratedRoomAvatar.tsx +++ b/src/components/views/avatars/DecoratedRoomAvatar.tsx @@ -21,8 +21,8 @@ import { TagID } from '../../../stores/room-list/models'; import RoomAvatar from "./RoomAvatar"; import RoomTileIcon from "../rooms/RoomTileIcon"; import NotificationBadge from '../rooms/NotificationBadge'; -import { INotificationState } from "../../../stores/notifications/INotificationState"; -import { TagSpecificNotificationState } from "../../../stores/notifications/TagSpecificNotificationState"; +import { RoomNotificationStateStore } from "../../../stores/notifications/RoomNotificationStateStore"; +import { NotificationState } from "../../../stores/notifications/NotificationState"; interface IProps { room: Room; @@ -33,7 +33,7 @@ interface IProps { } interface IState { - notificationState?: INotificationState; + notificationState?: NotificationState; } export default class DecoratedRoomAvatar extends React.PureComponent { @@ -42,7 +42,7 @@ export default class DecoratedRoomAvatar extends React.PureComponent= NotificationColor.Red; - const hasCount = notification.color >= NotificationColor.Grey; const hasAnySymbol = notification.symbol || notification.count > 0; - let isEmptyBadge = !hasAnySymbol || !hasCount; + let isEmptyBadge = !hasAnySymbol || !notification.hasUnreadCount; if (forceCount) { isEmptyBadge = false; - if (!hasCount) return null; // Can't render a badge + if (!notification.hasUnreadCount) return null; // Can't render a badge } let symbol = notification.symbol || formatMinimalBadgeCount(notification.count); @@ -117,8 +114,8 @@ export default class NotificationBadge extends React.PureComponent 0 && symbol.length < 3, 'mx_NotificationBadge_3char': symbol.length > 2, diff --git a/src/components/views/rooms/RoomList2.tsx b/src/components/views/rooms/RoomList2.tsx index fb0136fb29..25ffc74fc2 100644 --- a/src/components/views/rooms/RoomList2.tsx +++ b/src/components/views/rooms/RoomList2.tsx @@ -38,9 +38,9 @@ import GroupAvatar from "../avatars/GroupAvatar"; import TemporaryTile from "./TemporaryTile"; import { StaticNotificationState } from "../../../stores/notifications/StaticNotificationState"; import { NotificationColor } from "../../../stores/notifications/NotificationColor"; -import { TagSpecificNotificationState } from "../../../stores/notifications/TagSpecificNotificationState"; import { Action } from "../../../dispatcher/actions"; import { ViewRoomDeltaPayload } from "../../../dispatcher/payloads/ViewRoomDeltaPayload"; +import { RoomNotificationStateStore } from "../../../stores/notifications/RoomNotificationStateStore"; // TODO: Remove banner on launch: https://github.com/vector-im/riot-web/issues/14231 // TODO: Rename on launch: https://github.com/vector-im/riot-web/issues/14231 @@ -204,14 +204,11 @@ export default class RoomList2 extends React.Component { let listRooms = lists[t]; if (unread) { - // TODO Be smarter and not spin up a bunch of wasted listeners just to kill them 4 lines later - // https://github.com/vector-im/riot-web/issues/14035 - const notificationStates = rooms.map(r => new TagSpecificNotificationState(r, t)); // filter to only notification rooms (and our current active room so we can index properly) - listRooms = notificationStates.filter(state => { - return state.room.roomId === roomId || state.color >= NotificationColor.Bold; + listRooms = listRooms.filter(r => { + const state = RoomNotificationStateStore.instance.getRoomState(r, t); + return state.room.roomId === roomId || state.isUnread; }); - notificationStates.forEach(state => state.destroy()); } rooms.push(...listRooms); @@ -301,7 +298,6 @@ export default class RoomList2 extends React.Component { label={_t(aesthetics.sectionLabel)} onAddRoom={onAddRoomFn} addRoomLabel={aesthetics.addRoomLabel} - isInvite={aesthetics.isInvite} layout={this.state.layouts.get(orderedTagId)} isMinimized={this.props.isMinimized} onResize={this.props.onResize} diff --git a/src/components/views/rooms/RoomSublist2.tsx b/src/components/views/rooms/RoomSublist2.tsx index 732585d3ac..0561c7b3c7 100644 --- a/src/components/views/rooms/RoomSublist2.tsx +++ b/src/components/views/rooms/RoomSublist2.tsx @@ -45,6 +45,7 @@ import {ActionPayload} from "../../../dispatcher/payloads"; import { Enable, Resizable } from "re-resizable"; import { Direction } from "re-resizable/lib/resizer"; import { polyfillTouchEvent } from "../../../@types/polyfill"; +import { RoomNotificationStateStore } from "../../../stores/notifications/RoomNotificationStateStore"; // TODO: Remove banner on launch: https://github.com/vector-im/riot-web/issues/14231 // TODO: Rename on launch: https://github.com/vector-im/riot-web/issues/14231 @@ -73,7 +74,6 @@ interface IProps { label: string; onAddRoom?: () => void; addRoomLabel: string; - isInvite: boolean; layout?: ListLayout; isMinimized: boolean; tagId: TagID; @@ -103,7 +103,7 @@ export default class RoomSublist2 extends React.Component { super(props); this.state = { - notificationState: new ListNotificationState(this.props.isInvite, this.props.tagId), + notificationState: RoomNotificationStateStore.instance.getListState(this.props.tagId), contextMenuPosition: null, isResizing: false, }; diff --git a/src/components/views/rooms/RoomTile2.tsx b/src/components/views/rooms/RoomTile2.tsx index 4ecd6bb1ff..db8084baa2 100644 --- a/src/components/views/rooms/RoomTile2.tsx +++ b/src/components/views/rooms/RoomTile2.tsx @@ -46,15 +46,14 @@ import { MUTE, } from "../../../RoomNotifs"; import { MatrixClientPeg } from "../../../MatrixClientPeg"; -import { TagSpecificNotificationState } from "../../../stores/notifications/TagSpecificNotificationState"; -import { INotificationState } from "../../../stores/notifications/INotificationState"; import NotificationBadge from "./NotificationBadge"; -import { NotificationColor } from "../../../stores/notifications/NotificationColor"; import { Volume } from "../../../RoomNotifsTypes"; import RoomListStore from "../../../stores/room-list/RoomListStore2"; import RoomListActions from "../../../actions/RoomListActions"; import defaultDispatcher from "../../../dispatcher/dispatcher"; import {ActionPayload} from "../../../dispatcher/payloads"; +import { RoomNotificationStateStore } from "../../../stores/notifications/RoomNotificationStateStore"; +import { NotificationState } from "../../../stores/notifications/NotificationState"; // TODO: Remove banner on launch: https://github.com/vector-im/riot-web/issues/14231 // TODO: Rename on launch: https://github.com/vector-im/riot-web/issues/14231 @@ -80,7 +79,7 @@ type PartialDOMRect = Pick; interface IState { hover: boolean; - notificationState: INotificationState; + notificationState: NotificationState; selected: boolean; notificationsMenuPosition: PartialDOMRect; generalMenuPosition: PartialDOMRect; @@ -132,7 +131,7 @@ export default class RoomTile2 extends React.Component { this.state = { hover: false, - notificationState: new TagSpecificNotificationState(this.props.room, this.props.tag), + notificationState: RoomNotificationStateStore.instance.getRoomState(this.props.room, this.props.tag), selected: ActiveRoomObserver.activeRoomId === this.props.room.roomId, notificationsMenuPosition: null, generalMenuPosition: null, @@ -492,11 +491,10 @@ export default class RoomTile2 extends React.Component { } } - const notificationColor = this.state.notificationState.color; const nameClasses = classNames({ "mx_RoomTile2_name": true, "mx_RoomTile2_nameWithPreview": !!messagePreview, - "mx_RoomTile2_nameHasUnreadEvents": notificationColor >= NotificationColor.Bold, + "mx_RoomTile2_nameHasUnreadEvents": this.state.notificationState.isUnread, }); let nameContainer = ( @@ -513,15 +511,15 @@ export default class RoomTile2 extends React.Component { // The following labels are written in such a fashion to increase screen reader efficiency (speed). if (this.props.tag === DefaultTagID.Invite) { // append nothing - } else if (notificationColor >= NotificationColor.Red) { + } else if (this.state.notificationState.hasMentions) { ariaLabel += " " + _t("%(count)s unread messages including mentions.", { count: this.state.notificationState.count, }); - } else if (notificationColor >= NotificationColor.Grey) { + } else if (this.state.notificationState.hasUnreadCount) { ariaLabel += " " + _t("%(count)s unread messages.", { count: this.state.notificationState.count, }); - } else if (notificationColor >= NotificationColor.Bold) { + } else if (this.state.notificationState.isUnread) { ariaLabel += " " + _t("Unread messages."); } diff --git a/src/components/views/rooms/TemporaryTile.tsx b/src/components/views/rooms/TemporaryTile.tsx index b6c165ecda..a3ee7eb5bd 100644 --- a/src/components/views/rooms/TemporaryTile.tsx +++ b/src/components/views/rooms/TemporaryTile.tsx @@ -18,16 +18,15 @@ import React from "react"; import classNames from "classnames"; import { RovingTabIndexWrapper } from "../../../accessibility/RovingTabIndex"; import AccessibleButton from "../../views/elements/AccessibleButton"; -import { INotificationState } from "../../../stores/notifications/INotificationState"; import NotificationBadge from "./NotificationBadge"; -import { NotificationColor } from "../../../stores/notifications/NotificationColor"; +import { NotificationState } from "../../../stores/notifications/NotificationState"; interface IProps { isMinimized: boolean; isSelected: boolean; displayName: string; avatar: React.ReactElement; - notificationState: INotificationState; + notificationState: NotificationState; onClick: () => void; } @@ -74,7 +73,7 @@ export default class TemporaryTile extends React.Component { const nameClasses = classNames({ "mx_RoomTile2_name": true, - "mx_RoomTile2_nameHasUnreadEvents": this.props.notificationState.color >= NotificationColor.Bold, + "mx_RoomTile2_nameHasUnreadEvents": this.props.notificationState.isUnread, }); let nameContainer = ( diff --git a/src/stores/notifications/INotificationState.ts b/src/stores/notifications/INotificationState.ts deleted file mode 100644 index 65bd7b7957..0000000000 --- a/src/stores/notifications/INotificationState.ts +++ /dev/null @@ -1,26 +0,0 @@ -/* -Copyright 2020 The Matrix.org Foundation C.I.C. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -import { EventEmitter } from "events"; -import { NotificationColor } from "./NotificationColor"; - -export const NOTIFICATION_STATE_UPDATE = "update"; - -export interface INotificationState extends EventEmitter { - symbol?: string; - count: number; - color: NotificationColor; -} diff --git a/src/stores/notifications/ListNotificationState.ts b/src/stores/notifications/ListNotificationState.ts index 5773693b47..6c5f6fc6dd 100644 --- a/src/stores/notifications/ListNotificationState.ts +++ b/src/stores/notifications/ListNotificationState.ts @@ -14,23 +14,20 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { EventEmitter } from "events"; -import { INotificationState, NOTIFICATION_STATE_UPDATE } from "./INotificationState"; import { NotificationColor } from "./NotificationColor"; -import { IDestroyable } from "../../utils/IDestroyable"; import { TagID } from "../room-list/models"; import { Room } from "matrix-js-sdk/src/models/room"; import { arrayDiff } from "../../utils/arrays"; import { RoomNotificationState } from "./RoomNotificationState"; -import { TagSpecificNotificationState } from "./TagSpecificNotificationState"; +import { NOTIFICATION_STATE_UPDATE, NotificationState } from "./NotificationState"; -export class ListNotificationState extends EventEmitter implements IDestroyable, INotificationState { - private _count: number; - private _color: NotificationColor; +export type FetchRoomFn = (room: Room) => RoomNotificationState; + +export class ListNotificationState extends NotificationState { private rooms: Room[] = []; private states: { [roomId: string]: RoomNotificationState } = {}; - constructor(private byTileCount = false, private tagId: TagID) { + constructor(private byTileCount = false, private tagId: TagID, private getRoomFn: FetchRoomFn) { super(); } @@ -38,14 +35,6 @@ export class ListNotificationState extends EventEmitter implements IDestroyable, return null; // This notification state doesn't support symbols } - public get count(): number { - return this._count; - } - - public get color(): NotificationColor { - return this._color; - } - public setRooms(rooms: Room[]) { // If we're only concerned about the tile count, don't bother setting up listeners. if (this.byTileCount) { @@ -62,10 +51,9 @@ export class ListNotificationState extends EventEmitter implements IDestroyable, if (!state) continue; // We likely just didn't have a badge (race condition) delete this.states[oldRoom.roomId]; state.off(NOTIFICATION_STATE_UPDATE, this.onRoomNotificationStateUpdate); - state.destroy(); } for (const newRoom of diff.added) { - const state = new TagSpecificNotificationState(newRoom, this.tagId); + const state = this.getRoomFn(newRoom); state.on(NOTIFICATION_STATE_UPDATE, this.onRoomNotificationStateUpdate); if (this.states[newRoom.roomId]) { // "Should never happen" disclaimer. @@ -85,8 +73,9 @@ export class ListNotificationState extends EventEmitter implements IDestroyable, } public destroy() { + super.destroy(); for (const state of Object.values(this.states)) { - state.destroy(); + state.off(NOTIFICATION_STATE_UPDATE, this.onRoomNotificationStateUpdate); } this.states = {}; } @@ -96,7 +85,7 @@ export class ListNotificationState extends EventEmitter implements IDestroyable, }; private calculateTotalState() { - const before = {count: this.count, symbol: this.symbol, color: this.color}; + const snapshot = this.snapshot(); if (this.byTileCount) { this._color = NotificationColor.Red; @@ -111,10 +100,7 @@ export class ListNotificationState extends EventEmitter implements IDestroyable, } // finally, publish an update if needed - const after = {count: this.count, symbol: this.symbol, color: this.color}; - if (JSON.stringify(before) !== JSON.stringify(after)) { - this.emit(NOTIFICATION_STATE_UPDATE); - } + this.emitIfUpdated(snapshot); } } diff --git a/src/stores/notifications/NotificationState.ts b/src/stores/notifications/NotificationState.ts new file mode 100644 index 0000000000..c8ef0ba859 --- /dev/null +++ b/src/stores/notifications/NotificationState.ts @@ -0,0 +1,87 @@ +/* +Copyright 2020 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import { EventEmitter } from "events"; +import { NotificationColor } from "./NotificationColor"; +import { IDestroyable } from "../../utils/IDestroyable"; + +export const NOTIFICATION_STATE_UPDATE = "update"; + +export abstract class NotificationState extends EventEmitter implements IDestroyable { + protected _symbol: string; + protected _count: number; + protected _color: NotificationColor; + + public get symbol(): string { + return this._symbol; + } + + public get count(): number { + return this._count; + } + + public get color(): NotificationColor { + return this._color; + } + + public get isIdle(): boolean { + return this.color <= NotificationColor.None; + } + + public get isUnread(): boolean { + return this.color >= NotificationColor.Bold; + } + + public get hasUnreadCount(): boolean { + return this.color >= NotificationColor.Grey && (!!this.count || !!this.symbol); + } + + public get hasMentions(): boolean { + return this.color >= NotificationColor.Red; + } + + protected emitIfUpdated(snapshot: NotificationStateSnapshot) { + if (snapshot.isDifferentFrom(this)) { + this.emit(NOTIFICATION_STATE_UPDATE); + } + } + + protected snapshot(): NotificationStateSnapshot { + return new NotificationStateSnapshot(this); + } + + public destroy(): void { + this.removeAllListeners(NOTIFICATION_STATE_UPDATE); + } +} + +export class NotificationStateSnapshot { + private readonly symbol: string; + private readonly count: number; + private readonly color: NotificationColor; + + constructor(state: NotificationState) { + this.symbol = state.symbol; + this.count = state.count; + this.color = state.color; + } + + public isDifferentFrom(other: NotificationState): boolean { + const before = {count: this.count, symbol: this.symbol, color: this.color}; + const after = {count: other.count, symbol: other.symbol, color: other.color}; + return JSON.stringify(before) !== JSON.stringify(after); + } +} diff --git a/src/stores/notifications/RoomNotificationState.ts b/src/stores/notifications/RoomNotificationState.ts index 51355a2d4d..ab354c0e93 100644 --- a/src/stores/notifications/RoomNotificationState.ts +++ b/src/stores/notifications/RoomNotificationState.ts @@ -14,8 +14,6 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { EventEmitter } from "events"; -import { INotificationState, NOTIFICATION_STATE_UPDATE } from "./INotificationState"; import { NotificationColor } from "./NotificationColor"; import { IDestroyable } from "../../utils/IDestroyable"; import { MatrixClientPeg } from "../../MatrixClientPeg"; @@ -25,12 +23,9 @@ import { MatrixEvent } from "matrix-js-sdk/src/models/event"; import { Room } from "matrix-js-sdk/src/models/room"; import * as RoomNotifs from '../../RoomNotifs'; import * as Unread from '../../Unread'; +import { NotificationState } from "./NotificationState"; -export class RoomNotificationState extends EventEmitter implements IDestroyable, INotificationState { - private _symbol: string; - private _count: number; - private _color: NotificationColor; - +export class RoomNotificationState extends NotificationState implements IDestroyable { constructor(public readonly room: Room) { super(); this.room.on("Room.receipt", this.handleReadReceipt); @@ -41,23 +36,12 @@ export class RoomNotificationState extends EventEmitter implements IDestroyable, this.updateNotificationState(); } - public get symbol(): string { - return this._symbol; - } - - public get count(): number { - return this._count; - } - - public get color(): NotificationColor { - return this._color; - } - private get roomIsInvite(): boolean { return getEffectiveMembership(this.room.getMyMembership()) === EffectiveMembership.Invite; } public destroy(): void { + super.destroy(); this.room.removeListener("Room.receipt", this.handleReadReceipt); this.room.removeListener("Room.timeline", this.handleRoomEventUpdate); this.room.removeListener("Room.redaction", this.handleRoomEventUpdate); @@ -87,7 +71,7 @@ export class RoomNotificationState extends EventEmitter implements IDestroyable, }; private updateNotificationState() { - const before = {count: this.count, symbol: this.symbol, color: this.color}; + const snapshot = this.snapshot(); if (RoomNotifs.getRoomNotifsState(this.room.roomId) === RoomNotifs.MUTE) { // When muted we suppress all notification states, even if we have context on them. @@ -136,9 +120,6 @@ export class RoomNotificationState extends EventEmitter implements IDestroyable, } // finally, publish an update if needed - const after = {count: this.count, symbol: this.symbol, color: this.color}; - if (JSON.stringify(before) !== JSON.stringify(after)) { - this.emit(NOTIFICATION_STATE_UPDATE); - } + this.emitIfUpdated(snapshot); } } diff --git a/src/stores/notifications/RoomNotificationStateStore.ts b/src/stores/notifications/RoomNotificationStateStore.ts new file mode 100644 index 0000000000..311dcdf2d6 --- /dev/null +++ b/src/stores/notifications/RoomNotificationStateStore.ts @@ -0,0 +1,101 @@ +/* +Copyright 2020 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import { ActionPayload } from "../../dispatcher/payloads"; +import { AsyncStoreWithClient } from "../AsyncStoreWithClient"; +import defaultDispatcher from "../../dispatcher/dispatcher"; +import { DefaultTagID, TagID } from "../room-list/models"; +import { FetchRoomFn, ListNotificationState } from "./ListNotificationState"; +import { Room } from "matrix-js-sdk/src/models/room"; +import { RoomNotificationState } from "./RoomNotificationState"; +import { TagSpecificNotificationState } from "./TagSpecificNotificationState"; + +const INSPECIFIC_TAG = "INSPECIFIC_TAG"; +type INSPECIFIC_TAG = "INSPECIFIC_TAG"; + +interface IState {} + +export class RoomNotificationStateStore extends AsyncStoreWithClient { + private static internalInstance = new RoomNotificationStateStore(); + + private roomMap = new Map>(); + + private constructor() { + super(defaultDispatcher, {}); + } + + /** + * Creates a new list notification state. The consumer is expected to set the rooms + * on the notification state, and destroy the state when it no longer needs it. + * @param tagId The tag to create the notification state for. + * @returns The notification state for the tag. + */ + public getListState(tagId: TagID): ListNotificationState { + // Note: we don't cache these notification states as the consumer is expected to call + // .setRooms() on the returned object, which could confuse other consumers. + + // TODO: Update if/when invites move out of the room list. + const useTileCount = tagId === DefaultTagID.Invite; + const getRoomFn: FetchRoomFn = (room: Room) => { + return this.getRoomState(room, tagId); + }; + return new ListNotificationState(useTileCount, tagId, getRoomFn); + } + + /** + * Gets a copy of the notification state for a room. The consumer should not + * attempt to destroy the returned state as it may be shared with other + * consumers. + * @param room The room to get the notification state for. + * @param inTagId Optional tag ID to scope the notification state to. + * @returns The room's notification state. + */ + public getRoomState(room: Room, inTagId?: TagID): RoomNotificationState { + if (!this.roomMap.has(room)) { + this.roomMap.set(room, new Map()); + } + + const targetTag = inTagId ? inTagId : INSPECIFIC_TAG; + + const forRoomMap = this.roomMap.get(room); + if (!forRoomMap.has(targetTag)) { + if (inTagId) { + forRoomMap.set(inTagId, new TagSpecificNotificationState(room, inTagId)); + } else { + forRoomMap.set(INSPECIFIC_TAG, new RoomNotificationState(room)); + } + } + + return forRoomMap.get(targetTag); + } + + public static get instance(): RoomNotificationStateStore { + return RoomNotificationStateStore.internalInstance; + } + + protected async onNotReady(): Promise { + for (const roomMap of this.roomMap.values()) { + for (const roomState of roomMap.values()) { + roomState.destroy(); + } + } + } + + // We don't need this, but our contract says we do. + protected async onAction(payload: ActionPayload) { + return Promise.resolve(); + } +} diff --git a/src/stores/notifications/StaticNotificationState.ts b/src/stores/notifications/StaticNotificationState.ts index 51902688fe..0392ed3716 100644 --- a/src/stores/notifications/StaticNotificationState.ts +++ b/src/stores/notifications/StaticNotificationState.ts @@ -14,13 +14,15 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { EventEmitter } from "events"; -import { INotificationState } from "./INotificationState"; import { NotificationColor } from "./NotificationColor"; +import { NotificationState } from "./NotificationState"; -export class StaticNotificationState extends EventEmitter implements INotificationState { - constructor(public symbol: string, public count: number, public color: NotificationColor) { +export class StaticNotificationState extends NotificationState { + constructor(symbol: string, count: number, color: NotificationColor) { super(); + this._symbol = symbol; + this._count = count; + this._color = color; } public static forCount(count: number, color: NotificationColor): StaticNotificationState { From 1033eda7fb8ca7ab4187e7c5a2a8ee080319522a Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Thu, 9 Jul 2020 15:54:44 +0100 Subject: [PATCH 16/16] Move irc layout option to advanced --- .../settings/tabs/user/AppearanceUserSettingsTab.tsx | 7 ++++++- src/i18n/strings/en_EN.json | 3 +-- src/settings/Settings.js | 8 +------- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/components/views/settings/tabs/user/AppearanceUserSettingsTab.tsx b/src/components/views/settings/tabs/user/AppearanceUserSettingsTab.tsx index 9bed2fb039..3cb0028f45 100644 --- a/src/components/views/settings/tabs/user/AppearanceUserSettingsTab.tsx +++ b/src/components/views/settings/tabs/user/AppearanceUserSettingsTab.tsx @@ -402,6 +402,12 @@ export default class AppearanceUserSettingsTab extends React.Component + this.setState({useIRCLayout: checked})} + /> {this.renderThemeSection()} {SettingsStore.isFeatureEnabled("feature_font_scaling") ? this.renderFontSection() : null} - {SettingsStore.isFeatureEnabled("feature_irc_ui") ? this.renderLayoutSection() : null} {this.renderAdvancedSection()}
); diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index a38d87d08e..fb97bfa26c 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -489,7 +489,6 @@ "Try out new ways to ignore people (experimental)": "Try out new ways to ignore people (experimental)", "Use the improved room list (will refresh to apply changes)": "Use the improved room list (will refresh to apply changes)", "Support adding custom themes": "Support adding custom themes", - "Enable IRC layout option in the appearance tab": "Enable IRC layout option in the appearance tab", "Show info about bridges in room settings": "Show info about bridges in room settings", "Font size": "Font size", "Use custom size": "Use custom size", @@ -539,7 +538,7 @@ "How fast should messages be downloaded.": "How fast should messages be downloaded.", "Manually verify all remote sessions": "Manually verify all remote sessions", "IRC display name width": "IRC display name width", - "Use IRC layout": "Use IRC layout", + "Enable experimental, compact IRC style layout": "Enable experimental, compact IRC style layout", "Collecting app version information": "Collecting app version information", "Collecting logs": "Collecting logs", "Uploading report": "Uploading report", diff --git a/src/settings/Settings.js b/src/settings/Settings.js index 0011ecfccd..5f84b11247 100644 --- a/src/settings/Settings.js +++ b/src/settings/Settings.js @@ -159,12 +159,6 @@ export const SETTINGS = { supportedLevels: LEVELS_FEATURE, default: false, }, - "feature_irc_ui": { - supportedLevels: LEVELS_ACCOUNT_SETTINGS, - displayName: _td('Enable IRC layout option in the appearance tab'), - default: false, - isFeature: true, - }, "mjolnirRooms": { supportedLevels: ['account'], default: [], @@ -574,7 +568,7 @@ export const SETTINGS = { }, "useIRCLayout": { supportedLevels: LEVELS_ACCOUNT_SETTINGS, - displayName: _td("Use IRC layout"), + displayName: _td("Enable experimental, compact IRC style layout"), default: false, }, };