From b40f29f04cc86db8147f511af8a9f994ac997f1a Mon Sep 17 00:00:00 2001 From: Suguru Hirahara Date: Wed, 14 Jun 2023 11:11:06 +0000 Subject: [PATCH] Fix visual regressions around widget permissions (#10954) * Add a Jest snapshot of AppPermission * Move the test inside 'for a pinned widget' category * Make only spinner message bold * Set font size specified with "mx_AppPermission_smallText" by default - Add "mx_AppPermission_largeText" for elements whose size has not been specified with mx_AppPermission_smallText - Create _AppWarning.pcss for AppWarning * Make AppPermission panel scrollable, keeping the content at the center * Run prettier * Use Heading component * Use Icon component * Fix the test --- res/css/_components.pcss | 1 + .../views/elements/_AppPermission.pcss | 46 +++--- .../views/elements/_AppWarning.pcss | 25 +++ res/css/views/rooms/_AppsDrawer.pcss | 8 - res/img/feather-customised/help-circle.svg | 2 +- .../views/elements/AppPermission.tsx | 35 +++-- .../views/elements/AppTile-test.tsx | 61 +++++--- .../__snapshots__/AppTile-test.tsx.snap | 143 ++++++++++++++++++ 8 files changed, 251 insertions(+), 70 deletions(-) create mode 100644 res/css/components/views/elements/_AppWarning.pcss diff --git a/res/css/_components.pcss b/res/css/_components.pcss index 56628095f2..82f8cda556 100644 --- a/res/css/_components.pcss +++ b/res/css/_components.pcss @@ -21,6 +21,7 @@ @import "./components/views/dialogs/polls/_PollListItem.pcss"; @import "./components/views/dialogs/polls/_PollListItemEnded.pcss"; @import "./components/views/elements/_AppPermission.pcss"; +@import "./components/views/elements/_AppWarning.pcss"; @import "./components/views/elements/_FilterDropdown.pcss"; @import "./components/views/elements/_FilterTabGroup.pcss"; @import "./components/views/elements/_LearnMore.pcss"; diff --git a/res/css/components/views/elements/_AppPermission.pcss b/res/css/components/views/elements/_AppPermission.pcss index be78efa43b..71f282ebee 100644 --- a/res/css/components/views/elements/_AppPermission.pcss +++ b/res/css/components/views/elements/_AppPermission.pcss @@ -16,41 +16,29 @@ limitations under the License. */ .mx_AppPermission { - > div { - margin-bottom: 12px; - } + font-size: $font-12px; + width: 100%; /* make mx_AppPermission fill width of mx_AppTileBody so that scroll bar appears on the edge */ + overflow-y: scroll; - h4 { - margin: 0; - padding: 0; - } + .mx_AppPermission_content { + margin-block: auto; /* place at the center */ - .mx_AppPermission_smallText { - font-size: $font-12px; - } + > div { + margin-block: 12px; + } - .mx_AppPermission_bolder { - font-weight: var(--font-semi-bold); - } + .mx_AppPermission_content_bolder { + font-weight: var(--font-semi-bold); + } - .mx_AppPermission_helpIcon { - margin-top: 1px; - margin-right: 2px; - width: 10px; - height: 10px; - display: inline-block; - - &::before { + .mx_TextWithTooltip_target--helpIcon { display: inline-block; - background-color: $accent; - mask-repeat: no-repeat; - mask-size: 12px; - width: 12px; - height: 12px; - mask-position: center; - content: ""; + height: $font-14px; /* align with characters on the same line */ vertical-align: middle; - mask-image: url("$(res)/img/feather-customised/help-circle.svg"); + + .mx_Icon { + color: $accent; + } } } } diff --git a/res/css/components/views/elements/_AppWarning.pcss b/res/css/components/views/elements/_AppWarning.pcss new file mode 100644 index 0000000000..8d859d12a8 --- /dev/null +++ b/res/css/components/views/elements/_AppWarning.pcss @@ -0,0 +1,25 @@ +/* +Copyright 2023 Suguru Hirahara + +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. +*/ + +.mx_AppWarning { + font-size: $font-16px; + justify-content: center; + + h4 { + margin: 0; + padding: 0; + } +} diff --git a/res/css/views/rooms/_AppsDrawer.pcss b/res/css/views/rooms/_AppsDrawer.pcss index 20a68b4136..306e4c103c 100644 --- a/res/css/views/rooms/_AppsDrawer.pcss +++ b/res/css/views/rooms/_AppsDrawer.pcss @@ -311,14 +311,7 @@ limitations under the License. display: flex; height: 100%; flex-direction: column; - justify-content: center; align-items: center; - font-size: $font-16px; - - h4 { - margin: 0; - padding: 0; - } } .mx_AppTile_loading { @@ -326,7 +319,6 @@ limitations under the License. flex-direction: column; justify-content: center; align-items: center; - font-weight: bold; position: relative; height: 100%; diff --git a/res/img/feather-customised/help-circle.svg b/res/img/feather-customised/help-circle.svg index 7ecb0a8f35..61b853aae8 100644 --- a/res/img/feather-customised/help-circle.svg +++ b/res/img/feather-customised/help-circle.svg @@ -1,5 +1,5 @@ - + diff --git a/src/components/views/elements/AppPermission.tsx b/src/components/views/elements/AppPermission.tsx index e73c8ddf8a..f57a5e4a29 100644 --- a/src/components/views/elements/AppPermission.tsx +++ b/src/components/views/elements/AppPermission.tsx @@ -25,9 +25,11 @@ import WidgetUtils from "../../../utils/WidgetUtils"; import { MatrixClientPeg } from "../../../MatrixClientPeg"; import MemberAvatar from "../avatars/MemberAvatar"; import BaseAvatar from "../avatars/BaseAvatar"; +import Heading from "../typography/Heading"; import AccessibleButton from "./AccessibleButton"; import TextWithTooltip from "./TextWithTooltip"; import { parseUrl } from "../../../utils/UrlUtils"; +import { Icon as HelpIcon } from "../../../../res/img/feather-customised/help-circle.svg"; interface IProps { url: string; @@ -117,8 +119,9 @@ export default class AppPermission extends React.Component { - + ); @@ -139,20 +142,22 @@ export default class AppPermission extends React.Component { return (
-
{_t("Widget added by")}
-
- {avatar} -

{displayName}

-
{userId}
-
-
{warning}
-
- {_t("This widget may use cookies.")} {encryptionWarning} -
-
- - {_t("Continue")} - +
+
{_t("Widget added by")}
+
+ {avatar} + {displayName} +
{userId}
+
+
{warning}
+
+ {_t("This widget may use cookies.")} {encryptionWarning} +
+
+ + {_t("Continue")} + +
); diff --git a/test/components/views/elements/AppTile-test.tsx b/test/components/views/elements/AppTile-test.tsx index 8f77db9f1e..5a825b2a8b 100644 --- a/test/components/views/elements/AppTile-test.tsx +++ b/test/components/views/elements/AppTile-test.tsx @@ -62,6 +62,11 @@ jest.mock("../../../../src/stores/OwnProfileStore", () => ({ }, })); +// Fake random strings to give a predictable snapshot +jest.mock("matrix-js-sdk/src/randomstring", () => ({ + randomString: () => "abdefghi", +})); + describe("AppTile", () => { let cli: MatrixClient; let r1: Room; @@ -387,6 +392,45 @@ describe("AppTile", () => { expect(moveToContainerSpy).toHaveBeenCalledWith(r1, app1, Container.Center); }); + it("should render permission request", () => { + jest.spyOn(ModuleRunner.instance, "invoke").mockImplementation((lifecycleEvent, opts, widgetInfo) => { + if (lifecycleEvent === WidgetLifecycle.PreLoadRequest && (widgetInfo as WidgetInfo).id === app1.id) { + (opts as ApprovalOpts).approved = false; + } + }); + + // userId and creatorUserId are different + const renderResult = render( + + + , + ); + + const { container, asFragment } = renderResult; + + expect(container.querySelector(".mx_Spinner")).toBeFalsy(); + expect(asFragment()).toMatchSnapshot(); + + expect(renderResult.queryByRole("button", { name: "Continue" })).toBeInTheDocument(); + }); + + it("should not display 'Continue' button on permission load", () => { + jest.spyOn(ModuleRunner.instance, "invoke").mockImplementation((lifecycleEvent, opts, widgetInfo) => { + if (lifecycleEvent === WidgetLifecycle.PreLoadRequest && (widgetInfo as WidgetInfo).id === app1.id) { + (opts as ApprovalOpts).approved = true; + } + }); + + // userId and creatorUserId are different + const renderResult = render( + + + , + ); + + expect(renderResult.queryByRole("button", { name: "Continue" })).not.toBeInTheDocument(); + }); + describe("for a maximised (centered) widget", () => { beforeEach(() => { jest.spyOn(WidgetLayoutStore.instance, "isInContainer").mockImplementation( @@ -446,21 +490,4 @@ describe("AppTile", () => { expect(asFragment()).toMatchSnapshot(); }); }); - - it("for a pinned widget permission load", () => { - jest.spyOn(ModuleRunner.instance, "invoke").mockImplementation((lifecycleEvent, opts, widgetInfo) => { - if (lifecycleEvent === WidgetLifecycle.PreLoadRequest && (widgetInfo as WidgetInfo).id === app1.id) { - (opts as ApprovalOpts).approved = true; - } - }); - - // userId and creatorUserId are different - const renderResult = render( - - - , - ); - - expect(renderResult.queryByRole("button", { name: "Continue" })).not.toBeInTheDocument(); - }); }); diff --git a/test/components/views/elements/__snapshots__/AppTile-test.tsx.snap b/test/components/views/elements/__snapshots__/AppTile-test.tsx.snap index 9f345ff82c..dac8883dec 100644 --- a/test/components/views/elements/__snapshots__/AppTile-test.tsx.snap +++ b/test/components/views/elements/__snapshots__/AppTile-test.tsx.snap @@ -163,6 +163,149 @@ exports[`AppTile for a pinned widget should render 1`] = ` `; +exports[`AppTile for a pinned widget should render permission request 1`] = ` + +
+
+ + + + + Example 1 + + + + + +
+
+
+
+
+
+ +
+
+
+
+ Widget added by +
+
+ + + + +

+ @userAnother +

+
+
+
+ + Using this widget may share data +
+
+
+ with example.com. + +
+
+ This widget may use cookies.  +
+
+
+ Continue +
+
+
+
+
+
+ +`; + exports[`AppTile preserves non-persisted widget on container move 1`] = `