mirrors/element-web
1
0
Fork 0
mirror of https://github.com/vector-im/element-web.git synced 2026-01-06 09:01:09 +01:00
Code Issues Projects Releases Wiki Activity

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

Browse Source 
* Use EditInPlace for identity server picker.

* Exclude picker from default dialog button styles.

* Remove unused import.

* Update test

* Remove unused css

* Update test

* drop only

* Add a test for setting an ID server.

* Add a unit test for SetIdServer

* fix tests

* Reformat mx_Dialog button :not list to use a more readable selector.

* Reformat other :not sections

* forgot a comma

* We're in 2025 now.

* Update copyright + use class methods.
This commit is contained in:
Will Hunt 2025-02-21 13:18:14 +00:00 committed by GitHub
parent 2e1798edc4
commit 22943ee06a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 309 additions and 129 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

53
playwright/e2e/settings/security-user-settings-tab.spec.ts
View File

@ -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(
@ -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");

84
res/css/_common.pcss
View File

@ -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(
.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;
@ -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 {
@ -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;

1
res/css/_components.pcss
View File

@ -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";

23
res/css/views/settings/_SetIdServer.pcss
View File

@ -1,23 +0,0 @@
/*
Copyright 2024 New Vector Ltd.
Copyright 2019-2023 The Matrix.org Foundation C.I.C.
SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
Please see LICENSE files in the repository root for full details.
*/
.mx_SetIdServer {
display: flex;
flex-direction: column;
align-items: flex-start;
gap: $spacing-8;
.mx_Field {
width: 100%;
margin: 0;
}
}
.mx_SetIdServer_tooltip {
max-width: var(--SettingsTab_tooltip-max-width);
}

85
src/components/views/settings/SetIdServer.tsx
View File

@ -1,5 +1,5 @@
/*
Copyright 2024 New Vector Ltd.
Copyright 2024-2025 New Vector Ltd.
Copyright 2019-2021 The Matrix.org Foundation C.I.C.
SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
@ -9,6 +9,7 @@ Please see LICENSE files in the repository root for full details.
import React, { type ReactNode } from "react";
import { logger } from "matrix-js-sdk/src/logger";
import { type IThreepid } from "matrix-js-sdk/src/matrix";
import { EditInPlace, ErrorMessage } from "@vector-im/compound-web";
import { _t } from "../../../languageHandler";
import { MatrixClientPeg } from "../../../MatrixClientPeg";
@ -22,7 +23,6 @@ import { timeout } from "../../../utils/promise";
import { type ActionPayload } from "../../../dispatcher/payloads";
import InlineSpinner from "../elements/InlineSpinner";
import AccessibleButton from "../elements/AccessibleButton";
import Field from "../elements/Field";
import QuestionDialog from "../dialogs/QuestionDialog";
import SettingsFieldset from "./SettingsFieldset";
import { SettingsSubsectionText } from "./shared/SettingsSubsection";
@ -86,10 +86,12 @@ export default class SetIdServer extends React.Component<IProps, IState> {
defaultIdServer = abbreviateUrl(getDefaultIdentityServerUrl());
}
const currentClientIdServer = MatrixClientPeg.safeGet().getIdentityServerUrl();
this.state = {
defaultIdServer,
currentClientIdServer: MatrixClientPeg.safeGet().getIdentityServerUrl(),
idServer: "",
currentClientIdServer,
idServer: currentClientIdServer ?? "",
busy: false,
disconnectBusy: false,
checking: false,
@ -117,26 +119,7 @@ export default class SetIdServer extends React.Component<IProps, IState> {
private onIdentityServerChanged = (ev: React.ChangeEvent<HTMLInputElement>): void => {
const u = ev.target.value;
this.setState({ idServer: u });
};
private getTooltip = (): JSX.Element | undefined => {
if (this.state.checking) {
return (
<div>
<InlineSpinner />
{_t("identity_server|checking")}
</div>
);
} else if (this.state.error) {
return <strong className="warning">{this.state.error}</strong>;
} else {
return undefined;
}
};
private idServerChangeEnabled = (): boolean => {
return !!this.state.idServer && !this.state.busy;
this.setState({ idServer: u, error: undefined });
};
private saveIdServer = (fullUrl: string): void => {
@ -148,7 +131,7 @@ export default class SetIdServer extends React.Component<IProps, IState> {
busy: false,
error: undefined,
currentClientIdServer: fullUrl,
idServer: "",
idServer: fullUrl,
});
};
@ -175,7 +158,7 @@ export default class SetIdServer extends React.Component<IProps, IState> {
// Double check that the identity server even has terms of service.
const hasTerms = await doesIdentityServerHaveTerms(MatrixClientPeg.safeGet(), fullUrl);
if (!hasTerms) {
const [confirmed] = await this.showNoTermsWarning(fullUrl);
const [confirmed] = await this.showNoTermsWarning();
save = !!confirmed;
}
@ -213,7 +196,7 @@ export default class SetIdServer extends React.Component<IProps, IState> {
});
};
private showNoTermsWarning(fullUrl: string): Promise<[ok?: boolean]> {
private showNoTermsWarning(): Promise<[ok?: boolean]> {
const { finished } = Modal.createDialog(QuestionDialog, {
title: _t("terms|identity_server_no_terms_title"),
description: (
@ -347,6 +330,9 @@ export default class SetIdServer extends React.Component<IProps, IState> {
});
};
private onInputCancel = (): void => this.setState((s) => ({ idServer: s.currentClientIdServer ?? "" }));
private onClearServerErrors = (): void => this.setState({ error: undefined });
public render(): React.ReactNode {
const idServerUrl = this.state.currentClientIdServer;
let sectionTitle;
@ -356,13 +342,13 @@ export default class SetIdServer extends React.Component<IProps, IState> {
bodyText = _t(
"identity_server|description_connected",
{},
{ server: (sub) => <strong>{abbreviateUrl(idServerUrl)}</strong> },
{ server: () => <strong>{abbreviateUrl(idServerUrl)}</strong> },
);
if (this.props.missingTerms) {
bodyText = _t(
"identity_server|change_server_prompt",
{},
{ server: (sub) => <strong>{abbreviateUrl(idServerUrl)}</strong> },
{ server: () => <strong>{abbreviateUrl(idServerUrl)}</strong> },
);
}
} else {
@ -393,28 +379,25 @@ export default class SetIdServer extends React.Component<IProps, IState> {
return (
<SettingsFieldset legend={sectionTitle} description={bodyText}>
<form className="mx_SetIdServer" onSubmit={this.checkIdServer}>
<Field
label={_t("identity_server|url_field_label")}
type="text"
autoComplete="off"
placeholder={this.state.defaultIdServer}
value={this.state.idServer}
onChange={this.onIdentityServerChanged}
tooltipContent={this.getTooltip()}
tooltipClassName="mx_SetIdServer_tooltip"
disabled={this.state.busy}
forceValidity={this.state.error ? false : undefined}
/>
<AccessibleButton
kind="primary_sm"
onClick={this.checkIdServer}
disabled={!this.idServerChangeEnabled()}
>
{_t("action|change")}
</AccessibleButton>
{discoSection}
</form>
<EditInPlace
className="mx_IdentityServerPicker"
cancelButtonLabel={_t("action|reset")}
disabled={!!this.state.busy}
label={_t("identity_server|url_field_label")}
onCancel={this.onInputCancel}
onChange={this.onIdentityServerChanged}
onClearServerErrors={this.onClearServerErrors}
onSave={this.checkIdServer}
placeholder={this.state.defaultIdServer}
saveButtonLabel={_t("action|change")}
savedLabel={this.state.error ? undefined : _t("identity_server|changed")}
savingLabel={_t("identity_server|checking")}
serverInvalid={!!this.state.error}
value={this.state.idServer}
>
{this.state.error && <ErrorMessage>{this.state.error}</ErrorMessage>}
</EditInPlace>
{discoSection}
</SettingsFieldset>
);
}

1
src/i18n/strings/en_EN.json
View File

@ -1245,6 +1245,7 @@
"change": "Change identity server",
"change_prompt": "Disconnect from the identity server <current /> and connect to <new /> instead?",
"change_server_prompt": "If you don't want to use <server /> to discover and be discoverable by existing contacts you know, enter another identity server below.",
"changed": "Your identity server has been changed",
"checking": "Checking server",
"description_connected": "You are currently using <server></server> to discover and be discoverable by existing contacts you know. You can change your identity server below.",
"description_disconnected": "You are not currently using an identity server. To discover and be discoverable by existing contacts you know, add one below.",

101
test/unit-tests/components/views/settings/SetIdServer-test.tsx Normal file
View File

@ -0,0 +1,101 @@
/*
Copyright 2025 New Vector Ltd.
SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
Please see LICENSE files in the repository root for full details.
*/
import React from "react";
import { render, waitFor } from "jest-matrix-react";
import userEvent from "@testing-library/user-event";
import fetchMock from "fetch-mock-jest";
import SetIdServer from "../../../../../src/components/views/settings/SetIdServer";
import MatrixClientContext from "../../../../../src/contexts/MatrixClientContext";
import { getMockClientWithEventEmitter, mockClientMethodsUser, mockClientMethodsServer } from "../../../../test-utils";
describe("<SetIdServer />", () => {
const userId = "@alice:server.org";
const mockClient = getMockClientWithEventEmitter({
...mockClientMethodsUser(userId),
...mockClientMethodsServer(),
getOpenIdToken: jest.fn().mockResolvedValue("a_token"),
getTerms: jest.fn(),
setAccountData: jest.fn(),
});
const getComponent = () => (
<MatrixClientContext.Provider value={mockClient}>
<SetIdServer missingTerms={false} />
</MatrixClientContext.Provider>
);
afterAll(() => {
jest.resetAllMocks();
});
it("renders expected fields", () => {
const { asFragment } = render(getComponent());
expect(asFragment()).toMatchSnapshot();
});
it("should allow setting an identity server", async () => {
const { getByLabelText, getByRole } = render(getComponent());
fetchMock.get("https://identity.example.org/_matrix/identity/v2", {
body: {},
});
fetchMock.get("https://identity.example.org/_matrix/identity/v2/account", {
body: { user_id: userId },
});
fetchMock.post("https://identity.example.org/_matrix/identity/v2/account/register", {
body: { token: "foobar" },
});
const identServerField = getByLabelText("Enter a new identity server");
await userEvent.type(identServerField, "https://identity.example.org");
await userEvent.click(getByRole("button", { name: "Change" }));
await userEvent.click(getByRole("button", { name: "Continue" }));
});
it("should clear input on cancel", async () => {
const { getByLabelText, getByRole } = render(getComponent());
const identServerField = getByLabelText("Enter a new identity server");
await userEvent.type(identServerField, "https://identity.example.org");
await userEvent.click(getByRole("button", { name: "Reset" }));
expect((identServerField as HTMLInputElement).value).toEqual("");
});
it("should show error when an error occurs", async () => {
const { getByLabelText, getByRole, getByText } = render(getComponent());
fetchMock.get("https://invalid.example.org/_matrix/identity/v2", {
body: {},
status: 404,
});
fetchMock.get("https://invalid.example.org/_matrix/identity/v2/account", {
body: {},
status: 404,
});
fetchMock.post("https://invalid.example.org/_matrix/identity/v2/account/register", {
body: {},
status: 404,
});
const identServerField = getByLabelText("Enter a new identity server");
await userEvent.type(identServerField, "https://invalid.example.org");
await userEvent.click(getByRole("button", { name: "Change" }));
await waitFor(
() => {
expect(getByText("Not a valid identity server (status code 404)")).toBeVisible();
},
{ timeout: 3000 },
);
// Check the error vanishes when the input is edited.
await userEvent.type(identServerField, "https://identity2.example.org");
expect(() => getByText("Not a valid identity server (status code 404)")).toThrow();
});
});

54
test/unit-tests/components/views/settings/__snapshots__/SetIdServer-test.tsx.snap Normal file
View File

@ -0,0 +1,54 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`<SetIdServer /> renders expected fields 1`] = `
<DocumentFragment>
<fieldset
class="mx_SettingsFieldset"
>
<legend
class="mx_SettingsFieldset_legend"
>
Identity server
</legend>
<div
class="mx_SettingsFieldset_description"
>
<div
class="mx_SettingsSubsection_text"
>
You are not currently using an identity server. To discover and be discoverable by existing contacts you know, add one below.
</div>
</div>
<div
class="mx_SettingsFieldset_content"
>
<form
class="_root_ssths_24 mx_IdentityServerPicker"
>
<div
class="_field_ssths_34"
>
<label
class="_label_ssths_67"
for="radix-:r0:"
>
Enter a new identity server
</label>
<div
class="_controls_1h4nb_17"
>
<input
class="_control_9gon8_18"
id="radix-:r0:"
name="input"
placeholder=""
title=""
value=""
/>
</div>
</div>
</form>
</div>
</fieldset>
</DocumentFragment>
`;

36
test/unit-tests/components/views/settings/tabs/user/__snapshots__/SecurityUserSettingsTab-test.tsx.snap
View File

@ -438,33 +438,29 @@ exports[`<SecurityUserSettingsTab /> renders security section 1`] = `
class="mx_SettingsFieldset_content"
>
<form
class="mx_SetIdServer"
class="_root_ssths_24 mx_IdentityServerPicker"
>
<div
class="mx_Field mx_Field_input"
class="_field_ssths_34"
>
<input
autocomplete="off"
id="mx_Field_1"
label="Enter a new identity server"
placeholder=""
type="text"
value=""
/>
<label
for="mx_Field_1"
class="_label_ssths_67"
for="radix-:r0:"
>
Enter a new identity server
</label>
</div>
<div
aria-disabled="true"
class="mx_AccessibleButton mx_AccessibleButton_hasKind mx_AccessibleButton_kind_primary_sm mx_AccessibleButton_disabled"
disabled=""
role="button"
tabindex="0"
>
Change
<div
class="_controls_1h4nb_17"
>
<input
class="_control_9gon8_18"
id="radix-:r0:"
name="input"
placeholder=""
title=""
value=""
/>
</div>
</div>
</form>
</div>