Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use EditInPlace control for Identity Server picker to improve a11y #29280

Merged
merged 18 commits into from
Feb 21, 2025
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
53 changes: 47 additions & 6 deletions playwright/e2e/settings/security-user-settings-tab.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ test.describe("Security user settings tab", () => {
});

test.describe("AnalyticsLearnMoreDialog", () => {
test("should be rendered properly", { tag: "@screenshot" }, async ({ app, page }) => {
test("should be rendered properly", { tag: "@screenshot" }, async ({ app, page, user }) => {
const tab = await app.settings.openUserSettings("Security");
await tab.getByRole("button", { name: "Learn more" }).click();
await expect(page.locator(".mx_AnalyticsLearnMoreDialog_wrapper .mx_Dialog")).toMatchScreenshot(
Expand All @@ -41,16 +41,57 @@ test.describe("Security user settings tab", () => {
});
});

test("should contain section to set ID server", async ({ app }) => {
test("should be able to set an ID server", async ({ app, context, user, page }) => {
const tab = await app.settings.openUserSettings("Security");

const setIdServer = tab.locator(".mx_SetIdServer");
await context.route("https://identity.example.org/_matrix/identity/v2", async (route) => {
await route.fulfill({
status: 200,
json: {},
});
});
await context.route("https://identity.example.org/_matrix/identity/v2/account/register", async (route) => {
await route.fulfill({
status: 200,
json: {
token: "AToken",
},
});
});
await context.route("https://identity.example.org/_matrix/identity/v2/account", async (route) => {
await route.fulfill({
status: 200,
json: {
user_id: user.userId,
},
});
});
await context.route("https://identity.example.org/_matrix/identity/v2/terms", async (route) => {
await route.fulfill({
status: 200,
json: {
policies: {},
},
});
});
const setIdServer = tab.locator(".mx_IdentityServerPicker");
await setIdServer.scrollIntoViewIfNeeded();
// Assert that an input area for identity server exists
await expect(setIdServer.getByRole("textbox", { name: "Enter a new identity server" })).toBeVisible();

const textElement = setIdServer.getByRole("textbox", { name: "Enter a new identity server" });
await textElement.click();
await textElement.fill("https://identity.example.org");
await setIdServer.getByRole("button", { name: "Change" }).click();

await expect(setIdServer.getByText("Checking server")).toBeVisible();
// Accept terms
await page.getByTestId("dialog-primary-button").click();
// Check identity has changed.
await expect(setIdServer.getByText("Your identity server has been changed")).toBeVisible();
// Ensure section title is updated.
await expect(tab.getByText(`Identity server (identity.example.org)`, { exact: true })).toBeVisible();
});

test("should enable show integrations as enabled", async ({ app, page }) => {
test("should enable show integrations as enabled", async ({ app, page, user }) => {
const tab = await app.settings.openUserSettings("Security");

const setIntegrationManager = tab.locator(".mx_SetIntegrationManager");
Expand Down
84 changes: 56 additions & 28 deletions res/css/_common.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -589,18 +589,21 @@ legend {
* in the app look the same by being AccessibleButtons, or possibly by having explict button classes.
* We should go through and have one consistent set of styles for buttons throughout the app.
* For now, I am duplicating the selectors here for mx_Dialog and mx_DialogButtons.
*
* Elements that should not be styled like a dialog button are mentioned in a :not() pseudo-class.
* For the widest browser support, we use multiple :not pseudo-classes instead of :not(.a, .b).
*/
.mx_Dialog
button:not(.mx_Dialog_nonDialogButton):not([class|="maplibregl"]):not(.mx_AccessibleButton):not(
.mx_UserProfileSettings button
):not(.mx_ThemeChoicePanel_CustomTheme button):not(.mx_UnpinAllDialog button):not(.mx_ShareDialog button):not(
.mx_EncryptionUserSettingsTab button
button:not(
Copy link
Member Author

Choose a reason for hiding this comment

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

Browser support for this is now okay https://developer.mozilla.org/en-US/docs/Web/CSS/:not#browser_compatibility.

I caused a bug here, and it was borderline unreadable before, having a list of selectors like this makes this a bit easier to parse.

.mx_EncryptionUserSettingsTab button,
.mx_UserProfileSettings button,
.mx_ShareDialog button,
.mx_UnpinAllDialog button,
.mx_ThemeChoicePanel_CustomTheme button,
.mx_Dialog_nonDialogButton,
.mx_AccessibleButton,
.mx_IdentityServerPicker button,
[class|="maplibregl"]
),
.mx_Dialog_buttons button:not(.mx_Dialog_nonDialogButton, .mx_AccessibleButton),
.mx_Dialog input[type="submit"],
.mx_Dialog_buttons button:not(.mx_Dialog_nonDialogButton):not(.mx_AccessibleButton),
.mx_Dialog_buttons input[type="submit"] {
@mixin mx_DialogButton;
margin-left: 0px;
Expand All @@ -616,32 +619,46 @@ legend {
}

.mx_Dialog
button:not(.mx_Dialog_nonDialogButton):not([class|="maplibregl"]):not(.mx_AccessibleButton):not(
.mx_UserProfileSettings button
):not(.mx_ThemeChoicePanel_CustomTheme button):not(.mx_UnpinAllDialog button):not(.mx_ShareDialog button):not(
button:not(
.mx_Dialog_nonDialogButton,
[class|="maplibregl"],
.mx_AccessibleButton,
.mx_UserProfileSettings button,
.mx_ThemeChoicePanel_CustomTheme button,
.mx_UnpinAllDialog button,
.mx_ShareDialog button,
.mx_EncryptionUserSettingsTab button
):last-child {
margin-right: 0px;
}

.mx_Dialog
button:not(.mx_Dialog_nonDialogButton):not([class|="maplibregl"]):not(.mx_AccessibleButton):not(
.mx_UserProfileSettings button
):not(.mx_ThemeChoicePanel_CustomTheme button):not(.mx_UnpinAllDialog button):not(.mx_ShareDialog button):not(
button:not(
.mx_Dialog_nonDialogButton,
[class|="maplibregl"],
.mx_AccessibleButton,
.mx_UserProfileSettings button,
.mx_ThemeChoicePanel_CustomTheme button,
.mx_UnpinAllDialog button,
.mx_ShareDialog button,
.mx_EncryptionUserSettingsTab button
):focus,
.mx_Dialog input[type="submit"]:focus,
.mx_Dialog_buttons button:not(.mx_Dialog_nonDialogButton):not(.mx_AccessibleButton):focus,
.mx_Dialog_buttons button:not(.mx_Dialog_nonDialogButton, .mx_AccessibleButton):focus,
.mx_Dialog_buttons input[type="submit"]:focus {
filter: brightness($focus-brightness);
}

.mx_Dialog button.mx_Dialog_primary:not(.mx_Dialog_nonDialogButton):not([class|="maplibregl"]),
.mx_Dialog button.mx_Dialog_primary:not(.mx_Dialog_nonDialogButton, [class|="maplibregl"]),
.mx_Dialog input[type="submit"].mx_Dialog_primary,
.mx_Dialog_buttons
button.mx_Dialog_primary:not(.mx_Dialog_nonDialogButton):not(.mx_AccessibleButton):not(
.mx_UserProfileSettings button
):not(.mx_ThemeChoicePanel_CustomTheme button):not(.mx_UnpinAllDialog button):not(.mx_ShareDialog button):not(
button:not(
.mx_Dialog_nonDialogButton,
.mx_AccessibleButton,
.mx_UserProfileSettings button,
.mx_ThemeChoicePanel_CustomTheme button,
.mx_UnpinAllDialog button,
.mx_ShareDialog button,
.mx_EncryptionUserSettingsTab button
),
.mx_Dialog_buttons input[type="submit"].mx_Dialog_primary {
Expand All @@ -651,32 +668,43 @@ legend {
min-width: 156px;
}

.mx_Dialog button.danger:not(.mx_Dialog_nonDialogButton):not([class|="maplibregl"]),
.mx_Dialog button.danger:not(.mx_Dialog_nonDialogButton, [class|="maplibregl"]),
.mx_Dialog input[type="submit"].danger,
.mx_Dialog_buttons
button.danger:not(.mx_Dialog_nonDialogButton):not(.mx_AccessibleButton):not(.mx_UserProfileSettings button):not(
.mx_ThemeChoicePanel_CustomTheme button
):not(.mx_UnpinAllDialog button):not(.mx_ShareDialog button):not(.mx_EncryptionUserSettingsTab button),
button.danger:not(
.mx_Dialog_nonDialogButton,
.mx_AccessibleButton,
.mx_UserProfileSettings button,
.mx_ThemeChoicePanel_CustomTheme button,
.mx_UnpinAllDialog button,
.mx_ShareDialog button,
.mx_EncryptionUserSettingsTab button
),
.mx_Dialog_buttons input[type="submit"].danger {
background-color: var(--cpd-color-bg-critical-primary);
border: solid 1px var(--cpd-color-bg-critical-primary);
color: var(--cpd-color-text-on-solid-primary);
}

.mx_Dialog button.warning:not(.mx_Dialog_nonDialogButton):not([class|="maplibregl"]),
.mx_Dialog button.warning:not(.mx_Dialog_nonDialogButton, [class|="maplibregl"]),
.mx_Dialog input[type="submit"].warning {
border: solid 1px var(--cpd-color-border-critical-subtle);
color: var(--cpd-color-text-critical-primary);
}

.mx_Dialog
button:not(.mx_Dialog_nonDialogButton):not([class|="maplibregl"]):not(.mx_AccessibleButton):not(
.mx_UserProfileSettings button
):not(.mx_ThemeChoicePanel_CustomTheme button):not(.mx_UnpinAllDialog button):not(.mx_ShareDialog button):not(
button:not(
.mx_Dialog_nonDialogButton,
[class|="maplibregl"],
.mx_AccessibleButton,
.mx_UserProfileSettings button,
.mx_ThemeChoicePanel_CustomTheme button,
.mx_UnpinAllDialog button,
.mx_ShareDialog button,
.mx_EncryptionUserSettingsTab button
):disabled,
.mx_Dialog input[type="submit"]:disabled,
.mx_Dialog_buttons button:not(.mx_Dialog_nonDialogButton):not(.mx_AccessibleButton):disabled,
.mx_Dialog_buttons button:not(.mx_Dialog_nonDialogButton, .mx_AccessibleButton):disabled,
.mx_Dialog_buttons input[type="submit"]:disabled {
background-color: $light-fg-color;
border: solid 1px $light-fg-color;
Expand Down
1 change: 0 additions & 1 deletion res/css/_components.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,6 @@
@import "./views/settings/_PowerLevelSelector.pcss";
@import "./views/settings/_RoomProfileSettings.pcss";
@import "./views/settings/_SecureBackupPanel.pcss";
@import "./views/settings/_SetIdServer.pcss";
@import "./views/settings/_SetIntegrationManager.pcss";
@import "./views/settings/_SettingsFieldset.pcss";
@import "./views/settings/_SettingsHeader.pcss";
Expand Down
23 changes: 0 additions & 23 deletions res/css/views/settings/_SetIdServer.pcss

This file was deleted.

Loading
Loading