Skip to content

Commit 599112e

Browse files
authored
Replace checkboxes with Compound checkboxes, and appropriately label each checkbox. (#29363)
* Fix labelling of avatar menu * Make the integration manager toggle more clear. * fix label * lint * Update snapshots. * Refactor many cases of checkbox to use the new compound component. * Remove non-checkbox related changes * Reset some things * Remove usages of mx_checkbox* styling. * Use label locators for apperance tests. * small linter tweaks * lint * update screenshot * Test updates * lint * Realign checkboxes for device selection. * Fixup QuickSettings styling * remove comment * lint * flex comment * remove unused label * remove redundant classes * add test for spaces * lint * Copyright * fixup spaces test * spaces lint * Replace pin with compound pin. * Realign icons * Remove hack for colouring icons * Adjust existing rooms component to correctly label room. * Add test for adding an existing room to an existing space. * Set deterministic sort order for rooms * lint
1 parent 170dcd1 commit 599112e

File tree

43 files changed

+2358
-1433
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

43 files changed

+2358
-1433
lines changed

playwright/e2e/settings/appearance-user-settings-tab/appearance-user-settings-tab.spec.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
Copyright 2024 New Vector Ltd.
2+
Copyright 2024,2025 New Vector Ltd.
33
Copyright 2023 Suguru Hirahara
44
55
SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
@@ -50,8 +50,8 @@ test.describe("Appearance user settings tab", () => {
5050
// Click "Show advanced" link button
5151
await tab.getByRole("button", { name: "Show advanced" }).click();
5252

53-
await tab.locator(".mx_Checkbox", { hasText: "Use bundled emoji font" }).click();
54-
await tab.locator(".mx_Checkbox", { hasText: "Use a system font" }).click();
53+
await tab.getByLabel("Use bundled emoji font").click();
54+
await tab.getByLabel("Use a system font").click();
5555

5656
// Assert that the font-family value was removed
5757
await expect(page.locator("body")).toHaveCSS("font-family", '""');

playwright/e2e/spaces/spaces.spec.ts

+85-7
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
Copyright 2024 New Vector Ltd.
2+
Copyright 2024,2025 New Vector Ltd.
33
Copyright 2022 The Matrix.org Foundation C.I.C.
44
55
SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
@@ -35,17 +35,18 @@ function spaceCreateOptions(spaceName: string, roomIds: string[] = []): ICreateR
3535
name: spaceName,
3636
},
3737
},
38-
...roomIds.map(spaceChildInitialState),
38+
...roomIds.map((r) => spaceChildInitialState(r)),
3939
],
4040
};
4141
}
4242

43-
function spaceChildInitialState(roomId: string): ICreateRoomOpts["initial_state"]["0"] {
43+
function spaceChildInitialState(roomId: string, order?: string): ICreateRoomOpts["initial_state"]["0"] {
4444
return {
4545
type: "m.space.child",
4646
state_key: roomId,
4747
content: {
4848
via: [roomId.split(":")[1]],
49+
order,
4950
},
5051
};
5152
}
@@ -121,9 +122,10 @@ test.describe("Spaces", () => {
121122
await page.getByRole("button", { name: "Skip for now" }).click();
122123

123124
// Assert rooms exist in the room list
124-
await expect(page.getByRole("treeitem", { name: "General", exact: true })).toBeVisible();
125-
await expect(page.getByRole("treeitem", { name: "Random", exact: true })).toBeVisible();
126-
await expect(page.getByRole("treeitem", { name: "Projects", exact: true })).toBeVisible();
125+
const roomList = page.getByRole("tree", { name: "Rooms" });
126+
await expect(roomList.getByRole("treeitem", { name: "General", exact: true })).toBeVisible();
127+
await expect(roomList.getByRole("treeitem", { name: "Random", exact: true })).toBeVisible();
128+
await expect(roomList.getByRole("treeitem", { name: "Projects", exact: true })).toBeVisible();
127129

128130
// Assert rooms exist in the space explorer
129131
await expect(
@@ -155,7 +157,7 @@ test.describe("Spaces", () => {
155157

156158
await page.getByRole("button", { name: "Just me" }).click();
157159

158-
await page.getByText("Sample Room").click({ force: true }); // force click as checkbox size is zero
160+
await page.getByRole("checkbox", { name: "Sample Room" }).click();
159161

160162
// Temporal implementation as multiple elements with the role "button" and name "Add" are found
161163
await page.locator(".mx_AddExistingToSpace_footer").getByRole("button", { name: "Add" }).click();
@@ -165,6 +167,50 @@ test.describe("Spaces", () => {
165167
).toBeVisible();
166168
});
167169

170+
test(
171+
"should allow user to add an existing room to a space after creation",
172+
{ tag: "@screenshot" },
173+
async ({ page, app, user }) => {
174+
await app.client.createRoom({
175+
name: "Sample Room",
176+
});
177+
await app.client.createRoom({
178+
name: "A Room that will not be selected",
179+
});
180+
181+
const menu = await openSpaceCreateMenu(page);
182+
await menu.getByRole("button", { name: "Private" }).click();
183+
184+
await menu
185+
.locator('.mx_SpaceBasicSettings_avatarContainer input[type="file"]')
186+
.setInputFiles("playwright/sample-files/riot.png");
187+
await expect(menu.getByRole("textbox", { name: "Address" })).not.toBeVisible();
188+
await menu
189+
.getByRole("textbox", { name: "Description" })
190+
.fill("This is a personal space to mourn Riot.im...");
191+
await menu.getByRole("textbox", { name: "Name" }).fill("This is my Riot");
192+
await menu.getByRole("textbox", { name: "Name" }).press("Enter");
193+
194+
await page.getByRole("button", { name: "Just me" }).click();
195+
196+
await page.getByRole("button", { name: "Skip for now" }).click();
197+
198+
await page.getByRole("button", { name: "Add room" }).click();
199+
await page.getByRole("menuitem", { name: "Add existing room" }).click();
200+
201+
await page.getByRole("checkbox", { name: "Sample Room" }).click();
202+
203+
await expect(page.getByRole("dialog", { name: "Avatar Add existing rooms" })).toMatchScreenshot(
204+
"add-existing-rooms-dialog.png",
205+
);
206+
207+
await page.getByRole("button", { name: "Add" }).click();
208+
await expect(
209+
page.locator(".mx_SpaceHierarchy_list").getByRole("treeitem", { name: "Sample Room" }),
210+
).toBeVisible();
211+
},
212+
);
213+
168214
test("should allow user to invite another to a space", { tag: "@no-webkit" }, async ({ page, app, user, bot }) => {
169215
await app.client.createSpace({
170216
visibility: "public" as any,
@@ -291,4 +337,36 @@ test.describe("Spaces", () => {
291337
// Assert we get shown the new room intro, and thus not the soft crash screen
292338
await expect(page.locator(".mx_NewRoomIntro")).toBeVisible();
293339
});
340+
341+
test("should render spaces view", { tag: "@screenshot" }, async ({ page, app, user, axe }) => {
342+
axe.disableRules([
343+
// Disable this check as it triggers on nested roving tab index elements which are in practice fine
344+
"nested-interactive",
345+
// XXX: We have some known contrast issues here
346+
"color-contrast",
347+
]);
348+
349+
const childSpaceId1 = await app.client.createSpace({
350+
name: "Child Space 1",
351+
initial_state: [],
352+
});
353+
const childSpaceId2 = await app.client.createSpace({
354+
name: "Child Space 2",
355+
initial_state: [],
356+
});
357+
const childSpaceId3 = await app.client.createSpace({
358+
name: "Child Space 3",
359+
initial_state: [],
360+
});
361+
await app.client.createSpace({
362+
name: "Root Space",
363+
initial_state: [
364+
spaceChildInitialState(childSpaceId1, "a"),
365+
spaceChildInitialState(childSpaceId2, "b"),
366+
spaceChildInitialState(childSpaceId3, "c"),
367+
],
368+
});
369+
await app.viewSpaceByName("Root Space");
370+
await expect(page.locator(".mx_SpaceRoomView")).toMatchScreenshot("space-room-view.png");
371+
});
294372
});
Loading
Loading
Loading

res/css/_components.pcss

-2
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,6 @@
128128
@import "./views/dialogs/_AddExistingToSpaceDialog.pcss";
129129
@import "./views/dialogs/_AnalyticsLearnMoreDialog.pcss";
130130
@import "./views/dialogs/_BugReportDialog.pcss";
131-
@import "./views/dialogs/_BulkRedactDialog.pcss";
132131
@import "./views/dialogs/_ChangelogDialog.pcss";
133132
@import "./views/dialogs/_CompoundDialog.pcss";
134133
@import "./views/dialogs/_ConfirmSpaceUserActionDialog.pcss";
@@ -212,7 +211,6 @@
212211
@import "./views/elements/_ServerPicker.pcss";
213212
@import "./views/elements/_SettingsFlag.pcss";
214213
@import "./views/elements/_Spinner.pcss";
215-
@import "./views/elements/_StyledCheckbox.pcss";
216214
@import "./views/elements/_StyledRadioButton.pcss";
217215
@import "./views/elements/_SyntaxHighlight.pcss";
218216
@import "./views/elements/_TagComposer.pcss";

res/css/components/views/settings/devices/_SelectableDeviceTile.pcss

+5-5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
Copyright 2024 New Vector Ltd.
2+
Copyright 2024,2025 New Vector Ltd.
33
Copyright 2022 The Matrix.org Foundation C.I.C.
44
55
SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
@@ -16,9 +16,9 @@ Please see LICENSE files in the repository root for full details.
1616
.mx_SelectableDeviceTile_checkbox {
1717
flex: 1 0;
1818

19-
.mx_Checkbox_background + div {
20-
flex: 1 0;
21-
/* override more specific selector */
22-
margin-left: $spacing-16 !important;
19+
> div {
20+
margin-top: auto;
21+
margin-bottom: auto;
22+
margin-right: var(--cpd-space-1x);
2323
}
2424
}

res/css/structures/_QuickSettingsButton.pcss

+16-33
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
Copyright 2024 New Vector Ltd.
2+
Copyright 2024,2025 New Vector Ltd.
33
Copyright 2021 The Matrix.org Foundation C.I.C.
44
55
SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
@@ -70,38 +70,26 @@ Please see LICENSE files in the repository root for full details.
7070
text-transform: uppercase;
7171
color: var(--cpd-color-text-secondary);
7272
margin: 20px 0 12px;
73-
}
74-
75-
.mx_QuickSettingsButton_pinToSidebarHeading {
76-
padding-left: 24px;
7773
position: relative;
78-
}
79-
80-
.mx_Checkbox {
81-
margin-bottom: 8px;
82-
}
83-
84-
.mx_QuickSettingsButton_favouritesCheckbox,
85-
.mx_QuickSettingsButton_peopleCheckbox {
86-
.mx_Checkbox_background + div {
87-
padding-left: 22px;
88-
position: relative;
89-
margin-left: 6px;
90-
font-size: $font-15px;
91-
line-height: $font-24px;
92-
color: var(--cpd-color-text-primary);
93-
}
74+
display: flex;
9475
}
9576

9677
.mx_QuickSettingsButton_moreOptionsButton {
97-
padding-left: 22px;
98-
margin-left: 22px;
78+
margin-left: var(--cpd-space-7x);
9979
font-size: $font-15px;
10080
line-height: $font-24px;
10181
color: var(--cpd-color-text-primary);
10282
position: relative;
10383
margin-bottom: 16px;
10484
}
85+
86+
.mx_QuickSettingsButton_option {
87+
margin-bottom: var(--cpd-space-3x);
88+
label {
89+
/* Correctly line up icons and text. */
90+
display: flex;
91+
}
92+
}
10593
}
10694

10795
.mx_QuickSettingsButton_ContextMenuWrapper_new_room_list {
@@ -111,15 +99,10 @@ Please see LICENSE files in the repository root for full details.
11199
}
112100

113101
.mx_QuickSettingsButton_icon {
114-
// TODO remove when all icons have fill=currentColor
115-
* {
116-
fill: $secondary-content;
117-
}
102+
margin-right: var(--cpd-space-1x);
118103
color: $secondary-content;
119-
width: 16px;
120-
height: 16px;
121-
position: absolute;
122-
left: 0;
123-
top: 50%;
124-
transform: translateY(-50%);
104+
width: 18px;
105+
height: 18px;
106+
margin-top: auto;
107+
margin-bottom: auto;
125108
}

res/css/structures/_SpaceHierarchy.pcss

+1-10
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
Copyright 2024 New Vector Ltd.
2+
Copyright 2024,2025 New Vector Ltd.
33
Copyright 2021 The Matrix.org Foundation C.I.C.
44
55
SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
@@ -247,15 +247,6 @@ Please see LICENSE files in the repository root for full details.
247247
.mx_AccessibleButton_kind_primary_outline {
248248
padding: 3px 16px; /* to account for the 1px border */
249249
}
250-
251-
.mx_Checkbox {
252-
display: inline-flex;
253-
254-
label {
255-
width: 16px;
256-
height: 16px;
257-
}
258-
}
259250
}
260251

261252
&:hover,

res/css/views/dialogs/_AddExistingToSpaceDialog.pcss

+12-5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
Copyright 2024 New Vector Ltd.
2+
Copyright 2024,2025 New Vector Ltd.
33
Copyright 2021 The Matrix.org Foundation C.I.C.
44
55
SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
@@ -32,6 +32,11 @@ Please see LICENSE files in the repository root for full details.
3232
.mx_AddExistingToSpace_section {
3333
margin-right: 12px;
3434

35+
ul {
36+
list-style: none;
37+
padding-left: 0;
38+
}
39+
3540
// provides space for scrollbar so that checkbox and scrollbar do not collide
3641

3742
&:not(:first-child) {
@@ -214,6 +219,12 @@ Please see LICENSE files in the repository root for full details.
214219
display: flex;
215220
margin-top: 12px;
216221

222+
form {
223+
/* Align checkboxes. */
224+
margin-top: auto;
225+
margin-bottom: auto;
226+
}
227+
217228
.mx_DecoratedRoomAvatar, /* we can't target .mx_BaseAvatar here as it'll break the decorated avatar styling */ {
218229
margin-right: 12px;
219230
}
@@ -227,8 +238,4 @@ Please see LICENSE files in the repository root for full details.
227238
text-overflow: ellipsis;
228239
margin-right: 12px;
229240
}
230-
231-
.mx_Checkbox {
232-
align-items: center;
233-
}
234241
}

res/css/views/dialogs/_BulkRedactDialog.pcss

-19
This file was deleted.

res/css/views/dialogs/_ExportDialog.pcss

+1-6
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
Copyright 2024 New Vector Ltd.
2+
Copyright 2024,2025 New Vector Ltd.
33
Copyright 2021 The Matrix.org Foundation C.I.C.
44
55
SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
@@ -43,11 +43,6 @@ Please see LICENSE files in the repository root for full details.
4343
.mx_Field_valid.mx_Field:focus-within {
4444
border-color: $input-border-color;
4545
}
46-
47-
.mx_Checkbox input[type="checkbox"]:checked + label > .mx_Checkbox_background {
48-
background: $info-plinth-fg-color;
49-
border-color: $info-plinth-fg-color;
50-
}
5146
}
5247

5348
.mx_ExportDialog_progress {

res/css/views/dialogs/_ManageRestrictedJoinRuleDialog.pcss

+1-5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
Copyright 2024 New Vector Ltd.
2+
Copyright 2024,2025 New Vector Ltd.
33
Copyright 2021 The Matrix.org Foundation C.I.C.
44
55
SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
@@ -74,10 +74,6 @@ Please see LICENSE files in the repository root for full details.
7474
line-height: $font-15px;
7575
color: $tertiary-content;
7676
}
77-
78-
.mx_Checkbox {
79-
align-items: center;
80-
}
8177
}
8278
}
8379

0 commit comments

Comments
 (0)