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.

Sign up for GitHub

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

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

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
3a0cffe
Use EditInPlace for identity server picker.
Half-Shot Feb 17, 2025
02ffc77
Exclude picker from default dialog button styles.
Half-Shot Feb 17, 2025
35700c6
Remove unused import.
Half-Shot Feb 17, 2025
574b6db
Update test
Half-Shot Feb 17, 2025
3f39189
Remove unused css
Half-Shot Feb 17, 2025
48e3445
Update test
Half-Shot Feb 17, 2025
a2c411d
drop only
Half-Shot Feb 17, 2025
29647c9
Add a test for setting an ID server.
Half-Shot Feb 17, 2025
05b7812
Merge branch 'develop' into hs/identity-server-picker-modernize
Half-Shot Feb 17, 2025
24a0453
Add a unit test for SetIdServer
Half-Shot Feb 17, 2025
73d382d
fix tests
Half-Shot Feb 17, 2025
397b792
Reformat mx_Dialog button :not list to use a more readable selector.
Half-Shot Feb 18, 2025
5ed4e10
Reformat other :not sections
Half-Shot Feb 18, 2025
84e698e
forgot a comma
Half-Shot Feb 18, 2025
1fa1048
We're in 2025 now.
Half-Shot Feb 18, 2025
80e5cfa
Update copyright + use class methods.
Half-Shot Feb 18, 2025
0c4655c
Merge branch 'develop' into hs/identity-server-picker-modernize
Half-Shot Feb 21, 2025
ab4fdeb
Merge branch 'develop' into hs/identity-server-picker-modernize
Half-Shot Feb 21, 2025
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
View file
Open in desktop
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
79 changes: 51 additions & 28 deletions res/css/_common.pcss
View file
Open in desktop
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,43 @@ 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 +665,41 @@ 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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,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
View file
Open in desktop

This file was deleted.

76 changes: 28 additions & 48 deletions src/components/views/settings/SetIdServer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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";
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 => {
Expand All @@ -148,7 +131,7 @@ export default class SetIdServer extends React.Component<IProps, IState> {
busy: false,
error: undefined,
currentClientIdServer: fullUrl,
idServer: "",
idServer: fullUrl,
});
};

Expand All @@ -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;
}

Expand Down Expand Up @@ -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: (
Expand Down Expand Up @@ -393,28 +376,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.setState((s) => ({ idServer: s.currentClientIdServer ?? "" }))}
onChange={this.onIdentityServerChanged}
onClearServerErrors={() => this.setState({ error: undefined })}
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>
);
}
Expand Down
1 change: 1 addition & 0 deletions src/i18n/strings/en_EN.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1243,6 +1243,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.",
Expand Down
Loading
Loading