From 667a7548a72ab348f7f40ae8421c5bad8b688347 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 15 May 2024 10:01:45 +0100 Subject: [PATCH 01/42] Use `*` for italics as it doesn't break when used mid-word (#12523) * Use `*` for italics as it doesn't break when used mid-word Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Fix tests Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --------- Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/editor/operations.ts | 2 +- test/editor/operations-test.ts | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/editor/operations.ts b/src/editor/operations.ts index 13db8d56aa..ca8492b080 100644 --- a/src/editor/operations.ts +++ b/src/editor/operations.ts @@ -48,7 +48,7 @@ export function formatRange(range: Range, action: Formatting): void { toggleInlineFormat(range, "**"); break; case Formatting.Italics: - toggleInlineFormat(range, "_"); + toggleInlineFormat(range, "*"); break; case Formatting.Strikethrough: toggleInlineFormat(range, "", ""); diff --git a/test/editor/operations-test.ts b/test/editor/operations-test.ts index 2a456e4683..714dba42f0 100644 --- a/test/editor/operations-test.ts +++ b/test/editor/operations-test.ts @@ -101,7 +101,7 @@ describe("editor/operations: formatting operations", () => { expect(range.parts[0].text).toBe("world"); expect(model.serializeParts()).toEqual([{ text: "hello world!", type: "plain" }]); formatRange(range, Formatting.Italics); - expect(model.serializeParts()).toEqual([{ text: "hello _world_!", type: "plain" }]); + expect(model.serializeParts()).toEqual([{ text: "hello *world*!", type: "plain" }]); }); describe("escape backticks", () => { @@ -204,9 +204,9 @@ describe("editor/operations: formatting operations", () => { ]); formatRange(range, Formatting.Italics); expect(model.serializeParts()).toEqual([ - { text: "hello _there ", type: "plain" }, + { text: "hello *there ", type: "plain" }, { text: "@room", type: "at-room-pill" }, - { text: ", how are you_ doing?", type: "plain" }, + { text: ", how are you* doing?", type: "plain" }, ]); }); @@ -377,7 +377,7 @@ describe("editor/operations: formatting operations", () => { // We expect formatting to still happen in the first line as the caret should not jump down expect(model.serializeParts()).toEqual([ - { text: "hello _hello!_", type: "plain" }, + { text: "hello *hello!*", type: "plain" }, SERIALIZED_NEWLINE, { text: "world", type: "plain" }, ]); From 6b0cb75d82a5676b7d936e3fc10a2276c9630618 Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Wed, 15 May 2024 11:44:17 +0200 Subject: [PATCH 02/42] Use kdb in space panel shortcut (#12525) --- res/css/structures/_SpacePanel.pcss | 7 +++++++ src/accessibility/KeyboardShortcuts.ts | 1 + src/components/views/spaces/SpacePanel.tsx | 14 +++++--------- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/res/css/structures/_SpacePanel.pcss b/res/css/structures/_SpacePanel.pcss index 0252da01b7..d685617d5b 100644 --- a/res/css/structures/_SpacePanel.pcss +++ b/res/css/structures/_SpacePanel.pcss @@ -472,3 +472,10 @@ limitations under the License. .mx_SpacePanel_sharePublicSpace { margin: 0; } + +.mx_SpacePanel_Tooltip_KeyboardShortcut { + kbd { + font-family: inherit; + text-transform: capitalize; + } +} diff --git a/src/accessibility/KeyboardShortcuts.ts b/src/accessibility/KeyboardShortcuts.ts index 398dc27008..9a78f07df4 100644 --- a/src/accessibility/KeyboardShortcuts.ts +++ b/src/accessibility/KeyboardShortcuts.ts @@ -203,6 +203,7 @@ export const KEY_ICON: Record = { if (IS_MAC) { KEY_ICON[Key.META] = "⌘"; KEY_ICON[Key.ALT] = "⌥"; + KEY_ICON[Key.SHIFT] = "⇧"; } export const CATEGORIES: Record = { diff --git a/src/components/views/spaces/SpacePanel.tsx b/src/components/views/spaces/SpacePanel.tsx index a9b7093537..942be74bb9 100644 --- a/src/components/views/spaces/SpacePanel.tsx +++ b/src/components/views/spaces/SpacePanel.tsx @@ -62,17 +62,16 @@ import QuickSettingsButton from "./QuickSettingsButton"; import { useSettingValue } from "../../../hooks/useSettings"; import UserMenu from "../../structures/UserMenu"; import IndicatorScrollbar from "../../structures/IndicatorScrollbar"; -import { IS_MAC, Key } from "../../../Keyboard"; import { useDispatcher } from "../../../hooks/useDispatcher"; import defaultDispatcher from "../../../dispatcher/dispatcher"; import { ActionPayload } from "../../../dispatcher/payloads"; import { Action } from "../../../dispatcher/actions"; import { NotificationState } from "../../../stores/notifications/NotificationState"; -import { ALTERNATE_KEY_NAME } from "../../../accessibility/KeyboardShortcuts"; import { shouldShowComponent } from "../../../customisations/helpers/UIComponents"; import { UIComponent } from "../../../settings/UIFeature"; import { ThreadsActivityCentre } from "./threads-activity-centre/"; import AccessibleButton from "../elements/AccessibleButton"; +import { KeyboardShortcut } from "../settings/KeyboardShortcut"; const useSpaces = (): [Room[], MetaSpace[], Room[], SpaceKey] => { const invites = useEventEmitterState(SpaceStore.instance, UPDATE_INVITED_SPACES, () => { @@ -393,14 +392,11 @@ const SpacePanel: React.FC = () => { className={classNames("mx_SpacePanel_toggleCollapse", { expanded: !isPanelCollapsed })} onClick={() => setPanelCollapsed(!isPanelCollapsed)} title={isPanelCollapsed ? _t("action|expand") : _t("action|collapse")} - // TODO should use a kbd element for accessibility https://developer.mozilla.org/en-US/docs/Web/HTML/Element/kbd caption={ - IS_MAC - ? "⌘ + ⇧ + D" - : _t(ALTERNATE_KEY_NAME[Key.CONTROL]) + - " + " + - _t(ALTERNATE_KEY_NAME[Key.SHIFT]) + - " + D" + } /> From d184cacb6b115658844de6137f16fd3556665d61 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Wed, 15 May 2024 12:36:10 +0100 Subject: [PATCH 03/42] Update definitelyTyped (#12527) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- yarn.lock | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/yarn.lock b/yarn.lock index 2ed1cbc0ab..08e9c69938 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2733,9 +2733,9 @@ integrity sha512-HMwFiRujE5PjrgwHQ25+bsLJgowjGjm5Z8FVSf0N6PwgJrwxH0QxzHYDcKsTfV3wva0vzrpqMTJS2jXPr5BMEQ== "@types/lodash@^4.14.168": - version "4.17.0" - resolved "https://registry.yarnpkg.com/@types/lodash/-/lodash-4.17.0.tgz#d774355e41f372d5350a4d0714abb48194a489c3" - integrity sha512-t7dhREVv6dbNj0q17X12j7yDG4bD/DHYX7o5/DbDxobP0HnGPgpRz2Ej77aL7TZT3DSw13fqUTj8J4mMnqa7WA== + version "4.17.1" + resolved "https://registry.yarnpkg.com/@types/lodash/-/lodash-4.17.1.tgz#0fabfcf2f2127ef73b119d98452bd317c4a17eb8" + integrity sha512-X+2qazGS3jxLAIz5JDXDzglAF3KpijdhFxlf/V1+hEsOUc+HnWi81L/uv/EvGuV90WY+7mPGFCUDGfQC3Gj95Q== "@types/mapbox__point-geometry@*", "@types/mapbox__point-geometry@^0.1.2": version "0.1.4" @@ -2782,9 +2782,9 @@ undici-types "~5.26.4" "@types/node@18": - version "18.19.31" - resolved "https://registry.yarnpkg.com/@types/node/-/node-18.19.31.tgz#b7d4a00f7cb826b60a543cebdbda5d189aaecdcd" - integrity sha512-ArgCD39YpyyrtFKIqMDvjz79jto5fcI/SVUs2HwB+f0dAzq68yqOdyaSivLiLugSziTpNXLQrVb7RZFmdZzbhA== + version "18.19.33" + resolved "https://registry.yarnpkg.com/@types/node/-/node-18.19.33.tgz#98cd286a1b8a5e11aa06623210240bcc28e95c48" + integrity sha512-NR9+KrpSajr2qBVp/Yt5TU/rp+b5Mayi3+OlMlcg2cVCfRmcG5PWZ7S4+MG9PZ5gWBoc9Pd0BKSRViuBCRPu0A== dependencies: undici-types "~5.26.4" From a730e1d3d662aae333ed145cc8356b62f4281cda Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Wed, 15 May 2024 12:59:03 +0100 Subject: [PATCH 04/42] Update dependency @testing-library/jest-dom to v6.4.5 (#12528) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- yarn.lock | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/yarn.lock b/yarn.lock index 08e9c69938..9bca1b2e9c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2458,9 +2458,9 @@ pretty-format "^27.0.2" "@testing-library/jest-dom@^6.0.0": - version "6.4.2" - resolved "https://registry.yarnpkg.com/@testing-library/jest-dom/-/jest-dom-6.4.2.tgz#38949f6b63722900e2d75ba3c6d9bf8cffb3300e" - integrity sha512-CzqH0AFymEMG48CpzXFriYYkOjk6ZGPCLMhW9e9jg3KMCn5OfJecF8GtGW7yGfR/IgCe3SX8BSwjdzI6BBbZLw== + version "6.4.5" + resolved "https://registry.yarnpkg.com/@testing-library/jest-dom/-/jest-dom-6.4.5.tgz#badb40296477149136dabef32b572ddd3b56adf1" + integrity sha512-AguB9yvTXmCnySBP1lWjfNNUwpbElsaQ567lt2VdGqAdHtpieLgjmcVyv1q7PMIvLbgpDdkWV5Ydv3FEejyp2A== dependencies: "@adobe/css-tools" "^4.3.2" "@babel/runtime" "^7.9.2" @@ -2468,7 +2468,7 @@ chalk "^3.0.0" css.escape "^1.5.1" dom-accessibility-api "^0.6.3" - lodash "^4.17.15" + lodash "^4.17.21" redent "^3.0.0" "@testing-library/react-hooks@^8.0.1": @@ -6945,7 +6945,7 @@ lodash.truncate@^4.4.2: resolved "https://registry.yarnpkg.com/lodash.truncate/-/lodash.truncate-4.4.2.tgz#5a350da0b1113b837ecfffd5812cbe58d6eae193" integrity sha512-jttmRe7bRse52OsWIMDLaXxWqRAmtIUccAQ3garviCqJjafXOfNMO0yMfNpdD6zbGaTU0P5Nz7e7gAT6cKmJRw== -lodash@^4.17.15, lodash@^4.17.20, lodash@^4.17.21: +lodash@^4.17.20, lodash@^4.17.21: version "4.17.21" resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.21.tgz#679591c564c3bffaae8454cf0b3df370c3d6911c" integrity sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg== From 580bc8771c6ddaee2733a89e6816eb6f94fa18ba Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Wed, 15 May 2024 15:34:27 +0200 Subject: [PATCH 05/42] Fix avatar in chat export (#12537) * Update `@vector-im/compound-web` * Update test snapshots --- package.json | 2 +- .../views/elements/__snapshots__/AppTile-test.tsx.snap | 3 --- test/components/views/messages/TextualBody-test.tsx | 2 +- .../messages/__snapshots__/TextualBody-test.tsx.snap | 8 ++------ .../__snapshots__/PeopleRoomSettingsTab-test.tsx.snap | 1 - .../__snapshots__/AddExistingToSpaceDialog-test.tsx.snap | 1 - yarn.lock | 8 ++++---- 7 files changed, 8 insertions(+), 17 deletions(-) diff --git a/package.json b/package.json index 62eca6b344..181b2e77b9 100644 --- a/package.json +++ b/package.json @@ -76,7 +76,7 @@ "@sentry/browser": "^7.0.0", "@testing-library/react-hooks": "^8.0.1", "@vector-im/compound-design-tokens": "^1.2.0", - "@vector-im/compound-web": "^4.2.0", + "@vector-im/compound-web": "^4.3.1", "@zxcvbn-ts/core": "^3.0.4", "@zxcvbn-ts/language-common": "^3.0.4", "@zxcvbn-ts/language-en": "^3.0.2", diff --git a/test/components/views/elements/__snapshots__/AppTile-test.tsx.snap b/test/components/views/elements/__snapshots__/AppTile-test.tsx.snap index 4b84fa46c6..8f36256547 100644 --- a/test/components/views/elements/__snapshots__/AppTile-test.tsx.snap +++ b/test/components/views/elements/__snapshots__/AppTile-test.tsx.snap @@ -116,7 +116,6 @@ exports[`AppTile for a pinned widget should render 1`] = ` ", () => { const { container } = getComponent({ mxEvent: ev }); const content = container.querySelector(".mx_EventTile_body"); expect(content.innerHTML).toMatchInlineSnapshot( - `"Chat with Member"`, + `"Chat with Member"`, ); }); diff --git a/test/components/views/messages/__snapshots__/TextualBody-test.tsx.snap b/test/components/views/messages/__snapshots__/TextualBody-test.tsx.snap index aee9cc6d59..96bb641f9b 100644 --- a/test/components/views/messages/__snapshots__/TextualBody-test.tsx.snap +++ b/test/components/views/messages/__snapshots__/TextualBody-test.tsx.snap @@ -65,7 +65,6 @@ exports[` renders formatted m.text correctly pills appear for an renders formatted m.text correctly pills appear for eve renders formatted m.text correctly pills appear for roo renders formatted m.text correctly pills get injected c renders formatted m.text correctly pills get injected c `; -exports[` renders plain-text m.text correctly should pillify a permalink to a message in the same room with the label »Message from Member« 1`] = `"Visit Message from Member"`; +exports[` renders plain-text m.text correctly should pillify a permalink to a message in the same room with the label »Message from Member« 1`] = `"Visit Message from Member"`; -exports[` renders plain-text m.text correctly should pillify a permalink to an event in another room with the label »Message in Room 2« 1`] = `"Visit Message in Room 2"`; +exports[` renders plain-text m.text correctly should pillify a permalink to an event in another room with the label »Message in Room 2« 1`] = `"Visit Message in Room 2"`; exports[` renders plain-text m.text correctly should pillify a permalink to an unknown message in the same room with the label »Message« 1`] = ` looks as expected 1`] = ` Date: Wed, 15 May 2024 16:25:30 +0200 Subject: [PATCH 06/42] Tooltip: Improve accessibility for context menus (#12462) * Update `ContextMenuTooltipButton.tsx` * Fix placement * Update tests * Update space panel snapshot * Remove default placement * Update snapshots * Fix tooltip child rerender * Remove useless props title --- .../context_menu/ContextMenuTooltipButton.tsx | 10 +++++----- src/components/views/elements/AccessibleButton.tsx | 7 +++++++ .../spaces/__snapshots__/SpacePanel-test.tsx.snap | 4 ++-- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/accessibility/context_menu/ContextMenuTooltipButton.tsx b/src/accessibility/context_menu/ContextMenuTooltipButton.tsx index 2ae8a5de9d..3a3048d41f 100644 --- a/src/accessibility/context_menu/ContextMenuTooltipButton.tsx +++ b/src/accessibility/context_menu/ContextMenuTooltipButton.tsx @@ -18,9 +18,9 @@ limitations under the License. import React, { ComponentProps, forwardRef, Ref } from "react"; -import AccessibleTooltipButton from "../../components/views/elements/AccessibleTooltipButton"; +import AccessibleButton from "../../components/views/elements/AccessibleButton"; -type Props = ComponentProps> & { +type Props = ComponentProps> & { // whether the context menu is currently open isExpanded: boolean; }; @@ -31,17 +31,17 @@ export const ContextMenuTooltipButton = forwardRef(function , ) { return ( - {children} - + ); }); diff --git a/src/components/views/elements/AccessibleButton.tsx b/src/components/views/elements/AccessibleButton.tsx index d94162393e..76b90506dc 100644 --- a/src/components/views/elements/AccessibleButton.tsx +++ b/src/components/views/elements/AccessibleButton.tsx @@ -106,6 +106,11 @@ type Props = DynamicHtmlElementProps & * Callback for when the tooltip is opened or closed. */ onTooltipOpenChange?: TooltipProps["onOpenChange"]; + + /** + * Whether the tooltip should be disabled. + */ + disableTooltip?: TooltipProps["disabled"]; }; /** @@ -140,6 +145,7 @@ const AccessibleButton = forwardRef(function , ref: Ref, @@ -217,6 +223,7 @@ const AccessibleButton = forwardRef(function {button} diff --git a/test/components/views/spaces/__snapshots__/SpacePanel-test.tsx.snap b/test/components/views/spaces/__snapshots__/SpacePanel-test.tsx.snap index 22735eb9e7..8a155b3033 100644 --- a/test/components/views/spaces/__snapshots__/SpacePanel-test.tsx.snap +++ b/test/components/views/spaces/__snapshots__/SpacePanel-test.tsx.snap @@ -229,8 +229,8 @@ exports[` should show all activated MetaSpaces in the correct orde class="mx_ThreadsActivityCentre_container" > + ); } diff --git a/src/components/views/rooms/MessageComposerFormatBar.tsx b/src/components/views/rooms/MessageComposerFormatBar.tsx index 5893540528..04406158ae 100644 --- a/src/components/views/rooms/MessageComposerFormatBar.tsx +++ b/src/components/views/rooms/MessageComposerFormatBar.tsx @@ -18,7 +18,7 @@ import React, { createRef } from "react"; import classNames from "classnames"; import { _t } from "../../../languageHandler"; -import { RovingAccessibleTooltipButton } from "../../../accessibility/RovingTabIndex"; +import { RovingAccessibleButton } from "../../../accessibility/RovingTabIndex"; import Toolbar from "../../../accessibility/Toolbar"; export enum Formatting { @@ -131,7 +131,7 @@ class FormatButton extends React.PureComponent { // element="button" and type="button" are necessary for the buttons to work on WebKit, // otherwise the text is deselected before onClick can ever be called return ( -
Date: Fri, 17 May 2024 16:11:07 +0200 Subject: [PATCH 20/42] Tooltip: migrate remaining tooltips from `AccessibleTooltipButton` to `AccessibleButton` (#12522) * Use `AccessibleButton` in `RovingAccessibleTooltipButton` * Update snapshots * Update @vector-im/compound-web * Update composer * Update formating buttons * Update snapshots * Update `ContextMenuTooltipButton.tsx` * Fix placement * Update tests * Remove placement * Update space panel snapshot * Remove default placement * Update snapshots * Update snapshots * Use kbd * Update ``@vector-im/compound-web` * Migrate remaining files * Remove `AccessibleTooltipButton.tsx` * Add test to `InteractiveAuthEntryComponents` * Add test to `InteractiveAuthEntryComponents` * Back to old RoomList-test.tsx * Improve `InteractiveAuthEntryComponent` tests * Review changes --- .../auth/InteractiveAuthEntryComponents.tsx | 13 +- src/components/views/beta/BetaCard.tsx | 15 +-- .../elements/AccessibleTooltipButton.tsx | 118 ------------------ .../devices/DeviceExpandDetailsButton.tsx | 8 +- .../InteractiveAuthEntryComponents-test.tsx | 65 ++++++++++ ...teractiveAuthEntryComponents-test.tsx.snap | 34 +++++ 6 files changed, 114 insertions(+), 139 deletions(-) delete mode 100644 src/components/views/elements/AccessibleTooltipButton.tsx create mode 100644 test/components/views/auth/InteractiveAuthEntryComponents-test.tsx create mode 100644 test/components/views/auth/__snapshots__/InteractiveAuthEntryComponents-test.tsx.snap diff --git a/src/components/views/auth/InteractiveAuthEntryComponents.tsx b/src/components/views/auth/InteractiveAuthEntryComponents.tsx index 2bc0fe80db..7bed60d603 100644 --- a/src/components/views/auth/InteractiveAuthEntryComponents.tsx +++ b/src/components/views/auth/InteractiveAuthEntryComponents.tsx @@ -26,10 +26,8 @@ import SettingsStore from "../../../settings/SettingsStore"; import { LocalisedPolicy, Policies } from "../../../Terms"; import { AuthHeaderModifier } from "../../structures/auth/header/AuthHeaderModifier"; import AccessibleButton, { AccessibleButtonKind, ButtonEvent } from "../elements/AccessibleButton"; -import AccessibleTooltipButton from "../elements/AccessibleTooltipButton"; import Field from "../elements/Field"; import Spinner from "../elements/Spinner"; -import { Alignment } from "../elements/Tooltip"; import CaptchaForm from "./CaptchaForm"; /* This file contains a collection of components which are used by the @@ -501,15 +499,16 @@ export class EmailIdentityAuthEntry extends React.Component< {}, { a: (text: string) => ( - this.setState({ requested: false }) + ? (open) => { + if (!open) this.setState({ requested: false }); + } : undefined } onClick={async (): Promise => { @@ -524,7 +523,7 @@ export class EmailIdentityAuthEntry extends React.Component< }} > {text} - + ), }, )} diff --git a/src/components/views/beta/BetaCard.tsx b/src/components/views/beta/BetaCard.tsx index 84c7a27fe0..7d17c3af94 100644 --- a/src/components/views/beta/BetaCard.tsx +++ b/src/components/views/beta/BetaCard.tsx @@ -27,7 +27,6 @@ import SdkConfig from "../../../SdkConfig"; import SettingsFlag from "../elements/SettingsFlag"; import { useFeatureEnabled } from "../../../hooks/useSettings"; import InlineSpinner from "../elements/InlineSpinner"; -import AccessibleTooltipButton from "../elements/AccessibleTooltipButton"; import { shouldShowFeedback } from "../../../utils/Feedback"; // XXX: Keep this around for re-use in future Betas @@ -50,19 +49,15 @@ export const BetaPill: React.FC = ({ }) => { if (onClick) { return ( - -
{tooltipTitle}
-
{tooltipCaption}
-
- } + aria-label={`${tooltipTitle} ${tooltipCaption}`} + title={tooltipTitle} + caption={tooltipCaption} onClick={onClick} > {_t("common|beta")} - + ); } diff --git a/src/components/views/elements/AccessibleTooltipButton.tsx b/src/components/views/elements/AccessibleTooltipButton.tsx deleted file mode 100644 index 759643da1c..0000000000 --- a/src/components/views/elements/AccessibleTooltipButton.tsx +++ /dev/null @@ -1,118 +0,0 @@ -/* -Copyright 2019 Michael Telatynski <7t3chguy@gmail.com> -Copyright 2019 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 React, { SyntheticEvent, FocusEvent, forwardRef, useEffect, Ref, useState, ComponentProps } from "react"; - -import AccessibleButton from "./AccessibleButton"; -import Tooltip, { Alignment } from "./Tooltip"; - -/** - * Type of props accepted by {@link AccessibleTooltipButton}. - * - * Extends that of {@link AccessibleButton}. - */ -type Props = ComponentProps> & { - /** - * Title to show in the tooltip and use as aria-label - */ - title?: string; - /** - * Tooltip node to show in the tooltip, takes precedence over `title` - */ - tooltip?: React.ReactNode; - /** - * Trigger label to render - */ - label?: string; - /** - * Classname to apply to the tooltip - */ - tooltipClassName?: string; - /** - * Force the tooltip to be hidden - */ - forceHide?: boolean; - /** - * Alignment to render the tooltip with - */ - alignment?: Alignment; - /** - * Function to call when the children are hovered over - */ - onHover?: (hovering: boolean) => void; - /** - * Function to call when the tooltip goes from shown to hidden. - */ - onHideTooltip?(ev: SyntheticEvent): void; -}; - -/** - * @deprecated use AccessibleButton with `title` and `caption` instead. - */ -const AccessibleTooltipButton = forwardRef(function ( - { title, tooltip, children, forceHide, alignment, onHideTooltip, tooltipClassName, element, ...props }: Props, - ref: Ref, -) { - const [hover, setHover] = useState(false); - - useEffect(() => { - // If forceHide is set then force hover to off to hide the tooltip - if (forceHide && hover) { - setHover(false); - } - }, [forceHide, hover]); - - const showTooltip = (): void => { - props.onHover?.(true); - if (forceHide) return; - setHover(true); - }; - - const hideTooltip = (ev: SyntheticEvent): void => { - props.onHover?.(false); - setHover(false); - onHideTooltip?.(ev); - }; - - const onFocus = (ev: FocusEvent): void => { - // We only show the tooltip if focus arrived here from some other - // element, to avoid leaving tooltips hanging around when a modal closes - if (ev.relatedTarget) showTooltip(); - }; - - const tip = hover && (title || tooltip) && ( - - ); - return ( - - {children} - {props.label} - {(tooltip || title) && tip} - - ); -}); - -export default AccessibleTooltipButton; diff --git a/src/components/views/settings/devices/DeviceExpandDetailsButton.tsx b/src/components/views/settings/devices/DeviceExpandDetailsButton.tsx index 317afdfca1..791b612591 100644 --- a/src/components/views/settings/devices/DeviceExpandDetailsButton.tsx +++ b/src/components/views/settings/devices/DeviceExpandDetailsButton.tsx @@ -19,10 +19,10 @@ import React, { ComponentProps } from "react"; import { Icon as CaretIcon } from "../../../../../res/img/feather-customised/dropdown-arrow.svg"; import { _t } from "../../../../languageHandler"; -import AccessibleTooltipButton from "../../elements/AccessibleTooltipButton"; +import AccessibleButton from "../../elements/AccessibleButton"; type Props = Omit< - ComponentProps>, + ComponentProps>, "aria-label" | "title" | "kind" | "className" | "onClick" | "element" > & { isExpanded: boolean; @@ -36,7 +36,7 @@ export const DeviceExpandDetailsButton = }: Props): JSX.Element => { const label = isExpanded ? _t("settings|sessions|hide_details") : _t("settings|sessions|show_details"); return ( - onClick={onClick} > - + ); }; diff --git a/test/components/views/auth/InteractiveAuthEntryComponents-test.tsx b/test/components/views/auth/InteractiveAuthEntryComponents-test.tsx new file mode 100644 index 0000000000..e6e3e1383e --- /dev/null +++ b/test/components/views/auth/InteractiveAuthEntryComponents-test.tsx @@ -0,0 +1,65 @@ +/* + * Copyright 2024 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 React from "react"; +import { render, screen, waitFor, act } from "@testing-library/react"; +import { AuthType } from "matrix-js-sdk/src/interactive-auth"; +import userEvent from "@testing-library/user-event"; + +import { EmailIdentityAuthEntry } from "../../../../src/components/views/auth/InteractiveAuthEntryComponents"; +import { createTestClient } from "../../../test-utils"; + +describe("", () => { + const renderIdentityAuth = () => { + const matrixClient = createTestClient(); + + return render( + , + ); + }; + + test("should render", () => { + const { container } = renderIdentityAuth(); + expect(container).toMatchSnapshot(); + }); + + test("should clear the requested state when the button tooltip is hidden", async () => { + renderIdentityAuth(); + + // After a click on the resend button, the button should display the resent label + screen.getByRole("button", { name: "Resend" }).click(); + await waitFor(() => expect(screen.queryByRole("button", { name: "Resent!" })).toBeInTheDocument()); + expect(screen.queryByRole("button", { name: "Resend" })).toBeNull(); + + const resentButton = screen.getByRole("button", { name: "Resent!" }); + // Hover briefly the button and wait for the tooltip to be displayed + await userEvent.hover(resentButton); + await waitFor(() => expect(screen.getByRole("tooltip", { name: "Resent!" })).toBeInTheDocument()); + + // On unhover, it should display again the resend button + await act(() => userEvent.unhover(resentButton)); + await waitFor(() => expect(screen.queryByRole("button", { name: "Resend" })).toBeInTheDocument()); + }); +}); diff --git a/test/components/views/auth/__snapshots__/InteractiveAuthEntryComponents-test.tsx.snap b/test/components/views/auth/__snapshots__/InteractiveAuthEntryComponents-test.tsx.snap new file mode 100644 index 0000000000..65f86a35d2 --- /dev/null +++ b/test/components/views/auth/__snapshots__/InteractiveAuthEntryComponents-test.tsx.snap @@ -0,0 +1,34 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[` should render 1`] = ` +
+
+

+ + To create your account, open the link in the email we just sent to + + alice@example.xyz + + . + +

+

+ + Did not receive it? +

+ +

+
+
+`; From 3e103941d67b8f7e11637ed11caf839217857afb Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Fri, 17 May 2024 17:19:31 +0100 Subject: [PATCH 21/42] Cleanup work on `DecryptionFailureTracker` (#12546) * Inline `DecryptionFailureTracker.addDecryptionFailure` * Remove redundant TRACK_INTERVAL There really doesn't seem to be much point to this batching up of decryption failure reports. We still call the analytics callback the same number of times. * Rename `trackedEvents` to `reportedEvents` * Fix incorrect documentation on `visibleEvents` This *does* overlap with `failures`. * Combine `addFailure` and `reportFailure` * Calculate client properties before starting reporting --- src/DecryptionFailureTracker.ts | 111 +++++++++-------------- src/components/structures/MatrixChat.tsx | 2 +- test/DecryptionFailureTracker-test.ts | 51 +---------- 3 files changed, 51 insertions(+), 113 deletions(-) diff --git a/src/DecryptionFailureTracker.ts b/src/DecryptionFailureTracker.ts index 91e5a3f620..e8628f5d66 100644 --- a/src/DecryptionFailureTracker.ts +++ b/src/DecryptionFailureTracker.ts @@ -93,33 +93,24 @@ export class DecryptionFailureTracker { * Every `CHECK_INTERVAL_MS`, this map is checked for failures that happened > * `MAXIMUM_LATE_DECRYPTION_PERIOD` ago (considered undecryptable), or * decryptions that took > `GRACE_PERIOD_MS` (considered late decryptions). - * These are accumulated in `failuresToReport`. + * + * Any such events are then reported via the `TrackingFn`. */ public failures: Map = new Map(); /** Set of event IDs that have been visible to the user. * - * This will only contain events that are not already in `failures` or in - * `trackedEvents`. + * This will only contain events that are not already in `reportedEvents`. */ public visibleEvents: Set = new Set(); - /** The failures that will be reported at the next tracking interval. These are - * events that we have decided are undecryptable due to exceeding the - * `MAXIMUM_LATE_DECRYPTION_PERIOD`, or that we decrypted but we consider as late - * decryptions. */ - public failuresToReport: Set = new Set(); - - /** Event IDs of failures that were tracked previously */ - public trackedEvents: Set = new Set(); + /** Event IDs of failures that were reported previously */ + private reportedEvents: Set = new Set(); /** Set to an interval ID when `start` is called */ public checkInterval: number | null = null; public trackInterval: number | null = null; - /** Spread the load on `Analytics` by tracking at a low frequency, `TRACK_INTERVAL_MS`. */ - public static TRACK_INTERVAL_MS = 60000; - /** Call `checkFailures` every `CHECK_INTERVAL_MS`. */ public static CHECK_INTERVAL_MS = 40000; @@ -181,12 +172,12 @@ export class DecryptionFailureTracker { return DecryptionFailureTracker.internalInstance; } - // loadTrackedEvents() { - // this.trackedEvents = new Set(JSON.parse(localStorage.getItem('mx-decryption-failure-event-ids')) || []); + // loadReportedEvents() { + // this.reportedEvents = new Set(JSON.parse(localStorage.getItem('mx-decryption-failure-event-ids')) || []); // } - // saveTrackedEvents() { - // localStorage.setItem('mx-decryption-failure-event-ids', JSON.stringify([...this.trackedEvents])); + // saveReportedEvents() { + // localStorage.setItem('mx-decryption-failure-event-ids', JSON.stringify([...this.reportedEvents])); // } /** Callback for when an event is decrypted. @@ -195,7 +186,7 @@ export class DecryptionFailureTracker { * handler after a decryption attempt on an event, whether the decryption * is successful or not. * - * @param matrixEvent the event that was decrypted + * @param e the event that was decrypted * * @param nowTs the current timestamp */ @@ -213,6 +204,11 @@ export class DecryptionFailureTracker { const eventId = e.getId()!; + // if it's already reported, we don't need to do anything + if (this.reportedEvents.has(eventId)) { + return; + } + // if we already have a record of this event, use the previously-recorded timestamp const failure = this.failures.get(eventId); const ts = failure ? failure.ts : nowTs; @@ -223,8 +219,10 @@ export class DecryptionFailureTracker { if (this.userDomain !== undefined && senderDomain !== undefined) { isFederated = this.userDomain !== senderDomain; } + const wasVisibleToUser = this.visibleEvents.has(eventId); - this.addDecryptionFailure( + this.failures.set( + eventId, new DecryptionFailure(eventId, errCode, ts, isFederated, wasVisibleToUser, this.userTrustsOwnIdentity), ); } @@ -233,7 +231,7 @@ export class DecryptionFailureTracker { const eventId = e.getId()!; // if it's already reported, we don't need to do anything - if (this.trackedEvents.has(eventId)) { + if (this.reportedEvents.has(eventId)) { return; } @@ -247,16 +245,6 @@ export class DecryptionFailureTracker { this.visibleEvents.add(eventId); } - public addDecryptionFailure(failure: DecryptionFailure): void { - const eventId = failure.failedEventId; - - if (this.trackedEvents.has(eventId)) { - return; - } - - this.failures.set(eventId, failure); - } - public removeDecryptionFailuresForEvent(e: MatrixEvent, nowTs: number): void { const eventId = e.getId()!; const failure = this.failures.get(eventId); @@ -274,7 +262,7 @@ export class DecryptionFailureTracker { // undecryptable, and leave timeToDecryptMillis undefined failure.timeToDecryptMillis = timeToDecryptMillis; } - this.failuresToReport.add(failure); + this.reportFailure(failure); } } @@ -301,15 +289,13 @@ export class DecryptionFailureTracker { /** * Start checking for and tracking failures. */ - public start(client: MatrixClient): void { - this.calculateClientProperties(client); + public async start(client: MatrixClient): Promise { + await this.calculateClientProperties(client); this.registerHandlers(client); this.checkInterval = window.setInterval( () => this.checkFailures(Date.now()), DecryptionFailureTracker.CHECK_INTERVAL_MS, ); - - this.trackInterval = window.setInterval(() => this.trackFailures(), DecryptionFailureTracker.TRACK_INTERVAL_MS); } private async calculateClientProperties(client: MatrixClient): Promise { @@ -370,7 +356,6 @@ export class DecryptionFailureTracker { this.userTrustsOwnIdentity = undefined; this.failures = new Map(); this.visibleEvents = new Set(); - this.failuresToReport = new Set(); } /** @@ -392,7 +377,7 @@ export class DecryptionFailureTracker { // - we haven't decrypted yet and it's past the time for it to be // considered a "late" decryption, so we count it as // undecryptable. - this.addFailure(eventId, failure); + this.reportFailure(failure); } else { // the event isn't old enough, so we still need to keep track of it failuresNotReady.set(eventId, failure); @@ -402,40 +387,34 @@ export class DecryptionFailureTracker { // Commented out for now for expediency, we need to consider unbound nature of storing // this in localStorage - // this.saveTrackedEvents(); - } - - private addFailure(eventId: string, failure: DecryptionFailure): void { - this.failuresToReport.add(failure); - this.trackedEvents.add(eventId); - // once we've added it to trackedEvents, we won't check - // visibleEvents for it any more - this.visibleEvents.delete(eventId); + // this.saveReportedEvents(); } /** * If there are failures that should be tracked, call the given trackDecryptionFailure * function with the failures that should be tracked. */ - public trackFailures(): void { - for (const failure of this.failuresToReport) { - const errorCode = failure.errorCode; - const trackedErrorCode = this.errorCodeMapFn(errorCode); - const properties: ErrorProperties = { - timeToDecryptMillis: failure.timeToDecryptMillis ?? -1, - wasVisibleToUser: failure.wasVisibleToUser, - }; - if (failure.isFederated !== undefined) { - properties.isFederated = failure.isFederated; - } - if (failure.userTrustsOwnIdentity !== undefined) { - properties.userTrustsOwnIdentity = failure.userTrustsOwnIdentity; - } - if (this.baseProperties) { - Object.assign(properties, this.baseProperties); - } - this.fn(trackedErrorCode, errorCode, properties); + private reportFailure(failure: DecryptionFailure): void { + const errorCode = failure.errorCode; + const trackedErrorCode = this.errorCodeMapFn(errorCode); + const properties: ErrorProperties = { + timeToDecryptMillis: failure.timeToDecryptMillis ?? -1, + wasVisibleToUser: failure.wasVisibleToUser, + }; + if (failure.isFederated !== undefined) { + properties.isFederated = failure.isFederated; } - this.failuresToReport = new Set(); + if (failure.userTrustsOwnIdentity !== undefined) { + properties.userTrustsOwnIdentity = failure.userTrustsOwnIdentity; + } + if (this.baseProperties) { + Object.assign(properties, this.baseProperties); + } + this.fn(trackedErrorCode, errorCode, properties); + + this.reportedEvents.add(failure.failedEventId); + // once we've added it to reportedEvents, we won't check + // visibleEvents for it any more + this.visibleEvents.delete(failure.failedEventId); } } diff --git a/src/components/structures/MatrixChat.tsx b/src/components/structures/MatrixChat.tsx index b755f42692..29a3a2a779 100644 --- a/src/components/structures/MatrixChat.tsx +++ b/src/components/structures/MatrixChat.tsx @@ -1591,7 +1591,7 @@ export default class MatrixChat extends React.PureComponent { // tracked events across sessions. // dft.loadTrackedEventHashMap(); - dft.start(cli); + dft.start(cli).catch((e) => logger.error("Unable to start DecryptionFailureTracker", e)); cli.on(ClientEvent.Room, (room) => { if (cli.isCryptoEnabled()) { diff --git a/test/DecryptionFailureTracker-test.ts b/test/DecryptionFailureTracker-test.ts index 74dfd5b724..ef5e01719f 100644 --- a/test/DecryptionFailureTracker-test.ts +++ b/test/DecryptionFailureTracker-test.ts @@ -55,9 +55,6 @@ describe("DecryptionFailureTracker", function () { // Pretend "now" is Infinity tracker.checkFailures(Infinity); - // Immediately track the newest failures - tracker.trackFailures(); - // should track a failure for an event that failed decryption expect(count).not.toBe(0); }); @@ -84,9 +81,6 @@ describe("DecryptionFailureTracker", function () { // Pretend "now" is Infinity tracker.checkFailures(Infinity); - // Immediately track the newest failures - tracker.trackFailures(); - // should track a failure for an event that failed decryption expect(count).not.toBe(0); @@ -110,9 +104,6 @@ describe("DecryptionFailureTracker", function () { // Pretend "now" is Infinity tracker.checkFailures(Infinity); - // Immediately track the newest failures - tracker.trackFailures(); - // should track a failure for an event that failed decryption expect(count).not.toBe(0); }); @@ -151,8 +142,6 @@ describe("DecryptionFailureTracker", function () { // Pretend "now" is Infinity tracker.checkFailures(Infinity); - tracker.trackFailures(); - expect(propertiesByErrorCode[error1].wasVisibleToUser).toBe(true); expect(propertiesByErrorCode[error2].wasVisibleToUser).toBe(true); expect(propertiesByErrorCode[error3].wasVisibleToUser).toBe(false); @@ -181,9 +170,6 @@ describe("DecryptionFailureTracker", function () { // Pretend "now" is Infinity tracker.checkFailures(Infinity); - - // Immediately track the newest failures - tracker.trackFailures(); }); it( @@ -213,9 +199,6 @@ describe("DecryptionFailureTracker", function () { // Pretend "now" is Infinity tracker.checkFailures(Infinity); - - // Immediately track the newest failures - tracker.trackFailures(); }, ); @@ -247,11 +230,9 @@ describe("DecryptionFailureTracker", function () { // Pretend "now" is Infinity tracker.checkFailures(Infinity); - // Simulated polling of `trackFailures`, an arbitrary number ( > 2 ) times - tracker.trackFailures(); - tracker.trackFailures(); - tracker.trackFailures(); - tracker.trackFailures(); + // Simulated polling of `checkFailures`, an arbitrary number ( > 2 ) times + tracker.checkFailures(Infinity); + tracker.checkFailures(Infinity); // should only track a single failure per event expect(count).toBe(2); @@ -275,12 +256,9 @@ describe("DecryptionFailureTracker", function () { // Pretend "now" is Infinity tracker.checkFailures(Infinity); - tracker.trackFailures(); - // Indicate a second decryption, after having tracked the failure eventDecrypted(tracker, decryptedEvent, Date.now()); - - tracker.trackFailures(); + tracker.checkFailures(Infinity); // should only track a single failure per event expect(count).toBe(1); @@ -308,8 +286,6 @@ describe("DecryptionFailureTracker", function () { // NB: This saves to localStorage specific to DFT tracker.checkFailures(Infinity); - tracker.trackFailures(); - // Simulate the browser refreshing by destroying tracker and creating a new tracker // @ts-ignore access to private constructor const secondTracker = new DecryptionFailureTracker( @@ -323,7 +299,6 @@ describe("DecryptionFailureTracker", function () { eventDecrypted(secondTracker, decryptedEvent, Date.now()); secondTracker.checkFailures(Infinity); - secondTracker.trackFailures(); // should only track a single failure per event expect(count).toBe(1); @@ -363,8 +338,6 @@ describe("DecryptionFailureTracker", function () { // Pretend "now" is Infinity tracker.checkFailures(Infinity); - tracker.trackFailures(); - //expect(counts['UnknownError']).toBe(1, 'should track one UnknownError'); expect(counts["OlmKeysNotSentError"]).toBe(2); }); @@ -400,8 +373,6 @@ describe("DecryptionFailureTracker", function () { // Pretend "now" is Infinity tracker.checkFailures(Infinity); - tracker.trackFailures(); - expect(counts["OlmUnspecifiedError"]).toBe(3); }); @@ -423,8 +394,6 @@ describe("DecryptionFailureTracker", function () { // Pretend "now" is Infinity tracker.checkFailures(Infinity); - tracker.trackFailures(); - // should track remapped error code expect(counts["XEDNI_EGASSEM_NWONKNU_MLO"]).toBe(1); }); @@ -488,8 +457,6 @@ describe("DecryptionFailureTracker", function () { // Pretend "now" is Infinity tracker.checkFailures(Infinity); - tracker.trackFailures(); - expect(errorCodes).toEqual([ "OlmKeysNotSentError", "OlmIndexError", @@ -546,8 +513,6 @@ describe("DecryptionFailureTracker", function () { // Pretend "now" is Infinity tracker.checkFailures(Infinity); - tracker.trackFailures(); - expect(propertiesByErrorCode[error1].timeToDecryptMillis).toEqual(40000); expect(propertiesByErrorCode[error2].timeToDecryptMillis).toEqual(-1); expect(propertiesByErrorCode[error3].timeToDecryptMillis).toEqual(-1); @@ -576,14 +541,13 @@ describe("DecryptionFailureTracker", function () { // long enough for the timers to fire, but we'll use fake timers just // to be safe. jest.useFakeTimers(); - tracker.start(client); + await tracker.start(client); // If the client fails to decrypt, it should get tracked const failedDecryption = await createFailedDecryptionEvent(); client.emit(MatrixEventEvent.Decrypted, failedDecryption); tracker.checkFailures(Infinity); - tracker.trackFailures(); expect(errorCount).toEqual(1); @@ -597,7 +561,6 @@ describe("DecryptionFailureTracker", function () { // Pretend "now" is Infinity tracker.checkFailures(Infinity); - tracker.trackFailures(); expect(errorCount).toEqual(1); @@ -655,8 +618,6 @@ describe("DecryptionFailureTracker", function () { // Pretend "now" is Infinity tracker.checkFailures(Infinity); - tracker.trackFailures(); - expect(propertiesByErrorCode[error1].isMatrixDotOrg).toBe(true); expect(propertiesByErrorCode[error1].cryptoSDK).toEqual("Rust"); @@ -677,7 +638,6 @@ describe("DecryptionFailureTracker", function () { tracker.addVisibleEvent(anotherFailure); eventDecrypted(tracker, anotherFailure, now); tracker.checkFailures(Infinity); - tracker.trackFailures(); expect(propertiesByErrorCode[error3].isMatrixDotOrg).toBe(false); expect(propertiesByErrorCode[error3].cryptoSDK).toEqual("Legacy"); }); @@ -707,7 +667,6 @@ describe("DecryptionFailureTracker", function () { // Pretend "now" is Infinity tracker.checkFailures(Infinity); - tracker.trackFailures(); // the time to decrypt should be relative to the first time we failed // to decrypt, not the second From 1bb70c5b3bf372a0ad0010ba1725085593fd5761 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Mon, 20 May 2024 10:53:50 -0400 Subject: [PATCH 22/42] Only report undecryptable events once (#12501) * persist previously-reported event IDs as a bloom filter * Pin to older `@types/seedrandom` ... to work around https://github.com/Callidon/bloom-filters/issues/72 * Inline `DecryptionFailureTracker.addDecryptionFailure` * Remove redundant TRACK_INTERVAL There really doesn't seem to be much point to this batching up of decryption failure reports. We still call the analytics callback the same number of times. * Rename `trackedEvents` to `reportedEvents` * Fix incorrect documentation on `visibleEvents` This *does* overlap with `failures`. * Combine `addFailure` and `reportFailure` * Calculate client properties before starting reporting * Clear localstorage after each test ... otherwise they interfere * Remove redundant comment * Ensure that reports are cleared on a logout/login cycle * make private const private and const --------- Co-authored-by: Richard van der Hoff --- package.json | 2 + src/DecryptionFailureTracker.ts | 30 ++++-- src/components/structures/MatrixChat.tsx | 14 +-- test/DecryptionFailureTracker-test.ts | 124 ++++++++++++++++++----- yarn.lock | 100 ++++++++++++------ 5 files changed, 197 insertions(+), 73 deletions(-) diff --git a/package.json b/package.json index 74914b522a..84aff16103 100644 --- a/package.json +++ b/package.json @@ -81,6 +81,7 @@ "@zxcvbn-ts/language-common": "^3.0.4", "@zxcvbn-ts/language-en": "^3.0.2", "await-lock": "^2.1.0", + "bloom-filters": "^3.0.1", "blurhash": "^2.0.3", "classnames": "^2.2.6", "commonmark": "^0.31.0", @@ -182,6 +183,7 @@ "@types/react-transition-group": "^4.4.0", "@types/sanitize-html": "2.11.0", "@types/sdp-transform": "^2.4.6", + "@types/seedrandom": "<3.0.5", "@types/tar-js": "^0.3.2", "@types/ua-parser-js": "^0.7.36", "@types/uuid": "^9.0.2", diff --git a/src/DecryptionFailureTracker.ts b/src/DecryptionFailureTracker.ts index e8628f5d66..cc336cc5ce 100644 --- a/src/DecryptionFailureTracker.ts +++ b/src/DecryptionFailureTracker.ts @@ -14,12 +14,16 @@ See the License for the specific language governing permissions and limitations under the License. */ +import { ScalableBloomFilter } from "bloom-filters"; import { CryptoEvent, HttpApiEvent, MatrixClient, MatrixEventEvent, MatrixEvent } from "matrix-js-sdk/src/matrix"; import { Error as ErrorEvent } from "@matrix-org/analytics-events/types/typescript/Error"; import { DecryptionFailureCode } from "matrix-js-sdk/src/crypto-api"; import { PosthogAnalytics } from "./PosthogAnalytics"; +/** The key that we use to store the `reportedEvents` bloom filter in localstorage */ +const DECRYPTION_FAILURE_STORAGE_KEY = "mx_decryption_failure_event_ids"; + export class DecryptionFailure { /** * The time between our initial failure to decrypt and our successful @@ -104,8 +108,8 @@ export class DecryptionFailureTracker { */ public visibleEvents: Set = new Set(); - /** Event IDs of failures that were reported previously */ - private reportedEvents: Set = new Set(); + /** Bloom filter tracking event IDs of failures that were reported previously */ + private reportedEvents: ScalableBloomFilter = new ScalableBloomFilter(); /** Set to an interval ID when `start` is called */ public checkInterval: number | null = null; @@ -172,13 +176,18 @@ export class DecryptionFailureTracker { return DecryptionFailureTracker.internalInstance; } - // loadReportedEvents() { - // this.reportedEvents = new Set(JSON.parse(localStorage.getItem('mx-decryption-failure-event-ids')) || []); - // } + private loadReportedEvents(): void { + const storedFailures = localStorage.getItem(DECRYPTION_FAILURE_STORAGE_KEY); + if (storedFailures) { + this.reportedEvents = ScalableBloomFilter.fromJSON(JSON.parse(storedFailures)); + } else { + this.reportedEvents = new ScalableBloomFilter(); + } + } - // saveReportedEvents() { - // localStorage.setItem('mx-decryption-failure-event-ids', JSON.stringify([...this.reportedEvents])); - // } + private saveReportedEvents(): void { + localStorage.setItem(DECRYPTION_FAILURE_STORAGE_KEY, JSON.stringify(this.reportedEvents.saveAsJSON())); + } /** Callback for when an event is decrypted. * @@ -290,6 +299,7 @@ export class DecryptionFailureTracker { * Start checking for and tracking failures. */ public async start(client: MatrixClient): Promise { + this.loadReportedEvents(); await this.calculateClientProperties(client); this.registerHandlers(client); this.checkInterval = window.setInterval( @@ -385,9 +395,7 @@ export class DecryptionFailureTracker { } this.failures = failuresNotReady; - // Commented out for now for expediency, we need to consider unbound nature of storing - // this in localStorage - // this.saveReportedEvents(); + this.saveReportedEvents(); } /** diff --git a/src/components/structures/MatrixChat.tsx b/src/components/structures/MatrixChat.tsx index 29a3a2a779..5e50f68b48 100644 --- a/src/components/structures/MatrixChat.tsx +++ b/src/components/structures/MatrixChat.tsx @@ -23,8 +23,8 @@ import { MatrixClient, MatrixEvent, RoomType, - SyncStateData, SyncState, + SyncStateData, TimelineEvents, } from "matrix-js-sdk/src/matrix"; import { defer, IDeferred, QueryDict } from "matrix-js-sdk/src/utils"; @@ -128,7 +128,7 @@ import { TimelineRenderingType } from "../../contexts/RoomContext"; import { UseCaseSelection } from "../views/elements/UseCaseSelection"; import { ValidatedServerConfig } from "../../utils/ValidatedServerConfig"; import { isLocalRoom } from "../../utils/localRoom/isLocalRoom"; -import { SdkContextClass, SDKContext } from "../../contexts/SDKContext"; +import { SDKContext, SdkContextClass } from "../../contexts/SDKContext"; import { viewUserDeviceSettings } from "../../actions/handlers/viewUserDeviceSettings"; import { cleanUpBroadcasts, VoiceBroadcastResumer } from "../../voice-broadcast"; import GenericToast from "../views/toasts/GenericToast"; @@ -1585,13 +1585,9 @@ export default class MatrixChat extends React.PureComponent { ); }); - const dft = DecryptionFailureTracker.instance; - - // Shelved for later date when we have time to think about persisting history of - // tracked events across sessions. - // dft.loadTrackedEventHashMap(); - - dft.start(cli).catch((e) => logger.error("Unable to start DecryptionFailureTracker", e)); + DecryptionFailureTracker.instance + .start(cli) + .catch((e) => logger.error("Unable to start DecryptionFailureTracker", e)); cli.on(ClientEvent.Room, (room) => { if (cli.isCryptoEnabled()) { diff --git a/test/DecryptionFailureTracker-test.ts b/test/DecryptionFailureTracker-test.ts index ef5e01719f..06fe22ce54 100644 --- a/test/DecryptionFailureTracker-test.ts +++ b/test/DecryptionFailureTracker-test.ts @@ -14,14 +14,15 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { mocked, Mocked } from "jest-mock"; -import { CryptoEvent, HttpApiEvent, MatrixEvent, MatrixEventEvent } from "matrix-js-sdk/src/matrix"; +import { mocked, Mocked, MockedObject } from "jest-mock"; +import { CryptoEvent, HttpApiEvent, MatrixClient, MatrixEvent, MatrixEventEvent } from "matrix-js-sdk/src/matrix"; import { decryptExistingEvent, mkDecryptionFailureMatrixEvent } from "matrix-js-sdk/src/testing"; import { CryptoApi, DecryptionFailureCode, UserVerificationStatus } from "matrix-js-sdk/src/crypto-api"; import { sleep } from "matrix-js-sdk/src/utils"; import { DecryptionFailureTracker, ErrorProperties } from "../src/DecryptionFailureTracker"; import { stubClient } from "./test-utils"; +import * as Lifecycle from "../src/Lifecycle"; async function createFailedDecryptionEvent(opts: { sender?: string; code?: DecryptionFailureCode } = {}) { return await mkDecryptionFailureMatrixEvent({ @@ -39,6 +40,10 @@ function eventDecrypted(tracker: DecryptionFailureTracker, e: MatrixEvent, nowTs } describe("DecryptionFailureTracker", function () { + afterEach(() => { + localStorage.clear(); + }); + it("tracks a failed decryption for a visible event", async function () { const failedDecryptionEvent = await createFailedDecryptionEvent(); @@ -247,6 +252,7 @@ describe("DecryptionFailureTracker", function () { () => count++, () => "UnknownError", ); + await tracker.start(mockClient()); tracker.addVisibleEvent(decryptedEvent); @@ -264,10 +270,7 @@ describe("DecryptionFailureTracker", function () { expect(count).toBe(1); }); - it.skip("should not track a failure for an event that was tracked in a previous session", async () => { - // This test uses localStorage, clear it beforehand - localStorage.clear(); - + it("should not report a failure for an event that was reported in a previous session", async () => { const decryptedEvent = await createFailedDecryptionEvent(); let count = 0; @@ -276,6 +279,7 @@ describe("DecryptionFailureTracker", function () { () => count++, () => "UnknownError", ); + await tracker.start(mockClient()); tracker.addVisibleEvent(decryptedEvent); @@ -289,14 +293,13 @@ describe("DecryptionFailureTracker", function () { // Simulate the browser refreshing by destroying tracker and creating a new tracker // @ts-ignore access to private constructor const secondTracker = new DecryptionFailureTracker( - (total: number) => (count += total), + () => count++, () => "UnknownError", ); + await secondTracker.start(mockClient()); secondTracker.addVisibleEvent(decryptedEvent); - //secondTracker.loadTrackedEvents(); - eventDecrypted(secondTracker, decryptedEvent, Date.now()); secondTracker.checkFailures(Infinity); @@ -304,6 +307,70 @@ describe("DecryptionFailureTracker", function () { expect(count).toBe(1); }); + it("should report a failure for an event that was tracked but not reported in a previous session", async () => { + const decryptedEvent = await createFailedDecryptionEvent(); + + let count = 0; + + // @ts-ignore access to private constructor + const tracker = new DecryptionFailureTracker( + () => count++, + () => "UnknownError", + ); + await tracker.start(mockClient()); + + tracker.addVisibleEvent(decryptedEvent); + + // Indicate decryption + eventDecrypted(tracker, decryptedEvent, Date.now()); + + // we do *not* call `checkFailures` here + expect(count).toBe(0); + + // Simulate the browser refreshing by destroying tracker and creating a new tracker + // @ts-ignore access to private constructor + const secondTracker = new DecryptionFailureTracker( + () => count++, + () => "UnknownError", + ); + await secondTracker.start(mockClient()); + + secondTracker.addVisibleEvent(decryptedEvent); + + eventDecrypted(secondTracker, decryptedEvent, Date.now()); + secondTracker.checkFailures(Infinity); + expect(count).toBe(1); + }); + + it("should report a failure for an event that was reported before a logout/login cycle", async () => { + const decryptedEvent = await createFailedDecryptionEvent(); + + let count = 0; + + // @ts-ignore access to private constructor + const tracker = new DecryptionFailureTracker( + () => count++, + () => "UnknownError", + ); + await tracker.start(mockClient()); + + tracker.addVisibleEvent(decryptedEvent); + + // Indicate decryption + eventDecrypted(tracker, decryptedEvent, Date.now()); + tracker.checkFailures(Infinity); + expect(count).toBe(1); + + // Simulate a logout/login cycle + await Lifecycle.onLoggedOut(); + await tracker.start(mockClient()); + + tracker.addVisibleEvent(decryptedEvent); + eventDecrypted(tracker, decryptedEvent, Date.now()); + tracker.checkFailures(Infinity); + expect(count).toBe(2); + }); + it("should count different error codes separately for multiple failures with different error codes", async () => { const counts: Record = {}; @@ -521,12 +588,7 @@ describe("DecryptionFailureTracker", function () { it("listens for client events", async () => { // Test that the decryption failure tracker registers the right event // handlers on start, and unregisters them when the client logs out. - const client = mocked(stubClient()); - const mockCrypto = { - getVersion: jest.fn().mockReturnValue("Rust SDK 0.7.0 (61b175b), Vodozemac 0.5.1"), - getUserVerificationStatus: jest.fn().mockResolvedValue(new UserVerificationStatus(false, false, false)), - } as unknown as Mocked; - client.getCrypto.mockReturnValue(mockCrypto); + const client = mockClient(); let errorCount: number = 0; // @ts-ignore access to private constructor @@ -568,13 +630,7 @@ describe("DecryptionFailureTracker", function () { }); it("tracks client information", async () => { - const client = mocked(stubClient()); - const mockCrypto = { - getVersion: jest.fn().mockReturnValue("Rust SDK 0.7.0 (61b175b), Vodozemac 0.5.1"), - getUserVerificationStatus: jest.fn().mockResolvedValue(new UserVerificationStatus(false, false, false)), - } as unknown as Mocked; - client.getCrypto.mockReturnValue(mockCrypto); - + const client = mockClient(); const propertiesByErrorCode: Record = {}; // @ts-ignore access to private constructor const tracker = new DecryptionFailureTracker( @@ -610,7 +666,9 @@ describe("DecryptionFailureTracker", function () { const now = Date.now(); eventDecrypted(tracker, federatedDecryption, now); - mockCrypto.getUserVerificationStatus.mockResolvedValue(new UserVerificationStatus(true, true, false)); + mocked(client.getCrypto()!.getUserVerificationStatus).mockResolvedValue( + new UserVerificationStatus(true, true, false), + ); client.emit(CryptoEvent.KeysChanged, {}); await sleep(100); eventDecrypted(tracker, localDecryption, now); @@ -628,7 +686,7 @@ describe("DecryptionFailureTracker", function () { // change client params, and make sure the reports the right values client.getDomain.mockReturnValue("example.com"); - mockCrypto.getVersion.mockReturnValue("Olm 0.0.0"); + mocked(client.getCrypto()!.getVersion).mockReturnValue("Olm 0.0.0"); // @ts-ignore access to private method await tracker.calculateClientProperties(client); @@ -673,3 +731,21 @@ describe("DecryptionFailureTracker", function () { expect(failure?.timeToDecryptMillis).toEqual(50000); }); }); + +function mockClient(): MockedObject { + const client = mocked(stubClient()); + const mockCrypto = { + getVersion: jest.fn().mockReturnValue("Rust SDK 0.7.0 (61b175b), Vodozemac 0.5.1"), + getUserVerificationStatus: jest.fn().mockResolvedValue(new UserVerificationStatus(false, false, false)), + } as unknown as Mocked; + client.getCrypto.mockReturnValue(mockCrypto); + + // @ts-ignore + client.stopClient = jest.fn(() => {}); + // @ts-ignore + client.removeAllListeners = jest.fn(() => {}); + + client.store = { destroy: jest.fn(() => {}) } as any; + + return client; +} diff --git a/yarn.lock b/yarn.lock index 86f30bb6fb..695c5deabb 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2907,6 +2907,11 @@ resolved "https://registry.yarnpkg.com/@types/sdp-transform/-/sdp-transform-2.4.9.tgz#26ef39f487a6909b0512f580b80920a366b27f52" integrity sha512-bVr+/OoZZy7wrHlNcEAAa6PAgKA4BoXPYVN2EijMC5WnGgQ4ZEuixmKnVs2roiAvr7RhIFVH17QD27cojgIZCg== +"@types/seedrandom@<3.0.5": + version "3.0.4" + resolved "https://registry.yarnpkg.com/@types/seedrandom/-/seedrandom-3.0.4.tgz#e4a8d0fca0168cacc7dba2af0e4a4ea645d3a190" + integrity sha512-/rWdxeiuZenlawrHU+XV6ZHMTKOqrC2hMfeDfLTIWJhDZP5aVqXRysduYHBbhD7CeJO6FJr/D2uBVXB7GT6v7w== + "@types/semver@^7.5.8": version "7.5.8" resolved "https://registry.yarnpkg.com/@types/semver/-/semver-7.5.8.tgz#8268a8c57a3e4abd25c165ecd36237db7948a55e" @@ -3594,6 +3599,11 @@ base-x@^4.0.0: resolved "https://registry.yarnpkg.com/base-x/-/base-x-4.0.0.tgz#d0e3b7753450c73f8ad2389b5c018a4af7b2224a" integrity sha512-FuwxlW4H5kh37X/oW59pwTzzTKRzfrrQwhmyspRM7swOEZcHtDZSCt45U6oKgtuFE+WYPblePMVIPR4RZrh/hw== +base64-arraybuffer@^1.0.2: + version "1.0.2" + resolved "https://registry.yarnpkg.com/base64-arraybuffer/-/base64-arraybuffer-1.0.2.tgz#1c37589a7c4b0746e34bd1feb951da2df01c1bdc" + integrity sha512-I3yl4r9QB5ZRY3XuJVEPfc2XhZO6YweFPI+UovAzn+8/hb3oJ6lnysaFcjVpkCPfVWFUDvoZ8kmVDP7WyRtYtQ== + big-integer@^1.6.48: version "1.6.51" resolved "https://registry.yarnpkg.com/big-integer/-/big-integer-1.6.51.tgz#0df92a5d9880560d3ff2d5fd20245c889d130686" @@ -3614,6 +3624,21 @@ blob-polyfill@^7.0.0: resolved "https://registry.yarnpkg.com/blob-polyfill/-/blob-polyfill-7.0.20220408.tgz#38bf5e046c41a21bb13654d9d19f303233b8218c" integrity sha512-oD8Ydw+5lNoqq+en24iuPt1QixdPpe/nUF8azTHnviCZYu9zUC+TwdzIp5orpblJosNlgNbVmmAb//c6d6ImUQ== +bloom-filters@^3.0.1: + version "3.0.1" + resolved "https://registry.yarnpkg.com/bloom-filters/-/bloom-filters-3.0.1.tgz#13e28ed22febe2489cd00ba5bd98fdc90e820180" + integrity sha512-rU9IU6bgZ1jmqcLWhlKSidrFjbIGjB89CJBsQqUj1+3/11tAJDwn+f7iRu4bbQ2srTjGgNeoWNwcnelumqdi0g== + dependencies: + base64-arraybuffer "^1.0.2" + is-buffer "^2.0.5" + lodash "^4.17.15" + lodash.eq "^4.0.0" + lodash.indexof "^4.0.5" + long "^5.2.0" + reflect-metadata "^0.1.13" + seedrandom "^3.0.5" + xxhashjs "^0.2.2" + blurhash@^2.0.3: version "2.0.5" resolved "https://registry.yarnpkg.com/blurhash/-/blurhash-2.0.5.tgz#efde729fc14a2f03571a6aa91b49cba80d1abe4b" @@ -4155,6 +4180,11 @@ csstype@^3.0.2: resolved "https://registry.yarnpkg.com/csstype/-/csstype-3.1.3.tgz#d80ff294d114fb0e6ac500fbf85b60137d7eff81" integrity sha512-M1uQkMl8rQK/szD0LNhtqxIPLpimGm8sOBwU7lLnCpSbTyY3yeU1Vc7l4KT5zT4s/yOxHH5O7tIuuLOCnLADRw== +cuint@^0.2.2: + version "0.2.2" + resolved "https://registry.yarnpkg.com/cuint/-/cuint-0.2.2.tgz#408086d409550c2631155619e9fa7bcadc3b991b" + integrity sha512-d4ZVpCW31eWwCMe1YT3ur7mUDnTXbgwyzaL320DrcRT45rfjYxkt5QWLrmOJ+/UEAI2+fQgKe/fCjR8l4TpRgw== + damerau-levenshtein@^1.0.8: version "1.0.8" resolved "https://registry.yarnpkg.com/damerau-levenshtein/-/damerau-levenshtein-1.0.8.tgz#b43d286ccbd36bc5b2f7ed41caf2d0aba1f8a6e7" @@ -5976,6 +6006,11 @@ is-boolean-object@^1.1.0: call-bind "^1.0.2" has-tostringtag "^1.0.0" +is-buffer@^2.0.5: + version "2.0.5" + resolved "https://registry.yarnpkg.com/is-buffer/-/is-buffer-2.0.5.tgz#ebc252e400d22ff8d77fa09888821a24a658c191" + integrity sha512-i2R6zNFDwgEHJyQUtJEk0XFi1i0dPFn/oqjK3/vPCcDeJvW5NQ83V8QbicfF1SupOaB0h8ntgBC2YiE7dfyctQ== + is-buffer@~1.1.6: version "1.1.6" resolved "https://registry.yarnpkg.com/is-buffer/-/is-buffer-1.1.6.tgz#efaa2ea9daa0d7ab2ea13a97b2b8ad51fefbe8be" @@ -6961,6 +6996,16 @@ lodash.debounce@^4.0.8: resolved "https://registry.yarnpkg.com/lodash.debounce/-/lodash.debounce-4.0.8.tgz#82d79bff30a67c4005ffd5e2515300ad9ca4d7af" integrity sha512-FT1yDzDYEoYWhnSGnpE/4Kj1fLZkDFyqRb7fNt6FdYOSxlUWAtp42Eh6Wb0rGIv/m9Bgo7x4GhQbm5Ys4SG5ow== +lodash.eq@^4.0.0: + version "4.0.0" + resolved "https://registry.yarnpkg.com/lodash.eq/-/lodash.eq-4.0.0.tgz#a39f06779e72f9c0d1f310c90cd292c1661d5035" + integrity sha512-vbrJpXL6kQNG6TkInxX12DZRfuYVllSxhwYqjYB78g2zF3UI15nFO/0AgmZnZRnaQ38sZtjCiVjGr2rnKt4v0g== + +lodash.indexof@^4.0.5: + version "4.0.5" + resolved "https://registry.yarnpkg.com/lodash.indexof/-/lodash.indexof-4.0.5.tgz#53714adc2cddd6ed87638f893aa9b6c24e31ef3c" + integrity sha512-t9wLWMQsawdVmf6/IcAgVGqAJkNzYVcn4BHYZKTPW//l7N5Oq7Bq138BaVk19agcsPZePcidSgTTw4NqS1nUAw== + lodash.isequal@^4.5.0: version "4.5.0" resolved "https://registry.yarnpkg.com/lodash.isequal/-/lodash.isequal-4.5.0.tgz#415c4478f2bcc30120c22ce10ed3226f7d3e18e0" @@ -6981,7 +7026,7 @@ lodash.truncate@^4.4.2: resolved "https://registry.yarnpkg.com/lodash.truncate/-/lodash.truncate-4.4.2.tgz#5a350da0b1113b837ecfffd5812cbe58d6eae193" integrity sha512-jttmRe7bRse52OsWIMDLaXxWqRAmtIUccAQ3garviCqJjafXOfNMO0yMfNpdD6zbGaTU0P5Nz7e7gAT6cKmJRw== -lodash@^4.17.20, lodash@^4.17.21: +lodash@^4.17.15, lodash@^4.17.20, lodash@^4.17.21: version "4.17.21" resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.21.tgz#679591c564c3bffaae8454cf0b3df370c3d6911c" integrity sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg== @@ -6991,6 +7036,11 @@ loglevel@^1.7.1: resolved "https://registry.yarnpkg.com/loglevel/-/loglevel-1.8.1.tgz#5c621f83d5b48c54ae93b6156353f555963377b4" integrity sha512-tCRIJM51SHjAayKwC+QAg8hT8vg6z7GSgLJKGvzuPb1Wc+hLzqtuVLxp6/HzSPOozuK+8ErAhy7U/sVzw8Dgfg== +long@^5.2.0: + version "5.2.3" + resolved "https://registry.yarnpkg.com/long/-/long-5.2.3.tgz#a3ba97f3877cf1d778eccbcb048525ebb77499e1" + integrity sha512-lcHwpNoggQTObv5apGNCTdJrO69eHOZMi4BNC+rTLER8iHAqGrUVeLh/irVIM7zTw2bOXA8T6uNPeujwOLg/2Q== + loose-envify@^1.0.0, loose-envify@^1.1.0, loose-envify@^1.4.0: version "1.4.0" resolved "https://registry.yarnpkg.com/loose-envify/-/loose-envify-1.4.0.tgz#71ee51fa7be4caec1a63839f7e682d8132d30caf" @@ -8172,6 +8222,11 @@ redux@^4.0.0, redux@^4.0.4: dependencies: "@babel/runtime" "^7.9.2" +reflect-metadata@^0.1.13: + version "0.1.14" + resolved "https://registry.yarnpkg.com/reflect-metadata/-/reflect-metadata-0.1.14.tgz#24cf721fe60677146bb77eeb0e1f9dece3d65859" + integrity sha512-ZhYeb6nRaXCfhnndflDK8qI6ZQ/YcWZCISRAWICW9XYqMUwjZM9Z0DveWX/ABN01oxSHwVxKQmxeYZSsm0jh5A== + reflect.getprototypeof@^1.0.4: version "1.0.6" resolved "https://registry.yarnpkg.com/reflect.getprototypeof/-/reflect.getprototypeof-1.0.6.tgz#3ab04c32a8390b770712b7a8633972702d278859" @@ -8472,6 +8527,11 @@ sdp-transform@^2.14.1: resolved "https://registry.yarnpkg.com/sdp-transform/-/sdp-transform-2.14.1.tgz#2bb443583d478dee217df4caa284c46b870d5827" integrity sha512-RjZyX3nVwJyCuTo5tGPx+PZWkDMCg7oOLpSlhjDdZfwUoNqG1mM8nyj31IGHyaPWXhjbP7cdK3qZ2bmkJ1GzRw== +seedrandom@^3.0.5: + version "3.0.5" + resolved "https://registry.yarnpkg.com/seedrandom/-/seedrandom-3.0.5.tgz#54edc85c95222525b0c7a6f6b3543d8e0b3aa0a7" + integrity sha512-8OwmbklUNzwezjGInmZ+2clQmExQPvomqjL7LFqOYqtmuxRgQYqOD3mHaU+MvZn5FLUeVxVfQjwLZW/n/JFuqg== + "semver@2 || 3 || 4 || 5", semver@^5.6.0: version "5.7.2" resolved "https://registry.yarnpkg.com/semver/-/semver-5.7.2.tgz#48d55db737c3287cd4835e17fa13feace1c41ef8" @@ -8729,16 +8789,7 @@ string-length@^4.0.1: char-regex "^1.0.2" strip-ansi "^6.0.0" -"string-width-cjs@npm:string-width@^4.2.0": - version "4.2.3" - resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" - integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== - dependencies: - emoji-regex "^8.0.0" - is-fullwidth-code-point "^3.0.0" - strip-ansi "^6.0.1" - -string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3: +"string-width-cjs@npm:string-width@^4.2.0", string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3: version "4.2.3" resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== @@ -8832,14 +8883,7 @@ string_decoder@~1.1.1: dependencies: safe-buffer "~5.1.0" -"strip-ansi-cjs@npm:strip-ansi@^6.0.1": - version "6.0.1" - resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" - integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== - dependencies: - ansi-regex "^5.0.1" - -strip-ansi@^6.0.0, strip-ansi@^6.0.1: +"strip-ansi-cjs@npm:strip-ansi@^6.0.1", strip-ansi@^6.0.0, strip-ansi@^6.0.1: version "6.0.1" resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== @@ -9632,7 +9676,7 @@ which@^2.0.1: dependencies: isexe "^2.0.0" -"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0": +"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0", wrap-ansi@^7.0.0: version "7.0.0" resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== @@ -9650,15 +9694,6 @@ wrap-ansi@^6.2.0: string-width "^4.1.0" strip-ansi "^6.0.0" -wrap-ansi@^7.0.0: - version "7.0.0" - resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" - integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== - dependencies: - ansi-styles "^4.0.0" - string-width "^4.1.0" - strip-ansi "^6.0.0" - wrap-ansi@^8.1.0: version "8.1.0" resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-8.1.0.tgz#56dc22368ee570face1b49819975d9b9a5ead214" @@ -9709,6 +9744,13 @@ xmlchars@^2.2.0: resolved "https://registry.yarnpkg.com/xmlchars/-/xmlchars-2.2.0.tgz#060fe1bcb7f9c76fe2a17db86a9bc3ab894210cb" integrity sha512-JZnDKK8B0RCDw84FNdDAIpZK+JuJw+s7Lz8nksI7SIuU3UXJJslUthsi+uWBUYOwPFwW7W7PRLRfUKpxjtjFCw== +xxhashjs@^0.2.2: + version "0.2.2" + resolved "https://registry.yarnpkg.com/xxhashjs/-/xxhashjs-0.2.2.tgz#8a6251567621a1c46a5ae204da0249c7f8caa9d8" + integrity sha512-AkTuIuVTET12tpsVIQo+ZU6f/qDmKuRUcjaqR+OIvm+aCBsZ95i7UVY5WJ9TMsSaZ0DA2WxoZ4acu0sPH+OKAw== + dependencies: + cuint "^0.2.2" + y18n@^4.0.0: version "4.0.3" resolved "https://registry.yarnpkg.com/y18n/-/y18n-4.0.3.tgz#b5f259c82cd6e336921efd7bfd8bf560de9eeedf" From f6e919021ad91f4f24d2456f6c6278921158034b Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Mon, 20 May 2024 18:08:50 +0200 Subject: [PATCH 23/42] Fix E2E icon display in room header (#12545) * Fix E2E icon display * Add e2e test --- playwright/e2e/room/room-header.spec.ts | 22 ++++++++++++++++++ .../encrypted-room-header-linux.png | Bin 0 -> 5505 bytes res/css/views/rooms/_LegacyRoomHeader.pcss | 5 ++++ 3 files changed, 27 insertions(+) create mode 100644 playwright/snapshots/room/room-header.spec.ts/encrypted-room-header-linux.png diff --git a/playwright/e2e/room/room-header.spec.ts b/playwright/e2e/room/room-header.spec.ts index a3c5e8c8bc..4008517d09 100644 --- a/playwright/e2e/room/room-header.spec.ts +++ b/playwright/e2e/room/room-header.spec.ts @@ -276,4 +276,26 @@ test.describe("Room Header", () => { await expect(header).toMatchScreenshot("room-header-with-apps-button-not-highlighted.png"); }); }); + + test.describe("with encryption", () => { + test("should render the E2E icon and the buttons", async ({ page, app, user }) => { + // Create an encrypted room + await app.client.createRoom({ + name: "Test Encrypted Room", + initial_state: [ + { + type: "m.room.encryption", + state_key: "", + content: { + algorithm: "m.megolm.v1.aes-sha2", + }, + }, + ], + }); + await app.viewRoomByName("Test Encrypted Room"); + + const header = page.locator(".mx_LegacyRoomHeader"); + await expect(header).toMatchScreenshot("encrypted-room-header.png"); + }); + }); }); diff --git a/playwright/snapshots/room/room-header.spec.ts/encrypted-room-header-linux.png b/playwright/snapshots/room/room-header.spec.ts/encrypted-room-header-linux.png new file mode 100644 index 0000000000000000000000000000000000000000..6dced2e99093b4809375d86f6d95443fe6533b92 GIT binary patch literal 5505 zcmXX~1zc0#7oVacA~6t<4y8m!IXVOfBcudGT4Hq92w@g793kD|=oWAw+dxG^xSZ)9S04fc2$P)m7%z{KKU%yPc=Fgk> zk#3iKo;#Ll6-TjPfoy@RE*CS#Dt3DbrRW^ zEvOND?PADeNM&=Fqxi+}#X86sC6MJ}1v|Q$*RNd2K0rcut zn9%LC7woUf!Q2-R?QhJ*bT4%#dZ214|BbH9;bgvGsrzQKd!N_=Tv_p2Ey2Z z%r}R?GgK^6Ev=xRng+TSul6q{erY-L8G``=>4DW=A3a|RKtCOzvE25Zi9+F(b#=Z- z?HqK)Y6C&cF{*HqcFpn@$Az9-YJa_~El(Ow5sG8xQ=_(;RO}euX^B-nvvx~Y68{5K z73Sx)M23pBe>BBnvp1LOxcV||>1!ZEu-yU1FvCPA(+8V}OxJ6wiD+1+abju_*_E|+~9T^pJvvjf=^EwY*uV!zKFL zy1O-q>a*eO$E@jT^8$%U1+7OzAFGMWgIIM9>+;yE#+o!CTbzem92QK0{KXLc3$uG! z@m{lokn;hVW@gHBHs)VMn_Ovkx6pp~_APOs^T1G7r?GfYU|U{77YCW3y}g^t6X(`l zd6P;JD?I_L*2W4{g5ldjZd7L6{dxKM=7!W0>i*u}>-GWnlWc*=ciAT zvdb(P0ZkQX-Z+xxf+TGhlEwqm%SBy+Uz z7u;s0Vzx&gV2jja3mimBSiNmG{1viT2LnH(n#UokigLA2ZqAPra2TWAqm3n`g4yuE zi{gX5mYFdM)w+!#L%P@@uG|R5+DnYNC-t?m&J&=(8tYSsuGXY-xzW*4!o=jPNtC;@ z^D=1Ez{tn~XOBUz=Rb?nnq+k?7rMuht+1axkFfMRApmuC;bBvxKXi`EpRf7NR@-}Q z<4(K;wg7QZv~^Iw$XA>`Vrn+VGaE;k09e7ND~x<3416R8=YEyVB_s=bUVimQTM$>m) z8}tnLgXjUvGn`g8bwmkno~fVcz3Hnt)?N5Y2c0S}#+w<`OPp?H^y~05m94I)xhSsP z?D1b6pZNW|b$t)az>uhjWy<*q8(24c zLI_ktY&U0WZ-(Bkj+Xo>3oqSQZI56aocXD@OjqMvS}d0m$%VGmvFqu^9ZsY5#`^c< zOZz1pdbKpqq7;lwj4<|Vvd4dWh~_R*G})YzZr0(j6zT;@Fj$P04qJi~XYbkFOrr$? z=ZVv{P73yaBTbG_Q~#b7aPC zTGt-TUMlT}xVU>T?THDCh($|l?%=x;+oSkH&SxW$m|9Hm`I{K;ZTB06$(~7pjsgKFS=|}+l$;7B*QYRT?IUj-z(M^pF`ULv-wQwTpLB~wIFan4+TRBNz)RfR*;TtK z0aO(C>1dX+*4zpsbc$aMgBU(DZ*>pGMmk`h##h)&K6pUte3tF%*fb4@y9Ii(DXtpC zWN_QWeCn;m7bQi!pgESPCwOR{EOUA|FP26kMtRYXlLG>X&}7tpt8024aN_V zihJ>QDWm~!tK&Y|WwO1qyG!gzEzirl1qAL+qxgsC7Tr=wEC)n&a3!bvSc}LTSCFX z0Sa+^q{V~vx`V?WMWmOaA~Y6^Qx`Cp(x;VF6x1Z^<=4$LseLh2l^c$4P-saq-+F{Z z={~~*MZJ61`ufo4>gDh6jj}ZlU4?{7k?LBdHN7*Vm=zzQu%UrLx)j`>JR337T%1}% zR6AKHD=*iMaYzlXg{?#v8>qy^A+)r5Qxt8ZU%fgjGZ_@I(!pk=>WRK6l<);~Nn(V?C}kYZSsfWjVqVn61! zD=SJU1jPyRfm}VDHU?3mYhgMCM2O+kz>Dc0V6C2$lP?vu$?);pwZ#~WUt4W=D_<+* z!S`W@@5iWG4;lv*bQ9W!I(&P(Q6VS@D=b{D-!CSQ>bX`6&aOxr7n>L#U!7m;T@S6@ z8}!Srw60}kW!*o=JG@ODiQ`3OWGFm&GMF-(9y&hI-(My4LQcZ?O(#1uJA3f9z2YFv z*yw1`&gLahosV;Lda0IaP-{bQSW*(6@Z}0mnnG~F=0u&xv6r_V7ch#`;r#GO#DC*7 zB7zck@u>*Rjp0Z_sf=`_)$z#DdKNz`m9deL$IUSaB>u82rwXA!m=s4$olhy*_ogXB z}fRO<`(lHSuBxGv~y3NA!4s_^<&t1W|NMRY( z%(Q$ob3E+K2hMC4RG50^whzf;vOOFfGw2pkIkbQyVXbv!za;)dE;CaGSQWw9m03Wj z`*S9imS#p++uQRenQQbH<);?MN;or?O|Y{wqf&`CZ1MH?cVs0cE_-aUf|AN>lr6)m zVqPCrii+T7x3!JHJWq%Myl*Q!RvgdoO|K4B_w;^FDoGoM<3knFkwoDGy@NjuO+5_v zlSA=#E4A$F9Ev?4ojrg5v(Uy|wGqugn4ce}+$_`%E*!|XT1ztJ>k<+?EY5a$#lRhbR$gtYv`s7}uLN&?a+NF0uUjVUmJn*5+V0h<$YkK#_UnNexrmeuVH z9t$c|f}Ggd@vz7|7X{`k-KSRkEQ6E^GFIaWe(yjSliakBtD``)>*?#;OG=iV-@hxp zIFsLea&poTyzh2%?C$s~e`;LZR>FsN8(qJB%;T+LaBL0tcV(-PQ|%WoRP>%d)X-SU zF3=w851e1B$_~Cnk!O;D$AxowgBu?X{G^a%IUD}kw4R(OOggb@YY*n1z)D;_MD_n1 zE^4t+I|O|?zip0_sVu5MR|KfYC}L!DD27se%-rWO&3~7T#~St z6BcqvY1cz|7=yt>kt=~6nwlVL+lkCJv4(auvmZlkblr%_N_?=9jpKBj!}Bsk_ywuPzT-S5osQfE!^L&0@*7WIgYj?>^(FJU8$9=S;UhNFi^ za$HQiGSnGj+bZwwS7V);u|O;t!p2yxVY&z|~yZv1h-RdkgXH$CoiU0rFB z0FilgEJ|BLTkA_es?*_El@Hv|w=3Do_4yG(b#ZxFC+!x=2m2lT6oEVl-n%(nbNLmQ z;@M1s;CIL^dU`0NR#I}+BsNYf;5x+4);45T&EM7az8>_ZERSyXe6pu zm^$nd5Owsy;Y&2*vaspwUQRlhv%$!iNZ;N0#@Tm%E)wG6$sh|cm)|!-LQW}xvGemW zji*ExV#Bc94|sB-g8JjfGv4*ZPD_xvnbl?@=F=bI{6vP-IT=5H_}9=X<++ZvwKcQ4 zHCn}i*=le$qLEMGj!*ZfUvaTh7oPsnk5_wp6Ch5}U}7W^Swu!oL&G8eb8ID%pieSX zl*m_2ZLI~8Kh(+MLnjJZ`D>sO%$pu@p^b!+MJ#H!w*3$qoj-4F9`=iQ4V-XJOMRE( z`liG^JusOadSX+Pe;cc3T^vK&p$LYq-b_b5pTIQGC3Rdz~EOt zDIum}B>s3qhR>aePn{rbxUU*u@S**5!|n8`zBIvZckX{R8B zo8nyjOLO#8E|Wq~7nwtgWZn%FR=}PYuYbn)ByV|%Co~RJJ>I_Xrz~cKw~Pwot0Vjj z)7OKMf|JrVX`f|dCZ+}C&!SUu>ZO$xkE7|S&Lj{w4-7U>|CJ+CEW}jJ{&&yj1Oo#@ zXHwYN7pFVv@a=*qj&*mbZ4Y*tpt6;NT2lMODM@+0TqO$X(P={{*my|o{aUI4#`cGX zHah6A4-XEmff{0BhQGZci*P*Jaq8%(^gS*qE%g}NvJFxr77scE$fL-VHg-MXD;0Be z9G8Jtuj<^Gs`C3XvT4At>##7lK87pBRagTN=X)N@%g$r)x5iJ6fUz|-v&Hmnt81-U z(t$@isXic3k3(;UU7L%hA|(J2^8LHJ;fM21#X9r=`Pd$Zg1>BBRIhWEqMYx^3uWu< zuSLJVhyI#590bEsZ#j~(ZL5vMONQ`u2F{b+ak%w#dJ_$Y9D8ayV*guJO>fpP3$^Gc zq91ji6x;LXu)nP>VHPV*HfNw0@)=m_=#5;gr>U7)s=w{@lxk~$hQy@H*tnHDf5-U^ zDjZj7B9yqBQuYxuKbm{2$rfF+r0#ds?@wd?mCN6+oPR^3XATJJ{t8SK8^xrYeevG8 z&UAediq78FQ#zw>-y!wpT79Rt!GUtBWgxn|9OFI&U9BkF_%0+Uq!hIEdhUVjsOjvQ zYfbewDV&vI>mjWP=6rSnx^I5e-1GnpmMo)AR1AUpF~ho9AM`zZ_}WFLjwNiAa=r+a zaEsBj`*i9X;%ME;gW)_4$8qChf&BT@jDr)h@2SDQSlf#yqcw0VXVV~k{i<+Wx~Co~ z=i`d+my{P4?lxGkZP5vNp2p3QV60Bh6wPYu13ty>)Nx1=6SJ*IF<&QBZ=&L{y2Kzw zR!@Yn1aDrkhojw;9NXf;A^A?kd7bMIyT>Appov|PeG7|Qdurf2lR075+*|V@h{1$Q ztwHbawae*p(XvSgNQ1!>9EvV=ZXbv>;4>ePDm$&w)i<`bKY(XhpD=~( z-jU#u>CctH>8E0_B6~yNB3~QBRQ;rD{!Xp;d5#j z1u4xVWw*_ZHYvz!)P313(jQlFAM?4#V_aQYGFLb9g;jrzv4$8k2?|Ps?xM$2aO6)j zT`HKS*n}THf(rZgc2&iH%3}inzNMovnDV?bJaNB}o&AKgvp-&4tHePRTs$@e1nvvo z7c33h3U+g2_i}vMm{ryPA@Ws8WwGK)A3?51eo!&o0(OUXIDO{&{WLX_AQRUtKn+APnx&d~uL$%+w_%wfea zxWRg|CBo3>d}Ouex|uz&F-BnpqVR0SG|txQT!-D7-K& zDRCw>ZqP;f@1+is@}ds+@*nz;zd7uJ`S2f_LqGGMIM+Y4kHYI;-R~c|`1Z?1IS_FD zAL^@Oeo+Dh{2!J5rvUJOH1q#zfw3KL+P>0zA(1}<4{@9Ceo9`*;Nzvs%gdX9-2ct| c@@(n+k`;Y7tp)b6%mrIRRR>a`^8EGx0LhJ+Gynhq literal 0 HcmV?d00001 diff --git a/res/css/views/rooms/_LegacyRoomHeader.pcss b/res/css/views/rooms/_LegacyRoomHeader.pcss index ce088f7deb..a570b0435a 100644 --- a/res/css/views/rooms/_LegacyRoomHeader.pcss +++ b/res/css/views/rooms/_LegacyRoomHeader.pcss @@ -65,6 +65,11 @@ limitations under the License. .mx_BetaCard_betaPill { margin-right: $spacing-8; } + + /* The container of E2EIcon in the legacy header needs to have its height set */ + & > span { + height: 100%; + } } .mx_LegacyRoomHeader_name { From 3342aa5ff8ac27de52df96c0f1d21494e6a40976 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 21 May 2024 11:37:02 +0100 Subject: [PATCH 24/42] Refactor some logic into common AvatarSetting component (#12544) * Refactor some logic into common AvatarSetting component We duplicated some of the logic of setting avatars between profiles & rooms. This pulls some of that logic into the AvatarSetting component and hopefully make things a little simpler. * Unsed import * Convert JS based hover to CSS * Remove unnecessary container * Test avatar-as-file path * Test file upload * Unused imports * Add test for RoomProfileSettings * Test removing room avatar * Move upload control CSS too * Remove commented code Co-authored-by: Florian Duros * Prettier * Coments & move style to inline as per PR suggestion * Better test names Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> * Fix test Upload input doesn't have that class anymore --------- Co-authored-by: Florian Duros Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- .../general-user-settings-tab.spec.ts | 4 +- res/css/views/settings/_AvatarSetting.pcss | 7 +- res/css/views/settings/_ProfileSettings.pcss | 4 - .../room_settings/RoomProfileSettings.tsx | 82 ++++------- .../views/settings/AvatarSetting.tsx | 127 +++++++++++++----- .../views/settings/ProfileSettings.tsx | 78 ++++------- .../RoomProfileSettings-test.tsx | 105 +++++++++++++++ .../views/settings/AvatarSetting-test.tsx | 53 ++++++-- 8 files changed, 296 insertions(+), 164 deletions(-) create mode 100644 test/components/views/room_settings/RoomProfileSettings-test.tsx diff --git a/playwright/e2e/settings/general-user-settings-tab.spec.ts b/playwright/e2e/settings/general-user-settings-tab.spec.ts index 625f1d6bd5..ae3718aba9 100644 --- a/playwright/e2e/settings/general-user-settings-tab.spec.ts +++ b/playwright/e2e/settings/general-user-settings-tab.spec.ts @@ -133,9 +133,7 @@ test.describe("General user settings tab", () => { test("should support adding and removing a profile picture", async ({ uut }) => { const profileSettings = uut.locator(".mx_ProfileSettings"); // Upload a picture - await profileSettings - .locator(".mx_ProfileSettings_avatarUpload") - .setInputFiles("playwright/sample-files/riot.png"); + await profileSettings.getByAltText("Upload").setInputFiles("playwright/sample-files/riot.png"); // Find and click "Remove" link button await profileSettings.locator(".mx_ProfileSettings_profile").getByRole("button", { name: "Remove" }).click(); diff --git a/res/css/views/settings/_AvatarSetting.pcss b/res/css/views/settings/_AvatarSetting.pcss index 98bf3ab9b8..a6d70a697a 100644 --- a/res/css/views/settings/_AvatarSetting.pcss +++ b/res/css/views/settings/_AvatarSetting.pcss @@ -23,6 +23,7 @@ limitations under the License. .mx_AvatarSetting_hover { transition: opacity var(--hover-transition); + opacity: 0; /* position to place the hover bg over the entire thing */ position: absolute; @@ -50,14 +51,10 @@ limitations under the License. } } - &.mx_AvatarSetting_avatar_hovering .mx_AvatarSetting_hover { + &.mx_AvatarSetting_avatarDisplay:hover .mx_AvatarSetting_hover { opacity: 1; } - &:not(.mx_AvatarSetting_avatar_hovering) .mx_AvatarSetting_hover { - opacity: 0; - } - & > * { box-sizing: border-box; } diff --git a/res/css/views/settings/_ProfileSettings.pcss b/res/css/views/settings/_ProfileSettings.pcss index 5caff1f2c0..73cdcd75c8 100644 --- a/res/css/views/settings/_ProfileSettings.pcss +++ b/res/css/views/settings/_ProfileSettings.pcss @@ -17,10 +17,6 @@ limitations under the License. .mx_ProfileSettings { border-bottom: 1px solid $quinary-content; - .mx_ProfileSettings_avatarUpload { - display: none; - } - .mx_ProfileSettings_profile { display: flex; diff --git a/src/components/views/room_settings/RoomProfileSettings.tsx b/src/components/views/room_settings/RoomProfileSettings.tsx index 15d03a4c5d..9e7183b33a 100644 --- a/src/components/views/room_settings/RoomProfileSettings.tsx +++ b/src/components/views/room_settings/RoomProfileSettings.tsx @@ -1,5 +1,5 @@ /* -Copyright 2019 New Vector Ltd +Copyright 2019, 2024 New Vector Ltd Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -21,11 +21,9 @@ import { EventType } from "matrix-js-sdk/src/matrix"; import { _t } from "../../../languageHandler"; import { MatrixClientPeg } from "../../../MatrixClientPeg"; import Field from "../elements/Field"; -import { mediaFromMxc } from "../../../customisations/Media"; import AccessibleButton, { ButtonEvent } from "../elements/AccessibleButton"; import AvatarSetting from "../settings/AvatarSetting"; import { htmlSerializeFromMdIfNeeded } from "../../../editor/serialize"; -import { chromeFileInputFix } from "../../../utils/BrowserWorkarounds"; interface IProps { roomId: string; @@ -35,8 +33,9 @@ interface IState { originalDisplayName: string; displayName: string; originalAvatarUrl: string | null; - avatarUrl: string | null; avatarFile: File | null; + // If true, the user has indicated that they wish to remove the avatar and this should happen on save. + avatarRemovalPending: boolean; originalTopic: string; topic: string; profileFieldsTouched: Record; @@ -57,8 +56,7 @@ export default class RoomProfileSettings extends React.Component if (!room) throw new Error(`Expected a room for ID: ${props.roomId}`); const avatarEvent = room.currentState.getStateEvents(EventType.RoomAvatar, ""); - let avatarUrl = avatarEvent?.getContent()["url"] ?? null; - if (avatarUrl) avatarUrl = mediaFromMxc(avatarUrl).getSquareThumbnailHttp(96); + const avatarUrl = avatarEvent?.getContent()["url"] ?? null; const topicEvent = room.currentState.getStateEvents(EventType.RoomTopic, ""); const topic = topicEvent && topicEvent.getContent() ? topicEvent.getContent()["topic"] : ""; @@ -71,8 +69,8 @@ export default class RoomProfileSettings extends React.Component originalDisplayName: name, displayName: name, originalAvatarUrl: avatarUrl, - avatarUrl: avatarUrl, avatarFile: null, + avatarRemovalPending: false, originalTopic: topic, topic: topic, profileFieldsTouched: {}, @@ -82,16 +80,23 @@ export default class RoomProfileSettings extends React.Component }; } - private uploadAvatar = (): void => { - this.avatarUpload.current?.click(); + private onAvatarChanged = (file: File): void => { + this.setState({ + avatarFile: file, + avatarRemovalPending: false, + profileFieldsTouched: { + ...this.state.profileFieldsTouched, + avatar: true, + }, + }); }; private removeAvatar = (): void => { // clear file upload field so same file can be selected if (this.avatarUpload.current) this.avatarUpload.current.value = ""; this.setState({ - avatarUrl: null, avatarFile: null, + avatarRemovalPending: true, profileFieldsTouched: { ...this.state.profileFieldsTouched, avatar: true, @@ -112,8 +117,8 @@ export default class RoomProfileSettings extends React.Component profileFieldsTouched: {}, displayName: this.state.originalDisplayName, topic: this.state.originalTopic, - avatarUrl: this.state.originalAvatarUrl, avatarFile: null, + avatarRemovalPending: false, }); }; @@ -138,11 +143,12 @@ export default class RoomProfileSettings extends React.Component if (this.state.avatarFile) { const { content_uri: uri } = await client.uploadContent(this.state.avatarFile); await client.sendStateEvent(this.props.roomId, EventType.RoomAvatar, { url: uri }, ""); - newState.avatarUrl = mediaFromMxc(uri).getSquareThumbnailHttp(96); - newState.originalAvatarUrl = newState.avatarUrl; + newState.originalAvatarUrl = uri; newState.avatarFile = null; - } else if (this.state.originalAvatarUrl !== this.state.avatarUrl) { + } else if (this.state.avatarRemovalPending) { await client.sendStateEvent(this.props.roomId, EventType.RoomAvatar, {}, ""); + newState.avatarRemovalPending = false; + newState.originalAvatarUrl = null; } if (this.state.originalTopic !== this.state.topic) { @@ -192,34 +198,6 @@ export default class RoomProfileSettings extends React.Component } }; - private onAvatarChanged = (e: React.ChangeEvent): void => { - if (!e.target.files || !e.target.files.length) { - this.setState({ - avatarUrl: this.state.originalAvatarUrl, - avatarFile: null, - profileFieldsTouched: { - ...this.state.profileFieldsTouched, - avatar: false, - }, - }); - return; - } - - const file = e.target.files[0]; - const reader = new FileReader(); - reader.onload = (ev) => { - this.setState({ - avatarUrl: String(ev.target?.result), - avatarFile: file, - profileFieldsTouched: { - ...this.state.profileFieldsTouched, - avatar: true, - }, - }); - }; - reader.readAsDataURL(file); - }; - public render(): React.ReactNode { let profileSettingsButtons; if (this.state.canSetName || this.state.canSetTopic || this.state.canSetAvatar) { @@ -241,14 +219,6 @@ export default class RoomProfileSettings extends React.Component return (
-
/>
{profileSettingsButtons} diff --git a/src/components/views/settings/AvatarSetting.tsx b/src/components/views/settings/AvatarSetting.tsx index 4aaadc2acf..875c1f9c93 100644 --- a/src/components/views/settings/AvatarSetting.tsx +++ b/src/components/views/settings/AvatarSetting.tsx @@ -1,5 +1,5 @@ /* -Copyright 2019 The Matrix.org Foundation C.I.C. +Copyright 2019, 2024 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. @@ -14,51 +14,102 @@ See the License for the specific language governing permissions and limitations under the License. */ -import React, { useRef, useState } from "react"; -import classNames from "classnames"; +import React, { createRef, useCallback, useEffect, useRef, useState } from "react"; import { _t } from "../../../languageHandler"; -import AccessibleButton, { ButtonEvent } from "../elements/AccessibleButton"; +import AccessibleButton from "../elements/AccessibleButton"; +import { mediaFromMxc } from "../../../customisations/Media"; +import { chromeFileInputFix } from "../../../utils/BrowserWorkarounds"; interface IProps { - avatarUrl?: string; - avatarName: string; // name of user/room the avatar belongs to - uploadAvatar?: (e: ButtonEvent) => void; - removeAvatar?: (e: ButtonEvent) => void; + /** + * The current value of the avatar URL, as an mxc URL or a File. + * Generally, an mxc URL would be specified until the user selects a file, then + * the file supplied by the onChange callback would be supplied here until it's + * saved. + */ + avatar?: string | File; + + /** + * If true, the user cannot change the avatar + */ + disabled?: boolean; + + /** + * Called when the user has selected a new avatar + * The callback is passed a File object for the new avatar data + */ + onChange?: (f: File) => void; + + /** + * Called when the user wishes to remove the avatar + */ + removeAvatar?: () => void; + + /** + * The alt text for the avatar + */ avatarAltText: string; } -const AvatarSetting: React.FC = ({ avatarUrl, avatarAltText, avatarName, uploadAvatar, removeAvatar }) => { - const [isHovering, setIsHovering] = useState(false); - const hoveringProps = { - onMouseEnter: () => setIsHovering(true), - onMouseLeave: () => setIsHovering(false), - }; +/** + * Component for setting or removing an avatar on something (eg. a user or a room) + */ +const AvatarSetting: React.FC = ({ avatar, avatarAltText, onChange, removeAvatar, disabled }) => { + const fileInputRef = createRef(); + + // Real URL that we can supply to the img element, either a data URL or whatever mediaFromMxc gives + // This represents whatever avatar the user has chosen at the time + const [avatarURL, setAvatarURL] = useState(undefined); + useEffect(() => { + if (avatar instanceof File) { + const reader = new FileReader(); + reader.onload = () => { + setAvatarURL(reader.result as string); + }; + reader.readAsDataURL(avatar); + } else if (avatar) { + setAvatarURL(mediaFromMxc(avatar).getSquareThumbnailHttp(96) ?? undefined); + } else { + setAvatarURL(undefined); + } + }, [avatar]); + // TODO: Use useId() as soon as we're using React 18. // Prevents ID collisions when this component is used more than once on the same page. const a11yId = useRef(`hover-text-${Math.random()}`); + const onFileChanged = useCallback( + (e: React.ChangeEvent) => { + if (e.target.files) onChange?.(e.target.files[0]); + }, + [onChange], + ); + + const uploadAvatar = useCallback((): void => { + fileInputRef.current?.click(); + }, [fileInputRef]); + let avatarElement = ( ); - if (avatarUrl) { + if (avatarURL) { avatarElement = ( ); } @@ -67,17 +118,27 @@ const AvatarSetting: React.FC = ({ avatarUrl, avatarAltText, avatarName, if (uploadAvatar) { // insert an empty div to be the host for a css mask containing the upload.svg uploadAvatarBtn = ( - + <> + + + ); } let removeAvatarBtn: JSX.Element | undefined; - if (avatarUrl && removeAvatar) { + if (avatarURL && removeAvatar && !disabled) { removeAvatarBtn = ( {_t("action|remove")} @@ -85,16 +146,12 @@ const AvatarSetting: React.FC = ({ avatarUrl, avatarAltText, avatarName, ); } - const avatarClasses = classNames({ - mx_AvatarSetting_avatar: true, - mx_AvatarSetting_avatar_hovering: isHovering && uploadAvatar, - }); return ( -
+
{avatarElement}
@@ -531,10 +522,7 @@ exports[`RoomView for a local room in state NEW that is encrypted should match t
diff --git a/test/components/views/elements/RoomTopic-test.tsx b/test/components/views/elements/RoomTopic-test.tsx index dc05779794..8e62bd641f 100644 --- a/test/components/views/elements/RoomTopic-test.tsx +++ b/test/components/views/elements/RoomTopic-test.tsx @@ -16,7 +16,8 @@ limitations under the License. import React from "react"; import { Room } from "matrix-js-sdk/src/matrix"; -import { fireEvent, render, screen } from "@testing-library/react"; +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; import { mkEvent, stubClient } from "../../../test-utils"; import { MatrixClientPeg } from "../../../../src/MatrixClientPeg"; @@ -33,9 +34,12 @@ describe("", () => { window.location.href = originalHref; }); - function runClickTest(topic: string, clickText: string) { + /** + * Create a room with the given topic + * @param topic + */ + function createRoom(topic: string) { stubClient(); - const room = new Room("!pMBteVpcoJRdCJxDmn:matrix.org", MatrixClientPeg.safeGet(), "@alice:example.org"); const topicEvent = mkEvent({ type: "m.room.topic", @@ -45,11 +49,27 @@ describe("", () => { ts: 123, event: true, }); - room.addLiveEvents([topicEvent]); - render(); + return room; + } + /** + * Create a room and render it + * @param topic + */ + const renderRoom = (topic: string) => { + const room = createRoom(topic); + render(); + }; + + /** + * Create a room and click on the given text + * @param topic + * @param clickText + */ + function runClickTest(topic: string, clickText: string) { + renderRoom(topic); fireEvent.click(screen.getByText(clickText)); } @@ -78,4 +98,18 @@ describe("", () => { expect(window.location.href).toEqual(expectedHref); expect(dis.fire).toHaveBeenCalledWith(Action.ShowRoomTopic); }); + + it("should open the tooltip when hovering a text", async () => { + const topic = "room topic"; + renderRoom(topic); + await userEvent.hover(screen.getByText(topic)); + await waitFor(() => expect(screen.getByRole("tooltip", { name: "Click to read topic" })).toBeInTheDocument()); + }); + + it("should not open the tooltip when hovering a link", async () => { + const topic = "https://matrix.org"; + renderRoom(topic); + await userEvent.hover(screen.getByText(topic)); + await waitFor(() => expect(screen.queryByRole("tooltip", { name: "Click to read topic" })).toBeNull()); + }); }); diff --git a/test/components/views/elements/TooltipTarget-test.tsx b/test/components/views/elements/TooltipTarget-test.tsx deleted file mode 100644 index 0823229a90..0000000000 --- a/test/components/views/elements/TooltipTarget-test.tsx +++ /dev/null @@ -1,82 +0,0 @@ -/* -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 React from "react"; -import { fireEvent, render } from "@testing-library/react"; - -import { Alignment } from "../../../../src/components/views/elements/Tooltip"; -import TooltipTarget from "../../../../src/components/views/elements/TooltipTarget"; - -describe("", () => { - const defaultProps = { - "tooltipTargetClassName": "test tooltipTargetClassName", - "className": "test className", - "tooltipClassName": "test tooltipClassName", - "label": "test label", - "alignment": Alignment.Left, - "id": "test id", - "data-testid": "test", - }; - - const getComponent = (props = {}) => { - const wrapper = render( - // wrap in element so renderIntoDocument can render functional component - - - child - - , - ); - return wrapper.getByTestId("test"); - }; - - const getVisibleTooltip = () => document.querySelector(".mx_Tooltip.mx_Tooltip_visible"); - - it("renders container", () => { - const component = getComponent(); - expect(component).toMatchSnapshot(); - expect(getVisibleTooltip()).toBeFalsy(); - }); - - const alignmentKeys = Object.keys(Alignment).filter((o: any) => isNaN(o)); - it.each(alignmentKeys)("displays %s aligned tooltip on mouseover", async (alignment: any) => { - const wrapper = getComponent({ alignment: Alignment[alignment] })!; - fireEvent.mouseOver(wrapper); - expect(getVisibleTooltip()).toMatchSnapshot(); - }); - - it("hides tooltip on mouseleave", () => { - const wrapper = getComponent()!; - fireEvent.mouseOver(wrapper); - expect(getVisibleTooltip()).toBeTruthy(); - fireEvent.mouseLeave(wrapper); - expect(getVisibleTooltip()).toBeFalsy(); - }); - - it("displays tooltip on focus", () => { - const wrapper = getComponent()!; - fireEvent.focus(wrapper); - expect(getVisibleTooltip()).toBeTruthy(); - }); - - it("hides tooltip on blur", async () => { - const wrapper = getComponent()!; - fireEvent.focus(wrapper); - expect(getVisibleTooltip()).toBeTruthy(); - fireEvent.blur(wrapper); - expect(getVisibleTooltip()).toBeFalsy(); - }); -}); diff --git a/test/components/views/elements/__snapshots__/AppTile-test.tsx.snap b/test/components/views/elements/__snapshots__/AppTile-test.tsx.snap index 8f36256547..b344e3cd58 100644 --- a/test/components/views/elements/__snapshots__/AppTile-test.tsx.snap +++ b/test/components/views/elements/__snapshots__/AppTile-test.tsx.snap @@ -288,9 +288,7 @@ exports[`AppTile for a pinned widget should render permission request 1`] = ` Using this widget may share data
displays Bottom aligned tooltip on mouseover 1`] = ` -