From cea75fde277ab051a23435e14fbfbeddab4cfef2 Mon Sep 17 00:00:00 2001 From: Suguru Hirahara Date: Mon, 2 May 2022 01:08:42 +0000 Subject: [PATCH 01/13] Fix text link buttons on UserInfo panel (#8247) * Fix text link buttons on UserInfo (right) panel - Fix link button styling - Replace className="mx_linkButton" with kind="link" - Remove style rules required for mx_linkButton - Align E2E icon and devices on the device list - Replace margin with gap Signed-off-by: Suguru Hirahara * Spacing variables Signed-off-by: Suguru Hirahara * Replace link_inline with link Signed-off-by: Suguru Hirahara * Remove a redundant rule Signed-off-by: Suguru Hirahara * Wrap verifyButton Signed-off-by: Suguru Hirahara --- res/css/views/right_panel/_UserInfo.scss | 27 +++---- src/components/views/right_panel/UserInfo.tsx | 70 +++++++++++++++---- 2 files changed, 71 insertions(+), 26 deletions(-) diff --git a/res/css/views/right_panel/_UserInfo.scss b/res/css/views/right_panel/_UserInfo.scss index 3b7a51797f9..15cf0cdc1ec 100644 --- a/res/css/views/right_panel/_UserInfo.scss +++ b/res/css/views/right_panel/_UserInfo.scss @@ -52,6 +52,10 @@ limitations under the License. .mx_UserInfo_container { padding: 8px 16px; + + .mx_UserInfo_container_verifyButton { + margin-top: $spacing-8; + } } .mx_UserInfo_separator { @@ -193,10 +197,7 @@ limitations under the License. } .mx_UserInfo_field { - cursor: pointer; - color: $accent; line-height: $font-16px; - margin: 8px 0; &.mx_UserInfo_destructive { color: $alert; @@ -228,14 +229,18 @@ limitations under the License. padding-bottom: 0; > :not(h3) { - margin-left: 8px; + margin-inline-start: $spacing-8; + display: flex; + flex-flow: column; + align-items: flex-start; + row-gap: $spacing-8; } } .mx_UserInfo_devices { .mx_UserInfo_device { display: flex; - margin: 8px 0; + margin: $spacing-8 0; &.mx_UserInfo_device_verified { .mx_UserInfo_device_trusted { @@ -250,7 +255,7 @@ limitations under the License. .mx_UserInfo_device_name { flex: 1; - margin-right: 5px; + margin: 0 5px; word-break: break-word; } } @@ -259,20 +264,16 @@ limitations under the License. .mx_E2EIcon { // don't squeeze flex: 0 0 auto; - margin: 2px 5px 0 0; + margin: 0; width: 12px; height: 12px; } .mx_UserInfo_expand { - display: flex; - margin-top: 11px; + column-gap: 5px; // cf: mx_UserInfo_device_name + margin-bottom: 11px; } } - - .mx_AccessibleButton.mx_AccessibleButton_hasKind { - padding: 8px 18px; - } } .mx_UserInfo.mx_UserInfo_smallAvatar { diff --git a/src/components/views/right_panel/UserInfo.tsx b/src/components/views/right_panel/UserInfo.tsx index c9685c24670..9d17c5237d1 100644 --- a/src/components/views/right_panel/UserInfo.tsx +++ b/src/components/views/right_panel/UserInfo.tsx @@ -292,13 +292,17 @@ function DevicesSection({ devices, userId, loading }: { devices: IDevice[], user let expandButton; if (expandSectionDevices.length) { if (isExpanded) { - expandButton = ( setExpanded(false)} >
{ expandHideCaption }
); } else { - expandButton = ( setExpanded(true)} >
@@ -331,6 +335,7 @@ const MessageButton = ({ userId }: { userId: string }) => { return ( { if (busy) return; setBusy(true); @@ -383,6 +388,7 @@ const UserOptionsSection: React.FC<{ ignoreButton = ( @@ -413,14 +419,22 @@ const UserOptionsSection: React.FC<{ const room = cli.getRoom(member.roomId); if (room?.getEventReadUpTo(member.userId)) { readReceiptButton = ( - + { _t('Jump to read receipt') } ); } insertPillButton = ( - + { _t('Mention') } ); @@ -448,7 +462,11 @@ const UserOptionsSection: React.FC<{ }; inviteUserButton = ( - + { _t('Invite') } ); @@ -456,7 +474,11 @@ const UserOptionsSection: React.FC<{ } const shareUserButton = ( - + { _t('Share Link to User') } ); @@ -624,7 +646,11 @@ const RoomKickButton = ({ room, member, startUpdating, stopUpdating }: Omit + return { kickLabel } ; }; @@ -642,7 +668,11 @@ const RedactMessagesButton: React.FC = ({ member }) => { }); }; - return + return { _t("Remove recent messages") } ; }; @@ -739,7 +769,11 @@ const BanToggleButton = ({ room, member, startUpdating, stopUpdating }: Omit + return { label } ; }; @@ -809,7 +843,11 @@ const MuteToggleButton: React.FC = ({ member, room, powerLevels, }); const muteLabel = muted ? _t("Unmute") : _t("Mute"); - return + return { muteLabel } ; }; @@ -1212,7 +1250,11 @@ const BasicUserInfo: React.FC<{ // FIXME this should be using cli instead of MatrixClientPeg.matrixClient if (isSynapseAdmin && member.userId.endsWith(`:${MatrixClientPeg.getHomeserverName()}`)) { synapseDeactivateButton = ( - + { _t("Deactivate user") } ); @@ -1290,8 +1332,9 @@ const BasicUserInfo: React.FC<{ if (canVerify) { if (hasCrossSigningKeys !== undefined) { // Note: mx_UserInfo_verifyButton is for the end-to-end tests - verifyButton = ( + verifyButton = (
{ if (hasCrossSigningKeys) { @@ -1303,7 +1346,7 @@ const BasicUserInfo: React.FC<{ > { _t("Verify") } - ); +
); } else if (!showDeviceListSpinner) { // HACK: only show a spinner if the device section spinner is not shown, // to avoid showing a double spinner @@ -1316,6 +1359,7 @@ const BasicUserInfo: React.FC<{ if (member.userId == cli.getUserId()) { editDevices = (
{ dis.dispatch({ From d294dad04da5e7126f705a1fea72f557c83e43ea Mon Sep 17 00:00:00 2001 From: Emmanuel <63562663+EECvision@users.noreply.github.com> Date: Mon, 2 May 2022 02:09:59 +0100 Subject: [PATCH 02/13] fix timeline search with empty text box should do nothing (#8262) * fix timeline search with empty text box should do nothing * test SearchBar component * fix lint error * Update SearchBar-test.tsx Co-authored-by: Travis Ralston --- src/components/views/rooms/SearchBar.tsx | 1 + .../components/views/rooms/SearchBar-test.tsx | 134 ++++++++++++++++++ 2 files changed, 135 insertions(+) create mode 100644 test/components/views/rooms/SearchBar-test.tsx diff --git a/src/components/views/rooms/SearchBar.tsx b/src/components/views/rooms/SearchBar.tsx index c42131bba04..20a9e081a0c 100644 --- a/src/components/views/rooms/SearchBar.tsx +++ b/src/components/views/rooms/SearchBar.tsx @@ -78,6 +78,7 @@ export default class SearchBar extends React.Component { } private onSearch = (): void => { + if (!this.searchTerm.current.value.trim()) return; this.props.onSearch(this.searchTerm.current.value, this.state.scope); }; diff --git a/test/components/views/rooms/SearchBar-test.tsx b/test/components/views/rooms/SearchBar-test.tsx new file mode 100644 index 00000000000..b72f838bb9a --- /dev/null +++ b/test/components/views/rooms/SearchBar-test.tsx @@ -0,0 +1,134 @@ +/* +Copyright 2022 Emmanuel Ezeka + +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 React from 'react'; +import { mount } from "enzyme"; + +import DesktopBuildsNotice from "../../../../src/components/views/elements/DesktopBuildsNotice"; +import { PosthogScreenTracker } from "../../../../src/PosthogTrackers"; +import SearchBar, { SearchScope } from "../../../../src/components/views/rooms/SearchBar"; +import { KeyBindingAction } from "../../../../src/accessibility/KeyboardShortcuts"; + +let mockCurrentEvent = KeyBindingAction.Enter; +const mockWarningKind = true; +let wrapper: any = null; + +const searchProps = { + onCancelClick: jest.fn(), + onSearch: jest.fn(), + searchInProgress: false, + isRoomEncrypted: false, +}; + +jest.mock("../../../../src/KeyBindingsManager", () => ({ + __esModule: true, + getKeyBindingsManager: jest.fn(() => ( + { getAccessibilityAction: jest.fn(() => mockCurrentEvent) })), +})); + +/** mock out DesktopBuildsNotice component so it doesn't affect the result of our test */ +jest.mock('../../../../src/components/views/elements/DesktopBuildsNotice', () => ({ + __esModule: true, + WarningKind: { + get Search() { // eslint-disable-line @typescript-eslint/naming-convention + return mockWarningKind; + }, + }, + default: jest.fn(({ children }) => ( +
{ children }
+ )), +})); + +/** mock out PosthogTrackers component so it doesn't affect the result of our test */ +jest.mock('../../../../src/PosthogTrackers', () => ({ + __esModule: true, + PosthogScreenTracker: jest.fn(({ children }) => ( +
{ children }
+ )), +})); + +describe("SearchBar", () => { + beforeEach(() => { + wrapper = mount(); + }); + + afterEach(() => { + wrapper.unmount(); + searchProps.onCancelClick.mockClear(); + searchProps.onSearch.mockClear(); + }); + + it("must render child components and pass necessary props", () => { + const postHogScreenTracker = wrapper.find(PosthogScreenTracker); + const desktopBuildNotice = wrapper.find(DesktopBuildsNotice); + + expect(postHogScreenTracker.length).toBe(1); + expect(desktopBuildNotice.length).toBe(1); + expect(postHogScreenTracker.props().screenName).toEqual("RoomSearch"); + expect(desktopBuildNotice.props().isRoomEncrypted).toEqual(searchProps.isRoomEncrypted); + expect(desktopBuildNotice.props().kind).toEqual(mockWarningKind); + }); + + it("must not search when input value is empty", () => { + const roomButtons = wrapper.find(".mx_SearchBar_button"); + const searchButton = wrapper.find(".mx_SearchBar_searchButton"); + + expect(roomButtons.length).toEqual(4); + + searchButton.at(0).simulate("click"); + roomButtons.at(0).simulate("click"); + roomButtons.at(2).simulate("click"); + + expect(searchProps.onSearch).not.toHaveBeenCalled(); + }); + + it("must trigger onSearch when value is not empty", () => { + const searchValue = "abcd"; + + const roomButtons = wrapper.find(".mx_SearchBar_button"); + const searchButton = wrapper.find(".mx_SearchBar_searchButton"); + const input = wrapper.find(".mx_SearchBar_input input"); + input.instance().value = searchValue; + + expect(roomButtons.length).toEqual(4); + + searchButton.at(0).simulate("click"); + + expect(searchProps.onSearch).toHaveBeenCalledTimes(1); + expect(searchProps.onSearch).toHaveBeenNthCalledWith(1, searchValue, SearchScope.Room); + + roomButtons.at(0).simulate("click"); + + expect(searchProps.onSearch).toHaveBeenCalledTimes(2); + expect(searchProps.onSearch).toHaveBeenNthCalledWith(2, searchValue, SearchScope.Room); + + roomButtons.at(2).simulate("click"); + + expect(searchProps.onSearch).toHaveBeenCalledTimes(3); + expect(searchProps.onSearch).toHaveBeenNthCalledWith(3, searchValue, SearchScope.All); + }); + + it("cancel button and esc key should trigger onCancelClick", () => { + mockCurrentEvent = KeyBindingAction.Escape; + const cancelButton = wrapper.find(".mx_SearchBar_cancel"); + const input = wrapper.find(".mx_SearchBar_input input"); + input.simulate("focus"); + input.simulate("keydown", { key: "ESC" }); + cancelButton.at(0).simulate("click"); + + expect(searchProps.onCancelClick).toHaveBeenCalledTimes(2); + }); +}); From 3a245a0cbe59f17180671de28450ba75e08b14cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Mon, 2 May 2022 03:38:36 +0200 Subject: [PATCH 03/13] Fix jump to bottom button being always displayed in non-overflowing timelines (#8460) --- src/components/structures/RoomView.tsx | 11 +++-------- src/components/structures/TimelinePanel.tsx | 2 +- src/contexts/RoomContext.ts | 2 -- .../views/rooms/MessageComposerButtons-test.tsx | 1 - 4 files changed, 4 insertions(+), 12 deletions(-) diff --git a/src/components/structures/RoomView.tsx b/src/components/structures/RoomView.tsx index 761dd9b496f..956fee0060c 100644 --- a/src/components/structures/RoomView.tsx +++ b/src/components/structures/RoomView.tsx @@ -179,9 +179,7 @@ export interface IRoomState { // this is true if we are fully scrolled-down, and are looking at // the end of the live timeline. It has the effect of hiding the // 'scroll to bottom' knob, among a couple of other things. - atEndOfLiveTimeline: boolean; - // used by componentDidUpdate to avoid unnecessary checks - atEndOfLiveTimelineInit: boolean; + atEndOfLiveTimeline?: boolean; showTopUnreadMessagesBar: boolean; statusBarVisible: boolean; // We load this later by asking the js-sdk to suggest a version for us. @@ -257,8 +255,6 @@ export class RoomView extends React.Component { isPeeking: false, showRightPanel: false, joining: false, - atEndOfLiveTimeline: true, - atEndOfLiveTimelineInit: false, showTopUnreadMessagesBar: false, statusBarVisible: false, canReact: false, @@ -692,9 +688,8 @@ export class RoomView extends React.Component { // in render() prevents the ref from being set on first mount, so we try and // catch the messagePanel when it does mount. Because we only want the ref once, // we use a boolean flag to avoid duplicate work. - if (this.messagePanel && !this.state.atEndOfLiveTimelineInit) { + if (this.messagePanel && this.state.atEndOfLiveTimeline === undefined) { this.setState({ - atEndOfLiveTimelineInit: true, atEndOfLiveTimeline: this.messagePanel.isAtEndOfLiveTimeline(), }); } @@ -2102,7 +2097,7 @@ export class RoomView extends React.Component { } let jumpToBottom; // Do not show JumpToBottomButton if we have search results showing, it makes no sense - if (!this.state.atEndOfLiveTimeline && !this.state.searchResults) { + if (this.state.atEndOfLiveTimeline === false && !this.state.searchResults) { jumpToBottom = ( 0} numUnreadMessages={this.state.numUnreadMessages} diff --git a/src/components/structures/TimelinePanel.tsx b/src/components/structures/TimelinePanel.tsx index 7201e3c6f2e..59b87c639ac 100644 --- a/src/components/structures/TimelinePanel.tsx +++ b/src/components/structures/TimelinePanel.tsx @@ -1043,7 +1043,7 @@ class TimelinePanel extends React.Component { /* return true if the content is fully scrolled down and we are * at the end of the live timeline. */ - public isAtEndOfLiveTimeline = (): boolean => { + public isAtEndOfLiveTimeline = (): boolean | undefined => { return this.messagePanel.current?.isAtBottom() && this.timelineWindow && !this.timelineWindow.canPaginate(EventTimeline.FORWARDS); diff --git a/src/contexts/RoomContext.ts b/src/contexts/RoomContext.ts index 9c44553a983..cbf56e8a4a8 100644 --- a/src/contexts/RoomContext.ts +++ b/src/contexts/RoomContext.ts @@ -41,8 +41,6 @@ const RoomContext = createContext({ isPeeking: false, showRightPanel: true, joining: false, - atEndOfLiveTimeline: true, - atEndOfLiveTimelineInit: false, showTopUnreadMessagesBar: false, statusBarVisible: false, canReact: false, diff --git a/test/components/views/rooms/MessageComposerButtons-test.tsx b/test/components/views/rooms/MessageComposerButtons-test.tsx index 4abe3e36373..1ec08e455d0 100644 --- a/test/components/views/rooms/MessageComposerButtons-test.tsx +++ b/test/components/views/rooms/MessageComposerButtons-test.tsx @@ -216,7 +216,6 @@ function createRoomState(room: Room, narrow: boolean): IRoomState { showRightPanel: true, joining: false, atEndOfLiveTimeline: true, - atEndOfLiveTimelineInit: false, showTopUnreadMessagesBar: false, statusBarVisible: false, canReact: false, From 633229ca26382f2372b2140c59b452d3ab86bec6 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 2 May 2022 03:23:43 +0100 Subject: [PATCH 04/13] Clear local storage settings handler cache on logout (#8454) --- src/Lifecycle.ts | 2 + .../AbstractLocalStorageSettingsHandler.ts | 56 +++++++++++-------- 2 files changed, 36 insertions(+), 22 deletions(-) diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index f91158c38aa..6b7268d57ed 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -61,6 +61,7 @@ import { setSentryUser } from "./sentry"; import SdkConfig from "./SdkConfig"; import { DialogOpener } from "./utils/DialogOpener"; import { Action } from "./dispatcher/actions"; +import AbstractLocalStorageSettingsHandler from "./settings/handlers/AbstractLocalStorageSettingsHandler"; const HOMESERVER_URL_KEY = "mx_hs_url"; const ID_SERVER_URL_KEY = "mx_is_url"; @@ -878,6 +879,7 @@ async function clearStorage(opts?: { deleteEverything?: boolean }): Promise(); - private objectCache = new Map(); + // Shared cache between all subclass instances + private static itemCache = new Map(); + private static objectCache = new Map(); + private static storageListenerBound = false; + + private static onStorageEvent = (e: StorageEvent) => { + if (e.key === null) { + AbstractLocalStorageSettingsHandler.clear(); + } else { + AbstractLocalStorageSettingsHandler.itemCache.delete(e.key); + AbstractLocalStorageSettingsHandler.objectCache.delete(e.key); + } + }; + + // Expose the clear event for Lifecycle to call, the storage listener only fires for changes from other tabs + public static clear() { + AbstractLocalStorageSettingsHandler.itemCache.clear(); + AbstractLocalStorageSettingsHandler.objectCache.clear(); + } protected constructor() { super(); - // Listen for storage changes from other tabs to bust the cache - window.addEventListener("storage", (e: StorageEvent) => { - if (e.key === null) { - this.itemCache.clear(); - this.objectCache.clear(); - } else { - this.itemCache.delete(e.key); - this.objectCache.delete(e.key); - } - }); + if (!AbstractLocalStorageSettingsHandler.storageListenerBound) { + AbstractLocalStorageSettingsHandler.storageListenerBound = true; + // Listen for storage changes from other tabs to bust the cache + window.addEventListener("storage", AbstractLocalStorageSettingsHandler.onStorageEvent); + } } protected getItem(key: string): any { - if (!this.itemCache.has(key)) { + if (!AbstractLocalStorageSettingsHandler.itemCache.has(key)) { const value = localStorage.getItem(key); - this.itemCache.set(key, value); + AbstractLocalStorageSettingsHandler.itemCache.set(key, value); return value; } - return this.itemCache.get(key); + return AbstractLocalStorageSettingsHandler.itemCache.get(key); } protected getObject(key: string): T | null { - if (!this.objectCache.has(key)) { + if (!AbstractLocalStorageSettingsHandler.objectCache.has(key)) { try { const value = JSON.parse(localStorage.getItem(key)); - this.objectCache.set(key, value); + AbstractLocalStorageSettingsHandler.objectCache.set(key, value); return value; } catch (err) { console.error("Failed to parse localStorage object", err); @@ -61,24 +73,24 @@ export default abstract class AbstractLocalStorageSettingsHandler extends Settin } } - return this.objectCache.get(key) as T; + return AbstractLocalStorageSettingsHandler.objectCache.get(key) as T; } protected setItem(key: string, value: any): void { - this.itemCache.set(key, value); + AbstractLocalStorageSettingsHandler.itemCache.set(key, value); localStorage.setItem(key, value); } protected setObject(key: string, value: object): void { - this.objectCache.set(key, value); + AbstractLocalStorageSettingsHandler.objectCache.set(key, value); localStorage.setItem(key, JSON.stringify(value)); } // handles both items and objects protected removeItem(key: string): void { localStorage.removeItem(key); - this.itemCache.delete(key); - this.objectCache.delete(key); + AbstractLocalStorageSettingsHandler.itemCache.delete(key); + AbstractLocalStorageSettingsHandler.objectCache.delete(key); } public isSupported(): boolean { From 4a04be6d1c9370a923bc45c6ebc8f42e30b22447 Mon Sep 17 00:00:00 2001 From: Kerry Date: Mon, 2 May 2022 09:57:35 +0200 Subject: [PATCH 05/13] Test typescriptification continued (#8327) * test/utils/MegolmExportEncryption-test.js -> test/utils/MegolmExportEncryption-test.ts Signed-off-by: Kerry Archibald * test/utils/ShieldUtils-test.js - test/utils/ShieldUtils-test.ts Signed-off-by: Kerry Archibald * type fixes for ShieldUtils-test Signed-off-by: Kerry Archibald * test/DecryptionFailureTracker-test.js -> test/DecryptionFailureTracker-test.ts Signed-off-by: Kerry Archibald * remove unsupported assertion failure messages Signed-off-by: Kerry Archibald * fix ts issues in DecryptionFailureTracker Signed-off-by: Kerry Archibald * add mock restores Signed-off-by: Kerry Archibald * newline Signed-off-by: Kerry Archibald * remove commented decriptionfailuretracker code and test Signed-off-by: Kerry Archibald * make should aggregate error codes correctly pass Signed-off-by: Kerry Archibald * cheaters types in MegolmExportEncryption Signed-off-by: Kerry Archibald * lint Signed-off-by: Kerry Archibald * Revert "fix ts issues in DecryptionFailureTracker" This reverts commit 1ae748cc51088d60722320dbefae04a62310e2e1. Signed-off-by: Kerry Archibald * Revert "remove unsupported assertion failure messages" This reverts commit 7bd93d075c4d8d45befcbfd59c889782c9a44b48. * Revert "test/DecryptionFailureTracker-test.js -> test/DecryptionFailureTracker-test.ts" This reverts commit 1670025bd2af9a355c2761998202f602d61f242e. * revert change to DecryptionFailureTracker Signed-off-by: Kerry Archibald --- ...test.js => MegolmExportEncryption-test.ts} | 10 +++- ...hieldUtils-test.js => ShieldUtils-test.ts} | 59 ++++++++++++++----- 2 files changed, 50 insertions(+), 19 deletions(-) rename test/utils/{MegolmExportEncryption-test.js => MegolmExportEncryption-test.ts} (95%) rename test/utils/{ShieldUtils-test.js => ShieldUtils-test.ts} (83%) diff --git a/test/utils/MegolmExportEncryption-test.js b/test/utils/MegolmExportEncryption-test.ts similarity index 95% rename from test/utils/MegolmExportEncryption-test.js rename to test/utils/MegolmExportEncryption-test.ts index 8b16085efcc..4fb92a8f8bb 100644 --- a/test/utils/MegolmExportEncryption-test.js +++ b/test/utils/MegolmExportEncryption-test.ts @@ -20,7 +20,8 @@ import { Crypto } from "@peculiar/webcrypto"; const webCrypto = new Crypto(); -function getRandomValues(buf) { +function getRandomValues(buf: T): T { + // @ts-ignore fussy generics return nodeCrypto.randomFillSync(buf); } @@ -64,7 +65,7 @@ const TEST_VECTORS=[ ], ]; -function stringToArray(s) { +function stringToArray(s: string): ArrayBufferLike { return new TextEncoder().encode(s).buffer; } @@ -72,7 +73,10 @@ describe('MegolmExportEncryption', function() { let MegolmExportEncryption; beforeAll(() => { - window.crypto = { subtle: webCrypto.subtle, getRandomValues }; + window.crypto = { + subtle: webCrypto.subtle, + getRandomValues, + }; MegolmExportEncryption = require("../../src/utils/MegolmExportEncryption"); }); diff --git a/test/utils/ShieldUtils-test.js b/test/utils/ShieldUtils-test.ts similarity index 83% rename from test/utils/ShieldUtils-test.js rename to test/utils/ShieldUtils-test.ts index 85f9de31508..10ccb80f966 100644 --- a/test/utils/ShieldUtils-test.js +++ b/test/utils/ShieldUtils-test.ts @@ -1,7 +1,28 @@ +/* +Copyright 2022 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 { + MatrixClient, + Room, +} from 'matrix-js-sdk/src/matrix'; + import { shieldStatusForRoom } from '../../src/utils/ShieldUtils'; import DMRoomMap from '../../src/utils/DMRoomMap'; -function mkClient(selfTrust) { +function mkClient(selfTrust = false) { return { getUserId: () => "@self:localhost", checkUserTrust: (userId) => ({ @@ -12,7 +33,7 @@ function mkClient(selfTrust) { isVerified: () => userId === "@self:localhost" ? selfTrust : userId[2] == "T", }), getStoredDevicesForUser: (userId) => ["DEVICE"], - }; + } as unknown as MatrixClient; } describe("mkClient self-test", function() { @@ -42,9 +63,14 @@ describe("mkClient self-test", function() { describe("shieldStatusForMembership self-trust behaviour", function() { beforeAll(() => { - DMRoomMap.sharedInstance = { + const mockInstance = { getUserIdForRoomId: (roomId) => roomId === "DM" ? "@any:h" : null, - }; + } as unknown as DMRoomMap; + jest.spyOn(DMRoomMap, 'shared').mockReturnValue(mockInstance); + }); + + afterAll(() => { + jest.spyOn(DMRoomMap, 'shared').mockRestore(); }); it.each( @@ -55,7 +81,7 @@ describe("shieldStatusForMembership self-trust behaviour", function() { const room = { roomId: dm ? "DM" : "other", getEncryptionTargetMembers: () => ["@self:localhost", "@FF1:h", "@FF2:h"].map((userId) => ({ userId })), - }; + } as unknown as Room; const status = await shieldStatusForRoom(client, room); expect(status).toEqual("normal"); }); @@ -68,7 +94,7 @@ describe("shieldStatusForMembership self-trust behaviour", function() { const room = { roomId: dm ? "DM" : "other", getEncryptionTargetMembers: () => ["@self:localhost", "@TT1:h", "@TT2:h"].map((userId) => ({ userId })), - }; + } as unknown as Room; const status = await shieldStatusForRoom(client, room); expect(status).toEqual(result); }); @@ -81,7 +107,7 @@ describe("shieldStatusForMembership self-trust behaviour", function() { const room = { roomId: dm ? "DM" : "other", getEncryptionTargetMembers: () => ["@self:localhost", "@TT1:h", "@FF2:h"].map((userId) => ({ userId })), - }; + } as unknown as Room; const status = await shieldStatusForRoom(client, room); expect(status).toEqual(result); }); @@ -94,7 +120,7 @@ describe("shieldStatusForMembership self-trust behaviour", function() { const room = { roomId: dm ? "DM" : "other", getEncryptionTargetMembers: () => ["@self:localhost"].map((userId) => ({ userId })), - }; + } as unknown as Room; const status = await shieldStatusForRoom(client, room); expect(status).toEqual(result); }); @@ -107,7 +133,7 @@ describe("shieldStatusForMembership self-trust behaviour", function() { const room = { roomId: dm ? "DM" : "other", getEncryptionTargetMembers: () => ["@self:localhost", "@TT:h"].map((userId) => ({ userId })), - }; + } as unknown as Room; const status = await shieldStatusForRoom(client, room); expect(status).toEqual(result); }); @@ -120,7 +146,7 @@ describe("shieldStatusForMembership self-trust behaviour", function() { const room = { roomId: dm ? "DM" : "other", getEncryptionTargetMembers: () => ["@self:localhost", "@FF:h"].map((userId) => ({ userId })), - }; + } as unknown as Room; const status = await shieldStatusForRoom(client, room); expect(status).toEqual(result); }); @@ -128,9 +154,10 @@ describe("shieldStatusForMembership self-trust behaviour", function() { describe("shieldStatusForMembership other-trust behaviour", function() { beforeAll(() => { - DMRoomMap.sharedInstance = { + const mockInstance = { getUserIdForRoomId: (roomId) => roomId === "DM" ? "@any:h" : null, - }; + } as unknown as DMRoomMap; + jest.spyOn(DMRoomMap, 'shared').mockReturnValue(mockInstance); }); it.each( @@ -140,7 +167,7 @@ describe("shieldStatusForMembership other-trust behaviour", function() { const room = { roomId: dm ? "DM" : "other", getEncryptionTargetMembers: () => ["@self:localhost", "@TF:h"].map((userId) => ({ userId })), - }; + } as unknown as Room; const status = await shieldStatusForRoom(client, room); expect(status).toEqual(result); }); @@ -152,7 +179,7 @@ describe("shieldStatusForMembership other-trust behaviour", function() { const room = { roomId: dm ? "DM" : "other", getEncryptionTargetMembers: () => ["@self:localhost", "@TF:h", "@TT:h"].map((userId) => ({ userId })), - }; + } as unknown as Room; const status = await shieldStatusForRoom(client, room); expect(status).toEqual(result); }); @@ -164,7 +191,7 @@ describe("shieldStatusForMembership other-trust behaviour", function() { const room = { roomId: dm ? "DM" : "other", getEncryptionTargetMembers: () => ["@self:localhost", "@FF:h", "@FT:h"].map((userId) => ({ userId })), - }; + } as unknown as Room; const status = await shieldStatusForRoom(client, room); expect(status).toEqual(result); }); @@ -176,7 +203,7 @@ describe("shieldStatusForMembership other-trust behaviour", function() { const room = { roomId: dm ? "DM" : "other", getEncryptionTargetMembers: () => ["@self:localhost", "@WF:h", "@FT:h"].map((userId) => ({ userId })), - }; + } as unknown as Room; const status = await shieldStatusForRoom(client, room); expect(status).toEqual(result); }); From 483112950d14855bdecd9b0e61cb6f0bf93370e8 Mon Sep 17 00:00:00 2001 From: Suguru Hirahara Date: Mon, 2 May 2022 08:36:59 +0000 Subject: [PATCH 06/13] Fix poll overflowing a reply tile on bubble message layout (#8459) Signed-off-by: Suguru Hirahara --- res/css/views/rooms/_EventBubbleTile.scss | 1 + 1 file changed, 1 insertion(+) diff --git a/res/css/views/rooms/_EventBubbleTile.scss b/res/css/views/rooms/_EventBubbleTile.scss index 4104ae42733..93b5c789d76 100644 --- a/res/css/views/rooms/_EventBubbleTile.scss +++ b/res/css/views/rooms/_EventBubbleTile.scss @@ -406,6 +406,7 @@ limitations under the License. .mx_MPollBody { width: 550px; // to prevent timestamp overlapping summary text + max-width: 100%; // prevent overflowing a reply tile .mx_MPollBody_totalVotes { // align summary text with corner timestamp From 3e31fdb6a71f43774420e8da32452861296a263a Mon Sep 17 00:00:00 2001 From: Janne Mareike Koschinski Date: Mon, 2 May 2022 11:46:11 +0200 Subject: [PATCH 07/13] read receipts: improve tooltips to show names of users (#8438) --- .../views/rooms/ReadReceiptGroup.tsx | 54 ++++++++++--- src/i18n/strings/en_EN.json | 2 + .../views/rooms/ReadReceiptGroup-test.tsx | 80 +++++++++++++++++++ 3 files changed, 124 insertions(+), 12 deletions(-) create mode 100644 test/components/views/rooms/ReadReceiptGroup-test.tsx diff --git a/src/components/views/rooms/ReadReceiptGroup.tsx b/src/components/views/rooms/ReadReceiptGroup.tsx index 31a2a59dd30..c271d9f9ca5 100644 --- a/src/components/views/rooms/ReadReceiptGroup.tsx +++ b/src/components/views/rooms/ReadReceiptGroup.tsx @@ -52,13 +52,11 @@ interface IAvatarPosition { position: number; } -function determineAvatarPosition(index: number, count: number, max: number): IAvatarPosition { - const firstVisible = Math.max(0, count - max); - - if (index >= firstVisible) { +export function determineAvatarPosition(index: number, count: number, max: number): IAvatarPosition { + if (index < max) { return { hidden: false, - position: index - firstVisible, + position: Math.min(count, max) - index - 1, }; } else { return { @@ -68,12 +66,49 @@ function determineAvatarPosition(index: number, count: number, max: number): IAv } } +export function readReceiptTooltip(members: string[], hasMore: boolean): string | null { + if (hasMore) { + return _t("%(members)s and more", { + members: members.join(", "), + }); + } else if (members.length > 1) { + return _t("%(members)s and %(last)s", { + last: members.pop(), + members: members.join(", "), + }); + } else if (members.length) { + return members[0]; + } else { + return null; + } +} + export function ReadReceiptGroup( { readReceipts, readReceiptMap, checkUnmounting, suppressAnimation, isTwelveHour }: Props, ) { const [menuDisplayed, button, openMenu, closeMenu] = useContextMenu(); + + // If we are above MAX_READ_AVATARS, we’ll have to remove a few to have space for the +n count. + const hasMore = readReceipts.length > MAX_READ_AVATARS; + const maxAvatars = hasMore + ? MAX_READ_AVATARS_PLUS_N + : MAX_READ_AVATARS; + + const tooltipMembers: string[] = readReceipts.slice(0, maxAvatars) + .map(it => it.roomMember?.name ?? it.userId); + const tooltipText = readReceiptTooltip(tooltipMembers, hasMore); + const [{ showTooltip, hideTooltip }, tooltip] = useTooltip({ - label: _t("Seen by %(count)s people", { count: readReceipts.length }), + label: ( + <> +
+ { _t("Seen by %(count)s people", { count: readReceipts.length }) } +
+
+ { tooltipText } +
+ + ), alignment: Alignment.TopRight, }); @@ -97,11 +132,6 @@ export function ReadReceiptGroup( ); } - // If we are above MAX_READ_AVATARS, we’ll have to remove a few to have space for the +n count. - const maxAvatars = readReceipts.length > MAX_READ_AVATARS - ? MAX_READ_AVATARS_PLUS_N - : MAX_READ_AVATARS; - const avatars = readReceipts.map((receipt, index) => { const { hidden, position } = determineAvatarPosition(index, readReceipts.length, maxAvatars); @@ -130,7 +160,7 @@ export function ReadReceiptGroup( showTwelveHour={isTwelveHour} /> ); - }); + }).reverse(); let remText: JSX.Element; const remainder = readReceipts.length - maxAvatars; diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 17d730b4212..31c6f3383fd 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -1766,6 +1766,8 @@ "Preview": "Preview", "View": "View", "Join": "Join", + "%(members)s and more": "%(members)s and more", + "%(members)s and %(last)s": "%(members)s and %(last)s", "Seen by %(count)s people|other": "Seen by %(count)s people", "Seen by %(count)s people|one": "Seen by %(count)s person", "Recently viewed": "Recently viewed", diff --git a/test/components/views/rooms/ReadReceiptGroup-test.tsx b/test/components/views/rooms/ReadReceiptGroup-test.tsx new file mode 100644 index 00000000000..28f1caa511b --- /dev/null +++ b/test/components/views/rooms/ReadReceiptGroup-test.tsx @@ -0,0 +1,80 @@ +import { determineAvatarPosition, readReceiptTooltip } from "../../../../src/components/views/rooms/ReadReceiptGroup"; + +describe("ReadReceiptGroup", () => { + describe("TooltipText", () => { + it("returns '...and more' with hasMore", () => { + expect(readReceiptTooltip(["Alice", "Bob", "Charlie", "Dan", "Eve"], true)) + .toEqual("Alice, Bob, Charlie, Dan, Eve and more"); + expect(readReceiptTooltip(["Alice", "Bob", "Charlie", "Dan"], true)) + .toEqual("Alice, Bob, Charlie, Dan and more"); + expect(readReceiptTooltip(["Alice", "Bob", "Charlie"], true)) + .toEqual("Alice, Bob, Charlie and more"); + expect(readReceiptTooltip(["Alice", "Bob"], true)) + .toEqual("Alice, Bob and more"); + expect(readReceiptTooltip(["Alice"], true)) + .toEqual("Alice and more"); + expect(readReceiptTooltip([], false)) + .toEqual(null); + }); + it("returns a pretty list without hasMore", () => { + expect(readReceiptTooltip(["Alice", "Bob", "Charlie", "Dan", "Eve"], false)) + .toEqual("Alice, Bob, Charlie, Dan and Eve"); + expect(readReceiptTooltip(["Alice", "Bob", "Charlie", "Dan"], false)) + .toEqual("Alice, Bob, Charlie and Dan"); + expect(readReceiptTooltip(["Alice", "Bob", "Charlie"], false)) + .toEqual("Alice, Bob and Charlie"); + expect(readReceiptTooltip(["Alice", "Bob"], false)) + .toEqual("Alice and Bob"); + expect(readReceiptTooltip(["Alice"], false)) + .toEqual("Alice"); + expect(readReceiptTooltip([], false)) + .toEqual(null); + }); + }); + describe("AvatarPosition", () => { + // The avatar slots are numbered from right to left + // That means currently, we’ve got the slots | 3 | 2 | 1 | 0 | each with 10px distance to the next one. + // We want to fill slots so the first avatar is in the left-most slot without leaving any slots at the right + // unoccupied. + it("to handle the non-overflowing case correctly", () => { + expect(determineAvatarPosition(0, 1, 4)) + .toEqual({ hidden: false, position: 0 }); + + expect(determineAvatarPosition(0, 2, 4)) + .toEqual({ hidden: false, position: 1 }); + expect(determineAvatarPosition(1, 2, 4)) + .toEqual({ hidden: false, position: 0 }); + + expect(determineAvatarPosition(0, 3, 4)) + .toEqual({ hidden: false, position: 2 }); + expect(determineAvatarPosition(1, 3, 4)) + .toEqual({ hidden: false, position: 1 }); + expect(determineAvatarPosition(2, 3, 4)) + .toEqual({ hidden: false, position: 0 }); + + expect(determineAvatarPosition(0, 4, 4)) + .toEqual({ hidden: false, position: 3 }); + expect(determineAvatarPosition(1, 4, 4)) + .toEqual({ hidden: false, position: 2 }); + expect(determineAvatarPosition(2, 4, 4)) + .toEqual({ hidden: false, position: 1 }); + expect(determineAvatarPosition(3, 4, 4)) + .toEqual({ hidden: false, position: 0 }); + }); + + it("to handle the overflowing case correctly", () => { + expect(determineAvatarPosition(0, 6, 4)) + .toEqual({ hidden: false, position: 3 }); + expect(determineAvatarPosition(1, 6, 4)) + .toEqual({ hidden: false, position: 2 }); + expect(determineAvatarPosition(2, 6, 4)) + .toEqual({ hidden: false, position: 1 }); + expect(determineAvatarPosition(3, 6, 4)) + .toEqual({ hidden: false, position: 0 }); + expect(determineAvatarPosition(4, 6, 4)) + .toEqual({ hidden: true, position: 0 }); + expect(determineAvatarPosition(5, 6, 4)) + .toEqual({ hidden: true, position: 0 }); + }); + }); +}); From 7477a2df7d3737a29eca08c0bdf41cc43b8a6dc6 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 2 May 2022 21:34:31 +0100 Subject: [PATCH 08/13] Switch coverage over to SonarQube (#8463) --- .github/codecov.yml | 12 ------- .github/workflows/sonarqube.yml | 47 ++++++++++++++++++++++++++ .github/workflows/static_analysis.yaml | 13 ------- .github/workflows/tests.yml | 20 +++++------ package.json | 12 +++++-- sonar-project.properties | 6 ++++ yarn.lock | 12 +++++++ 7 files changed, 82 insertions(+), 40 deletions(-) delete mode 100644 .github/codecov.yml create mode 100644 .github/workflows/sonarqube.yml diff --git a/.github/codecov.yml b/.github/codecov.yml deleted file mode 100644 index 449fa0a733a..00000000000 --- a/.github/codecov.yml +++ /dev/null @@ -1,12 +0,0 @@ -codecov: - allow_coverage_offsets: True -coverage: - status: - project: off - patch: off -comment: - layout: "diff, files" - behavior: default - require_changes: false - require_base: no - require_head: no diff --git a/.github/workflows/sonarqube.yml b/.github/workflows/sonarqube.yml new file mode 100644 index 00000000000..d81aeac3118 --- /dev/null +++ b/.github/workflows/sonarqube.yml @@ -0,0 +1,47 @@ +name: SonarQube +on: + workflow_run: + workflows: [ "Tests" ] + types: + - completed +jobs: + sonarqube: + name: SonarQube + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + with: + fetch-depth: 0 # Shallow clones should be disabled for a better relevancy of analysis + + # There's a 'download artifact' action, but it hasn't been updated for the workflow_run action + # (https://github.com/actions/download-artifact/issues/60) so instead we get this mess: + - name: Download Coverage Report + uses: actions/github-script@v3.1.0 + if: github.event.workflow_run.conclusion == 'success' + with: + script: | + const artifacts = await github.actions.listWorkflowRunArtifacts({ + owner: context.repo.owner, + repo: context.repo.repo, + run_id: ${{ github.event.workflow_run.id }}, + }); + const matchArtifact = artifacts.data.artifacts.filter((artifact) => { + return artifact.name == "coverage" + })[0]; + const download = await github.actions.downloadArtifact({ + owner: context.repo.owner, + repo: context.repo.repo, + artifact_id: matchArtifact.id, + archive_format: 'zip', + }); + const fs = require('fs'); + fs.writeFileSync('${{github.workspace}}/coverage.zip', Buffer.from(download.data)); + - name: Extract Coverage Report + run: unzip -d coverage coverage.zip && rm coverage.zip + if: github.event.workflow_run.conclusion == 'success' + + - name: SonarCloud Scan + uses: SonarSource/sonarcloud-github-action@master + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # Needed to get PR information, if any + SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} diff --git a/.github/workflows/static_analysis.yaml b/.github/workflows/static_analysis.yaml index 63e939f7f9a..c1269e0721e 100644 --- a/.github/workflows/static_analysis.yaml +++ b/.github/workflows/static_analysis.yaml @@ -87,16 +87,3 @@ jobs: - name: Run Linter run: "yarn run lint:style" - - sonarqube: - name: "SonarQube" - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v2 - with: - fetch-depth: 0 # Shallow clones should be disabled for a better relevancy of analysis - - name: SonarCloud Scan - uses: SonarSource/sonarcloud-github-action@master - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index f160e42844d..9fa7a6f7cf1 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -11,16 +11,11 @@ env: PR_NUMBER: ${{ github.event.pull_request.number }} jobs: jest: - name: Jest with Codecov + name: Jest runs-on: ubuntu-latest steps: - name: Checkout code uses: actions/checkout@v2 - with: - # If this is a pull request, make sure we check out its head rather than the - # automatically generated merge commit, so that the coverage diff excludes - # unrelated changes in the base branch - ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || '' }} - name: Yarn cache uses: actions/setup-node@v3 @@ -31,11 +26,12 @@ jobs: run: "./scripts/ci/install-deps.sh --ignore-scripts" - name: Run tests with coverage - run: "yarn coverage" + run: "yarn coverage --ci" - - name: Upload coverage - uses: codecov/codecov-action@v2 + - name: Upload Artifact + uses: actions/upload-artifact@v2 with: - fail_ci_if_error: false - verbose: true - override_commit: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || '' }} + name: coverage + path: | + coverage + !coverage/lcov-report diff --git a/package.json b/package.json index c62184810c5..e96aac14a91 100644 --- a/package.json +++ b/package.json @@ -184,6 +184,7 @@ "jest-fetch-mock": "^3.0.3", "jest-mock": "^27.5.1", "jest-raw-loader": "^1.0.1", + "jest-sonar-reporter": "^2.0.0", "matrix-mock-request": "^1.2.3", "matrix-react-test-utils": "^0.2.3", "matrix-web-i18n": "^1.2.0", @@ -233,9 +234,14 @@ "/src/**/*.{js,ts,tsx}" ], "coverageReporters": [ - "text", - "json" - ] + "text-summary", + "lcov" + ], + "testResultsProcessor": "jest-sonar-reporter" + }, + "jestSonar": { + "reportPath": "coverage", + "sonar56x": true }, "typings": "./lib/index.d.ts" } diff --git a/sonar-project.properties b/sonar-project.properties index afeecf737be..e172bfbfa22 100644 --- a/sonar-project.properties +++ b/sonar-project.properties @@ -14,3 +14,9 @@ sonar.organization=matrix-org sonar.sources=src,res sonar.tests=test,cypress sonar.exclusions=__mocks__,docs + +sonar.typescript.tsconfigPath=./tsconfig.json +sonar.javascript.lcov.reportPaths=coverage/lcov.info +sonar.coverage.exclusions=spec/*.ts +sonar.testExecutionReportPaths=coverage/test-report.xml +sonar.genericcoverage.unitTestReportPaths=coverage/test-report.xml diff --git a/yarn.lock b/yarn.lock index 5e5e4838c3f..f3b83bfae14 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6072,6 +6072,13 @@ jest-snapshot@^27.5.1: pretty-format "^27.5.1" semver "^7.3.2" +jest-sonar-reporter@^2.0.0: + version "2.0.0" + resolved "https://registry.yarnpkg.com/jest-sonar-reporter/-/jest-sonar-reporter-2.0.0.tgz#faa54a7d2af7198767ee246a82b78c576789cf08" + integrity sha512-ZervDCgEX5gdUbdtWsjdipLN3bKJwpxbvhkYNXTAYvAckCihobSLr9OT/IuyNIRT1EZMDDwR6DroWtrq+IL64w== + dependencies: + xml "^1.0.1" + jest-util@^26.6.2: version "26.6.2" resolved "https://registry.yarnpkg.com/jest-util/-/jest-util-26.6.2.tgz#907535dbe4d5a6cb4c47ac9b926f6af29576cbc1" @@ -9538,6 +9545,11 @@ xml-name-validator@^3.0.0: resolved "https://registry.yarnpkg.com/xml-name-validator/-/xml-name-validator-3.0.0.tgz#6ae73e06de4d8c6e47f9fb181f78d648ad457c6a" integrity sha512-A5CUptxDsvxKJEU3yO6DuWBSJz/qizqzJKOMIfUJHETbBw/sFaDxgd6fxm1ewUaM0jZ444Fc5vC5ROYurg/4Pw== +xml@^1.0.1: + version "1.0.1" + resolved "https://registry.yarnpkg.com/xml/-/xml-1.0.1.tgz#78ba72020029c5bc87b8a81a3cfcd74b4a2fc1e5" + integrity sha1-eLpyAgApxbyHuKgaPPzXS0ovweU= + xmlchars@^2.2.0: version "2.2.0" resolved "https://registry.yarnpkg.com/xmlchars/-/xmlchars-2.2.0.tgz#060fe1bcb7f9c76fe2a17db86a9bc3ab894210cb" From 1c70696b1020c9f54f78855b78664cdccaa81a2e Mon Sep 17 00:00:00 2001 From: Robin Date: Mon, 2 May 2022 20:26:37 -0400 Subject: [PATCH 09/13] Don't linkify code blocks (#7859) * Don't linkify code blocks Signed-off-by: Robin Townsend * Put the linkify ignoreTags option in the right place Signed-off-by: Robin Townsend * Add code to list of ignored linkification tags as well Signed-off-by: Robin Townsend * Test that code blocks skip linkification Signed-off-by: Robin Townsend * Move test to the right spot Signed-off-by: Robin Townsend * Use a snapshot instead for test Signed-off-by: Robin Townsend --- src/linkify-matrix.ts | 2 ++ .../views/messages/TextualBody-test.tsx | 20 +++++++++++++++++++ .../__snapshots__/TextualBody-test.tsx.snap | 7 +++++++ 3 files changed, 29 insertions(+) diff --git a/src/linkify-matrix.ts b/src/linkify-matrix.ts index c42d98d326c..7935f5d0377 100644 --- a/src/linkify-matrix.ts +++ b/src/linkify-matrix.ts @@ -225,6 +225,8 @@ export const options = { rel: 'noreferrer noopener', }, + ignoreTags: ['pre', 'code'], + className: 'linkified', target: function(href: string, type: Type | string): string { diff --git a/test/components/views/messages/TextualBody-test.tsx b/test/components/views/messages/TextualBody-test.tsx index 371c372196f..fe4dbf99467 100644 --- a/test/components/views/messages/TextualBody-test.tsx +++ b/test/components/views/messages/TextualBody-test.tsx @@ -216,6 +216,26 @@ describe("", () => { ''); }); + it("linkification is not applied to code blocks", () => { + const ev = mkEvent({ + type: "m.room.message", + room: "room_id", + user: "sender", + content: { + body: "Visit `https://matrix.org/`\n```\nhttps://matrix.org/\n```", + msgtype: "m.text", + format: "org.matrix.custom.html", + formatted_body: "

Visit https://matrix.org/

\n
https://matrix.org/\n
\n", + }, + event: true, + }); + + const wrapper = getComponent({ mxEvent: ev }, matrixClient); + expect(wrapper.text()).toBe("Visit https://matrix.org/\n1https://matrix.org/\n\n"); + const content = wrapper.find(".mx_EventTile_body"); + expect(content.html()).toMatchSnapshot(); + }); + // If pills were rendered within a Portal/same shadow DOM then it'd be easier to test it("pills get injected correctly into the DOM", () => { const ev = mkEvent({ diff --git a/test/components/views/messages/__snapshots__/TextualBody-test.tsx.snap b/test/components/views/messages/__snapshots__/TextualBody-test.tsx.snap index 9d481765e1d..15d3b7e208f 100644 --- a/test/components/views/messages/__snapshots__/TextualBody-test.tsx.snap +++ b/test/components/views/messages/__snapshots__/TextualBody-test.tsx.snap @@ -1,5 +1,12 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[` renders formatted m.text correctly linkification is not applied to code blocks 1`] = ` +"

Visit https://matrix.org/

+
1https://matrix.org/
+
+" +`; + exports[` renders formatted m.text correctly pills do not appear in code blocks 1`] = ` "

@room

1@room

From f280fab47df45571c45e4a7db95259ec63f3246f Mon Sep 17 00:00:00 2001
From: Kerry 
Date: Tue, 3 May 2022 10:22:38 +0200
Subject: [PATCH 10/13] test typescriptification - ForwardDialog (#8469)

* test/components/views/dialogs/ForwardDialog-test.js -> tsx

Signed-off-by: Kerry Archibald 

* fix ts issues in ForwardDialog

Signed-off-by: Kerry Archibald 

* remove unused stub-component

Signed-off-by: Kerry Archibald 
---
 test/components/stub-component.js             | 20 -------
 ...dDialog-test.js => ForwardDialog-test.tsx} | 60 ++++++++++++++-----
 2 files changed, 46 insertions(+), 34 deletions(-)
 delete mode 100644 test/components/stub-component.js
 rename test/components/views/dialogs/{ForwardDialog-test.js => ForwardDialog-test.tsx} (75%)

diff --git a/test/components/stub-component.js b/test/components/stub-component.js
deleted file mode 100644
index f98959a5730..00000000000
--- a/test/components/stub-component.js
+++ /dev/null
@@ -1,20 +0,0 @@
-/* A dummy React component which we use for stubbing out app-level components
- */
-
-import React from 'react';
-
-export default function({ displayName = "StubComponent", render } = {}) {
-    if (!render) {
-        render = function() {
-            return 
{ displayName }
;
-        };
-    }
-
-    return class extends React.Component {
-        static displayName = displayName;
-
-        render() {
-            return render();
-        }
-    };
-}
diff --git a/test/components/views/dialogs/ForwardDialog-test.js b/test/components/views/dialogs/ForwardDialog-test.tsx
similarity index 75%
rename from test/components/views/dialogs/ForwardDialog-test.js
rename to test/components/views/dialogs/ForwardDialog-test.tsx
index 835f73bbf69..089a92a2b32 100644
--- a/test/components/views/dialogs/ForwardDialog-test.js
+++ b/test/components/views/dialogs/ForwardDialog-test.tsx
@@ -17,32 +17,59 @@ limitations under the License.
 import React from "react";
 import { mount } from "enzyme";
 import { act } from "react-dom/test-utils";
+import { MatrixEvent, EventType } from "matrix-js-sdk/src/matrix";
 
-import * as TestUtils from "../../../test-utils";
 import { MatrixClientPeg } from "../../../../src/MatrixClientPeg";
+import ForwardDialog from "../../../../src/components/views/dialogs/ForwardDialog";
 import DMRoomMap from "../../../../src/utils/DMRoomMap";
 import { RoomPermalinkCreator } from "../../../../src/utils/permalinks/Permalinks";
-import ForwardDialog from "../../../../src/components/views/dialogs/ForwardDialog";
+import {
+    getMockClientWithEventEmitter,
+    mkEvent,
+    mkMessage,
+    mkStubRoom,
+} from "../../../test-utils";
 
 describe("ForwardDialog", () => {
     const sourceRoom = "!111111111111111111:example.org";
-    const defaultMessage = TestUtils.mkMessage({
+    const aliceId = "@alice:example.org";
+    const defaultMessage = mkMessage({
         room: sourceRoom,
-        user: "@alice:example.org",
+        user: aliceId,
         msg: "Hello world!",
         event: true,
     });
-    const defaultRooms = ["a", "A", "b"].map(name => TestUtils.mkStubRoom(name, name));
+    const accountDataEvent = new MatrixEvent({
+        type: EventType.Direct,
+        sender: aliceId,
+        content: {},
+    });
+    const mockClient = getMockClientWithEventEmitter({
+        getUserId: jest.fn().mockReturnValue(aliceId),
+        isGuest: jest.fn().mockReturnValue(false),
+        getVisibleRooms: jest.fn().mockReturnValue([]),
+        getRoom: jest.fn(),
+        getAccountData: jest.fn().mockReturnValue(accountDataEvent),
+        getPushActionsForEvent: jest.fn(),
+        mxcUrlToHttp: jest.fn().mockReturnValue(''),
+        isRoomEncrypted: jest.fn().mockReturnValue(false),
+        getProfileInfo: jest.fn().mockResolvedValue({
+            displayname: 'Alice',
+        }),
+        decryptEventIfNeeded: jest.fn(),
+        sendEvent: jest.fn(),
+    });
+    const defaultRooms = ["a", "A", "b"].map(name => mkStubRoom(name, name, mockClient));
 
     const mountForwardDialog = async (message = defaultMessage, rooms = defaultRooms) => {
-        const client = MatrixClientPeg.get();
-        client.getVisibleRooms = jest.fn().mockReturnValue(rooms);
+        mockClient.getVisibleRooms.mockReturnValue(rooms);
+        mockClient.getRoom.mockImplementation(roomId => rooms.find(room => room.roomId === roomId));
 
         let wrapper;
         await act(async () => {
             wrapper = mount(
                  {
     };
 
     beforeEach(() => {
-        TestUtils.stubClient();
         DMRoomMap.makeShared();
-        MatrixClientPeg.get().getUserId = jest.fn().mockReturnValue("@bob:example.org");
+        jest.clearAllMocks();
+        mockClient.getUserId.mockReturnValue("@bob:example.org");
+        mockClient.sendEvent.mockReset();
+    });
+
+    afterAll(() => {
+        jest.spyOn(MatrixClientPeg, 'get').mockRestore();
     });
 
     it("shows a preview with us as the sender", async () => {
@@ -91,7 +123,7 @@ describe("ForwardDialog", () => {
         // Make sendEvent require manual resolution so we can see the sending state
         let finishSend;
         let cancelSend;
-        MatrixClientPeg.get().sendEvent = jest.fn(() => new Promise((resolve, reject) => {
+        mockClient.sendEvent.mockImplementation(() => new Promise((resolve, reject) => {
             finishSend = resolve;
             cancelSend = reject;
         }));
@@ -135,7 +167,7 @@ describe("ForwardDialog", () => {
     });
 
     it("can render replies", async () => {
-        const replyMessage = TestUtils.mkEvent({
+        const replyMessage = mkEvent({
             type: "m.room.message",
             room: "!111111111111111111:example.org",
             user: "@alice:example.org",
@@ -156,9 +188,9 @@ describe("ForwardDialog", () => {
     });
 
     it("disables buttons for rooms without send permissions", async () => {
-        const readOnlyRoom = TestUtils.mkStubRoom("a", "a");
+        const readOnlyRoom = mkStubRoom("a", "a", mockClient);
         readOnlyRoom.maySendMessage = jest.fn().mockReturnValue(false);
-        const rooms = [readOnlyRoom, TestUtils.mkStubRoom("b", "b")];
+        const rooms = [readOnlyRoom, mkStubRoom("b", "b", mockClient)];
 
         const wrapper = await mountForwardDialog(undefined, rooms);
 

From 3b1e71585470ec05ed8d54b37c95556d65db5a98 Mon Sep 17 00:00:00 2001
From: Kerry 
Date: Tue, 3 May 2022 10:29:43 +0200
Subject: [PATCH 11/13] Live location sharing: remove geoUri logs (#8465)

* remove geoUri logs

Signed-off-by: Kerry Archibald 

* Update src/components/views/location/Map.tsx

Co-authored-by: Michael Telatynski <7t3chguy@gmail.com>

Co-authored-by: Michael Telatynski <7t3chguy@gmail.com>
---
 src/components/views/location/Map.tsx       | 8 ++++----
 test/components/views/location/Map-test.tsx | 4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/components/views/location/Map.tsx b/src/components/views/location/Map.tsx
index b3e109f4538..023ff2d5ccb 100644
--- a/src/components/views/location/Map.tsx
+++ b/src/components/views/location/Map.tsx
@@ -51,8 +51,8 @@ const useMapWithStyle = ({ id, centerGeoUri, onError, interactive, bounds }) =>
             try {
                 const coords = parseGeoUri(centerGeoUri);
                 map.setCenter({ lon: coords.longitude, lat: coords.latitude });
-            } catch (error) {
-                logger.error('Could not set map center', centerGeoUri);
+            } catch (_error) {
+                logger.error('Could not set map center');
             }
         }
     }, [map, centerGeoUri]);
@@ -65,8 +65,8 @@ const useMapWithStyle = ({ id, centerGeoUri, onError, interactive, bounds }) =>
                     [bounds.east, bounds.north],
                 );
                 map.fitBounds(lngLatBounds, { padding: 100, maxZoom: 15 });
-            } catch (error) {
-                logger.error('Invalid map bounds', error);
+            } catch (_error) {
+                logger.error('Invalid map bounds');
             }
         }
     }, [map, bounds]);
diff --git a/test/components/views/location/Map-test.tsx b/test/components/views/location/Map-test.tsx
index 72826a9cdf2..12915c192ee 100644
--- a/test/components/views/location/Map-test.tsx
+++ b/test/components/views/location/Map-test.tsx
@@ -101,7 +101,7 @@ describe('', () => {
             const logSpy = jest.spyOn(logger, 'error').mockImplementation();
             getComponent({ centerGeoUri: '123 Sesame Street' });
             expect(mockMap.setCenter).not.toHaveBeenCalled();
-            expect(logSpy).toHaveBeenCalledWith('Could not set map center', '123 Sesame Street');
+            expect(logSpy).toHaveBeenCalledWith('Could not set map center');
         });
 
         it('updates map center when centerGeoUri prop changes', () => {
@@ -133,7 +133,7 @@ describe('', () => {
             const bounds = { north: 'a', south: 'b', east: 42, west: 41 };
             getComponent({ bounds });
             expect(mockMap.fitBounds).not.toHaveBeenCalled();
-            expect(logSpy).toHaveBeenCalledWith('Invalid map bounds', new Error('Invalid LngLat object: (41, NaN)'));
+            expect(logSpy).toHaveBeenCalledWith('Invalid map bounds');
         });
 
         it('updates map bounds when bounds prop changes', () => {

From c5633a24fe92f9983863d49682c91bb24fdd5682 Mon Sep 17 00:00:00 2001
From: Kerry 
Date: Tue, 3 May 2022 11:04:47 +0200
Subject: [PATCH 12/13] Live location sharing: don't group beacon info with
 room creation summary (#8468)

* dont group beacon info with room creation summary

Signed-off-by: Kerry Archibald 

* remove debugs

Signed-off-by: Kerry Archibald 

* add comment

Signed-off-by: Kerry Archibald 

* update comment

Signed-off-by: Kerry Archibald 
---
 src/components/structures/MessagePanel.tsx    |  9 ++++++-
 .../structures/MessagePanel-test.js           | 26 +++++++++++++++++++
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/src/components/structures/MessagePanel.tsx b/src/components/structures/MessagePanel.tsx
index 38d18e92f76..5dd5ea01936 100644
--- a/src/components/structures/MessagePanel.tsx
+++ b/src/components/structures/MessagePanel.tsx
@@ -23,6 +23,7 @@ import { MatrixEvent } from 'matrix-js-sdk/src/models/event';
 import { Relations } from "matrix-js-sdk/src/models/relations";
 import { logger } from 'matrix-js-sdk/src/logger';
 import { RoomStateEvent } from "matrix-js-sdk/src/models/room-state";
+import { M_BEACON_INFO } from 'matrix-js-sdk/src/@types/beacon';
 
 import shouldHideEvent from '../../shouldHideEvent';
 import { wantsDateSeparator } from '../../DateUtils';
@@ -1079,7 +1080,7 @@ abstract class BaseGrouper {
 
 // Wrap initial room creation events into a GenericEventListSummary
 // Grouping only events sent by the same user that sent the `m.room.create` and only until
-// the first non-state event or membership event which is not regarding the sender of the `m.room.create` event
+// the first non-state event, beacon_info event or membership event which is not regarding the sender of the `m.room.create` event
 class CreationGrouper extends BaseGrouper {
     static canStartGroup = function(panel: MessagePanel, ev: MatrixEvent): boolean {
         return ev.getType() === EventType.RoomCreate;
@@ -1098,9 +1099,15 @@ class CreationGrouper extends BaseGrouper {
             && (ev.getStateKey() !== createEvent.getSender() || ev.getContent()["membership"] !== "join")) {
             return false;
         }
+        // beacons are not part of room creation configuration
+        // should be shown in timeline
+        if (M_BEACON_INFO.matches(ev.getType())) {
+            return false;
+        }
         if (ev.isState() && ev.getSender() === createEvent.getSender()) {
             return true;
         }
+
         return false;
     }
 
diff --git a/test/components/structures/MessagePanel-test.js b/test/components/structures/MessagePanel-test.js
index 4b2d5499361..769e90c9bbb 100644
--- a/test/components/structures/MessagePanel-test.js
+++ b/test/components/structures/MessagePanel-test.js
@@ -34,6 +34,11 @@ import * as TestUtilsMatrix from "../../test-utils";
 import EventListSummary from "../../../src/components/views/elements/EventListSummary";
 import GenericEventListSummary from "../../../src/components/views/elements/GenericEventListSummary";
 import DateSeparator from "../../../src/components/views/messages/DateSeparator";
+import { makeBeaconInfoEvent } from '../../test-utils';
+
+jest.mock('../../../src/utils/beacon', () => ({
+    useBeacon: jest.fn(),
+}));
 
 let client;
 const room = new Matrix.Room("!roomId:server_name");
@@ -481,6 +486,27 @@ describe('MessagePanel', function() {
         expect(summaryEventTiles.length).toEqual(tiles.length - 3);
     });
 
+    it('should not collapse beacons as part of creation events', function() {
+        const [creationEvent] = mkCreationEvents();
+        const beaconInfoEvent = makeBeaconInfoEvent(
+            creationEvent.getSender(),
+            creationEvent.getRoomId(),
+            { isLive: true },
+        );
+        const combinedEvents = [creationEvent, beaconInfoEvent];
+        TestUtilsMatrix.upsertRoomStateEvents(room, combinedEvents);
+        const res = mount(
+            ,
+        );
+
+        const summaryTiles = res.find(GenericEventListSummary);
+        const summaryTile = summaryTiles.at(0);
+
+        const summaryEventTiles = summaryTile.find(UnwrappedEventTile);
+        // nothing in the summary
+        expect(summaryEventTiles.length).toEqual(0);
+    });
+
     it('should hide read-marker at the end of creation event summary', function() {
         const events = mkCreationEvents();
         TestUtilsMatrix.upsertRoomStateEvents(room, events);

From 7f78b1c968ce70cd63392c07b28c6983f41f2ea9 Mon Sep 17 00:00:00 2001
From: Michael Telatynski <7t3chguy@gmail.com>
Date: Tue, 3 May 2022 10:39:03 +0100
Subject: [PATCH 13/13] Iterate CI checks (#8472)

---
 .github/workflows/preview_changelog.yaml | 12 ------------
 .github/workflows/pull_request.yaml      | 24 ++++++++++++++++++++++++
 .github/workflows/static_analysis.yaml   | 20 +++++++++++++++++++-
 3 files changed, 43 insertions(+), 13 deletions(-)
 delete mode 100644 .github/workflows/preview_changelog.yaml
 create mode 100644 .github/workflows/pull_request.yaml

diff --git a/.github/workflows/preview_changelog.yaml b/.github/workflows/preview_changelog.yaml
deleted file mode 100644
index 786d828d419..00000000000
--- a/.github/workflows/preview_changelog.yaml
+++ /dev/null
@@ -1,12 +0,0 @@
-name: Preview Changelog
-on:
-  pull_request_target:
-    types: [ opened, edited, labeled ]
-jobs:
-  changelog:
-    runs-on: ubuntu-latest
-    steps:
-      - name: Preview Changelog
-        uses: matrix-org/allchange@main
-        with:
-          ghToken: ${{ secrets.GITHUB_TOKEN }}
diff --git a/.github/workflows/pull_request.yaml b/.github/workflows/pull_request.yaml
new file mode 100644
index 00000000000..22a92bf0b56
--- /dev/null
+++ b/.github/workflows/pull_request.yaml
@@ -0,0 +1,24 @@
+name: Pull Request
+on:
+  pull_request_target:
+    types: [ opened, edited, labeled, unlabeled ]
+jobs:
+  changelog:
+    name: Preview Changelog
+    runs-on: ubuntu-latest
+    steps:
+      - uses: matrix-org/allchange@main
+        with:
+          ghToken: ${{ secrets.GITHUB_TOKEN }}
+
+  enforce-label:
+    name: Enforce Labels
+    runs-on: ubuntu-latest
+    permissions:
+      pull-requests: read
+    steps:
+      - uses: yogevbd/enforce-label-action@2.1.0
+        with:
+          REQUIRED_LABELS_ANY: "T-Defect,T-Enhancement,T-Task"
+          BANNED_LABELS: "X-Blocked"
+          BANNED_LABELS_DESCRIPTION: "Preventing merge whilst PR is marked blocked!"
diff --git a/.github/workflows/static_analysis.yaml b/.github/workflows/static_analysis.yaml
index c1269e0721e..5e2f27f68b8 100644
--- a/.github/workflows/static_analysis.yaml
+++ b/.github/workflows/static_analysis.yaml
@@ -38,11 +38,29 @@ jobs:
         run: "yarn run lint:types"
 
   i18n_lint:
-    name: "i18n Diff Check"
+    name: "i18n Check"
     runs-on: ubuntu-latest
+    permissions:
+      pull-requests: read
     steps:
       - uses: actions/checkout@v2
 
+      - name: "Get modified files"
+        id: changed_files
+        if: github.event_name == 'pull_request'
+        uses: tj-actions/changed-files@v19
+        with:
+          files: |
+            src/i18n/strings/*
+          files_ignore: |
+            src/i18n/strings/en_EN.json
+
+      - name: "Assert only en_EN was modified"
+        if: github.event_name == 'pull_request' && steps.changed_files.outputs.any_modified == 'true'
+        run: |
+          echo "You can only modify en_EN.json, do not touch any of the other i18n files as Weblate will be confused"
+          exit 1
+
       - uses: actions/setup-node@v3
         with:
           cache: 'yarn'