Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Fix visual regressions around widget permissions #10954

Merged
merged 9 commits into from
Jun 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions res/css/_components.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
46 changes: 17 additions & 29 deletions res/css/components/views/elements/_AppPermission.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -16,41 +16,29 @@ limitations under the License.
*/

.mx_AppPermission {
> div {
margin-bottom: 12px;
}

h4 {
margin: 0;
padding: 0;
}
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;

.mx_AppPermission_smallText {
font-size: $font-12px;
}
.mx_AppPermission_content {
margin-block: auto; /* place at the center */

.mx_AppPermission_bolder {
font-weight: var(--font-semi-bold);
}
> div {
margin-block: 12px;
}

.mx_AppPermission_helpIcon {
margin-top: 1px;
margin-right: 2px;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This margin-right property is no longer necessary. This has been used to add 2px space between the question mark and "with".

width: 10px;
height: 10px;
display: inline-block;
.mx_AppPermission_content_bolder {
font-weight: var(--font-semi-bold);
}

&::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;
}
}
}
}
Expand Down
25 changes: 25 additions & 0 deletions res/css/components/views/elements/_AppWarning.pcss
Original file line number Diff line number Diff line change
@@ -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;
}
}
8 changes: 0 additions & 8 deletions res/css/views/rooms/_AppsDrawer.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -311,22 +311,14 @@ 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 {
display: flex;
flex-direction: column;
justify-content: center;
align-items: center;
font-weight: bold;
position: relative;
height: 100%;

Expand Down
2 changes: 1 addition & 1 deletion res/img/feather-customised/help-circle.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
35 changes: 20 additions & 15 deletions src/components/views/elements/AppPermission.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -117,8 +119,9 @@ export default class AppPermission extends React.Component<IProps, IState> {
<TextWithTooltip
tooltip={warningTooltipText}
tooltipClass="mx_Tooltip--appPermission mx_Tooltip--appPermission--dark"
class="mx_TextWithTooltip_target--helpIcon"
>
<span className="mx_AppPermission_helpIcon" />
<HelpIcon className="mx_Icon mx_Icon_12" />
</TextWithTooltip>
);

Expand All @@ -139,20 +142,22 @@ export default class AppPermission extends React.Component<IProps, IState> {

return (
<div className="mx_AppPermission">
<div className="mx_AppPermission_bolder mx_AppPermission_smallText">{_t("Widget added by")}</div>
<div>
{avatar}
<h4 className="mx_AppPermission_bolder">{displayName}</h4>
<div className="mx_AppPermission_smallText">{userId}</div>
</div>
<div className="mx_AppPermission_smallText">{warning}</div>
<div className="mx_AppPermission_smallText">
Comment on lines -142 to -149
Copy link
Contributor Author

@luixxiul luixxiul May 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since Heading component takes care of the h4's font size, mx_AppPermission_smallText can be removed by setting the font size it specifies by default (see above here).

{_t("This widget may use cookies.")}&nbsp;{encryptionWarning}
</div>
<div>
<AccessibleButton kind="primary_sm" onClick={this.props.onPermissionGranted}>
{_t("Continue")}
</AccessibleButton>
<div className="mx_AppPermission_content">
<div className="mx_AppPermission_content_bolder">{_t("Widget added by")}</div>
<div>
{avatar}
<Heading size="h4">{displayName}</Heading>
<div>{userId}</div>
</div>
<div>{warning}</div>
<div>
{_t("This widget may use cookies.")}&nbsp;{encryptionWarning}
</div>
<div>
<AccessibleButton kind="primary_sm" onClick={this.props.onPermissionGranted}>
{_t("Continue")}
</AccessibleButton>
</div>
</div>
</div>
);
Expand Down
61 changes: 44 additions & 17 deletions test/components/views/elements/AppTile-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(
<MatrixClientContext.Provider value={cli}>
<AppTile key={app1.id} app={app1} room={r1} userId="@user1" creatorUserId="@userAnother" />
</MatrixClientContext.Provider>,
);

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(
<MatrixClientContext.Provider value={cli}>
<AppTile key={app1.id} app={app1} room={r1} userId="@user1" creatorUserId="@userAnother" />
</MatrixClientContext.Provider>,
);

expect(renderResult.queryByRole("button", { name: "Continue" })).not.toBeInTheDocument();
});

describe("for a maximised (centered) widget", () => {
beforeEach(() => {
jest.spyOn(WidgetLayoutStore.instance, "isInContainer").mockImplementation(
Expand Down Expand Up @@ -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(
<MatrixClientContext.Provider value={cli}>
<AppTile key={app1.id} app={app1} room={r1} userId="@user1" creatorUserId="@userAnother" />
</MatrixClientContext.Provider>,
);

expect(renderResult.queryByRole("button", { name: "Continue" })).not.toBeInTheDocument();
});
});
Loading