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

Commit b40f29f

Browse files
authored
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
1 parent 127b542 commit b40f29f

File tree

8 files changed

+251
-70
lines changed

8 files changed

+251
-70
lines changed

res/css/_components.pcss

+1
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
@import "./components/views/dialogs/polls/_PollListItem.pcss";
2222
@import "./components/views/dialogs/polls/_PollListItemEnded.pcss";
2323
@import "./components/views/elements/_AppPermission.pcss";
24+
@import "./components/views/elements/_AppWarning.pcss";
2425
@import "./components/views/elements/_FilterDropdown.pcss";
2526
@import "./components/views/elements/_FilterTabGroup.pcss";
2627
@import "./components/views/elements/_LearnMore.pcss";

res/css/components/views/elements/_AppPermission.pcss

+17-29
Original file line numberDiff line numberDiff line change
@@ -16,41 +16,29 @@ limitations under the License.
1616
*/
1717

1818
.mx_AppPermission {
19-
> div {
20-
margin-bottom: 12px;
21-
}
22-
23-
h4 {
24-
margin: 0;
25-
padding: 0;
26-
}
19+
font-size: $font-12px;
20+
width: 100%; /* make mx_AppPermission fill width of mx_AppTileBody so that scroll bar appears on the edge */
21+
overflow-y: scroll;
2722

28-
.mx_AppPermission_smallText {
29-
font-size: $font-12px;
30-
}
23+
.mx_AppPermission_content {
24+
margin-block: auto; /* place at the center */
3125

32-
.mx_AppPermission_bolder {
33-
font-weight: var(--font-semi-bold);
34-
}
26+
> div {
27+
margin-block: 12px;
28+
}
3529

36-
.mx_AppPermission_helpIcon {
37-
margin-top: 1px;
38-
margin-right: 2px;
39-
width: 10px;
40-
height: 10px;
41-
display: inline-block;
30+
.mx_AppPermission_content_bolder {
31+
font-weight: var(--font-semi-bold);
32+
}
4233

43-
&::before {
34+
.mx_TextWithTooltip_target--helpIcon {
4435
display: inline-block;
45-
background-color: $accent;
46-
mask-repeat: no-repeat;
47-
mask-size: 12px;
48-
width: 12px;
49-
height: 12px;
50-
mask-position: center;
51-
content: "";
36+
height: $font-14px; /* align with characters on the same line */
5237
vertical-align: middle;
53-
mask-image: url("$(res)/img/feather-customised/help-circle.svg");
38+
39+
.mx_Icon {
40+
color: $accent;
41+
}
5442
}
5543
}
5644
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
/*
2+
Copyright 2023 Suguru Hirahara
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
.mx_AppWarning {
18+
font-size: $font-16px;
19+
justify-content: center;
20+
21+
h4 {
22+
margin: 0;
23+
padding: 0;
24+
}
25+
}

res/css/views/rooms/_AppsDrawer.pcss

-8
Original file line numberDiff line numberDiff line change
@@ -311,22 +311,14 @@ limitations under the License.
311311
display: flex;
312312
height: 100%;
313313
flex-direction: column;
314-
justify-content: center;
315314
align-items: center;
316-
font-size: $font-16px;
317-
318-
h4 {
319-
margin: 0;
320-
padding: 0;
321-
}
322315
}
323316

324317
.mx_AppTile_loading {
325318
display: flex;
326319
flex-direction: column;
327320
justify-content: center;
328321
align-items: center;
329-
font-weight: bold;
330322
position: relative;
331323
height: 100%;
332324

res/img/feather-customised/help-circle.svg

+1-1
Loading

src/components/views/elements/AppPermission.tsx

+20-15
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,11 @@ import WidgetUtils from "../../../utils/WidgetUtils";
2525
import { MatrixClientPeg } from "../../../MatrixClientPeg";
2626
import MemberAvatar from "../avatars/MemberAvatar";
2727
import BaseAvatar from "../avatars/BaseAvatar";
28+
import Heading from "../typography/Heading";
2829
import AccessibleButton from "./AccessibleButton";
2930
import TextWithTooltip from "./TextWithTooltip";
3031
import { parseUrl } from "../../../utils/UrlUtils";
32+
import { Icon as HelpIcon } from "../../../../res/img/feather-customised/help-circle.svg";
3133

3234
interface IProps {
3335
url: string;
@@ -117,8 +119,9 @@ export default class AppPermission extends React.Component<IProps, IState> {
117119
<TextWithTooltip
118120
tooltip={warningTooltipText}
119121
tooltipClass="mx_Tooltip--appPermission mx_Tooltip--appPermission--dark"
122+
class="mx_TextWithTooltip_target--helpIcon"
120123
>
121-
<span className="mx_AppPermission_helpIcon" />
124+
<HelpIcon className="mx_Icon mx_Icon_12" />
122125
</TextWithTooltip>
123126
);
124127

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

140143
return (
141144
<div className="mx_AppPermission">
142-
<div className="mx_AppPermission_bolder mx_AppPermission_smallText">{_t("Widget added by")}</div>
143-
<div>
144-
{avatar}
145-
<h4 className="mx_AppPermission_bolder">{displayName}</h4>
146-
<div className="mx_AppPermission_smallText">{userId}</div>
147-
</div>
148-
<div className="mx_AppPermission_smallText">{warning}</div>
149-
<div className="mx_AppPermission_smallText">
150-
{_t("This widget may use cookies.")}&nbsp;{encryptionWarning}
151-
</div>
152-
<div>
153-
<AccessibleButton kind="primary_sm" onClick={this.props.onPermissionGranted}>
154-
{_t("Continue")}
155-
</AccessibleButton>
145+
<div className="mx_AppPermission_content">
146+
<div className="mx_AppPermission_content_bolder">{_t("Widget added by")}</div>
147+
<div>
148+
{avatar}
149+
<Heading size="h4">{displayName}</Heading>
150+
<div>{userId}</div>
151+
</div>
152+
<div>{warning}</div>
153+
<div>
154+
{_t("This widget may use cookies.")}&nbsp;{encryptionWarning}
155+
</div>
156+
<div>
157+
<AccessibleButton kind="primary_sm" onClick={this.props.onPermissionGranted}>
158+
{_t("Continue")}
159+
</AccessibleButton>
160+
</div>
156161
</div>
157162
</div>
158163
);

test/components/views/elements/AppTile-test.tsx

+44-17
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,11 @@ jest.mock("../../../../src/stores/OwnProfileStore", () => ({
6262
},
6363
}));
6464

65+
// Fake random strings to give a predictable snapshot
66+
jest.mock("matrix-js-sdk/src/randomstring", () => ({
67+
randomString: () => "abdefghi",
68+
}));
69+
6570
describe("AppTile", () => {
6671
let cli: MatrixClient;
6772
let r1: Room;
@@ -387,6 +392,45 @@ describe("AppTile", () => {
387392
expect(moveToContainerSpy).toHaveBeenCalledWith(r1, app1, Container.Center);
388393
});
389394

395+
it("should render permission request", () => {
396+
jest.spyOn(ModuleRunner.instance, "invoke").mockImplementation((lifecycleEvent, opts, widgetInfo) => {
397+
if (lifecycleEvent === WidgetLifecycle.PreLoadRequest && (widgetInfo as WidgetInfo).id === app1.id) {
398+
(opts as ApprovalOpts).approved = false;
399+
}
400+
});
401+
402+
// userId and creatorUserId are different
403+
const renderResult = render(
404+
<MatrixClientContext.Provider value={cli}>
405+
<AppTile key={app1.id} app={app1} room={r1} userId="@user1" creatorUserId="@userAnother" />
406+
</MatrixClientContext.Provider>,
407+
);
408+
409+
const { container, asFragment } = renderResult;
410+
411+
expect(container.querySelector(".mx_Spinner")).toBeFalsy();
412+
expect(asFragment()).toMatchSnapshot();
413+
414+
expect(renderResult.queryByRole("button", { name: "Continue" })).toBeInTheDocument();
415+
});
416+
417+
it("should not display 'Continue' button on permission load", () => {
418+
jest.spyOn(ModuleRunner.instance, "invoke").mockImplementation((lifecycleEvent, opts, widgetInfo) => {
419+
if (lifecycleEvent === WidgetLifecycle.PreLoadRequest && (widgetInfo as WidgetInfo).id === app1.id) {
420+
(opts as ApprovalOpts).approved = true;
421+
}
422+
});
423+
424+
// userId and creatorUserId are different
425+
const renderResult = render(
426+
<MatrixClientContext.Provider value={cli}>
427+
<AppTile key={app1.id} app={app1} room={r1} userId="@user1" creatorUserId="@userAnother" />
428+
</MatrixClientContext.Provider>,
429+
);
430+
431+
expect(renderResult.queryByRole("button", { name: "Continue" })).not.toBeInTheDocument();
432+
});
433+
390434
describe("for a maximised (centered) widget", () => {
391435
beforeEach(() => {
392436
jest.spyOn(WidgetLayoutStore.instance, "isInContainer").mockImplementation(
@@ -446,21 +490,4 @@ describe("AppTile", () => {
446490
expect(asFragment()).toMatchSnapshot();
447491
});
448492
});
449-
450-
it("for a pinned widget permission load", () => {
451-
jest.spyOn(ModuleRunner.instance, "invoke").mockImplementation((lifecycleEvent, opts, widgetInfo) => {
452-
if (lifecycleEvent === WidgetLifecycle.PreLoadRequest && (widgetInfo as WidgetInfo).id === app1.id) {
453-
(opts as ApprovalOpts).approved = true;
454-
}
455-
});
456-
457-
// userId and creatorUserId are different
458-
const renderResult = render(
459-
<MatrixClientContext.Provider value={cli}>
460-
<AppTile key={app1.id} app={app1} room={r1} userId="@user1" creatorUserId="@userAnother" />
461-
</MatrixClientContext.Provider>,
462-
);
463-
464-
expect(renderResult.queryByRole("button", { name: "Continue" })).not.toBeInTheDocument();
465-
});
466493
});

0 commit comments

Comments
 (0)