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

Commit

Permalink
Move the active tab in user settings to the dialog title (#12481)
Browse files Browse the repository at this point in the history
* Convert tabbedview to functional component

The 'Tab' is still a class, so now it's a functional component that
has a supporting class, which is maybe a bit... jarring, but I think
is actually perfectly logical.

* put comment back

* Fix bad tab ID behaviour

* Make TabbedView a controlled component

This does mean the logic of keeping what tab is active is now in each
container component, but for a functional component, this is a single
line. It makes TabbedView simpler and the container components always
know exactly what tab is being displayed rather than having to effectively
keep the state separately themselves if they wanted it.

Based on #12478

* Move the active tab in user settings to the dialog title

Separated by a colon, as per the new design.

* Update snapshots

* Update a playwright test

* Fix more tests / snapshots

* Attempt to test all the cases of titleForTabID

* More tests
  • Loading branch information
dbkr committed May 7, 2024
1 parent f8e040a commit 18ef971
Show file tree
Hide file tree
Showing 29 changed files with 186 additions and 85 deletions.
2 changes: 0 additions & 2 deletions playwright/e2e/settings/appearance-user-settings-tab.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ test.describe("Appearance user settings tab", () => {
test("should be rendered properly", async ({ page, user, app }) => {
const tab = await app.settings.openUserSettings("Appearance");

await expect(tab.getByRole("heading", { name: "Customise your appearance" })).toBeVisible();

// Click "Show advanced" link button
await tab.getByRole("button", { name: "Show advanced" }).click();

Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion src/components/views/dialogs/BaseDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ interface IProps {
"top"?: React.ReactNode;

// Title for the dialog.
"title"?: JSX.Element | string;
"title"?: React.ReactNode;
// Specific aria label to use, if not provided will set aria-labelledBy to mx_Dialog_title
"aria-label"?: string;

Expand Down
2 changes: 1 addition & 1 deletion src/components/views/dialogs/InviteDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1536,7 +1536,7 @@ export default class InviteDialog extends React.PureComponent<Props, IInviteDial
);
dialogContent = (
<React.Fragment>
<TabbedView
<TabbedView<TabId>
tabs={tabs}
activeTabId={this.state.currentTabId}
tabLocation={TabLocation.TOP}
Expand Down
34 changes: 33 additions & 1 deletion src/components/views/dialogs/UserSettingsDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,38 @@ interface IProps {
onFinished(): void;
}

function titleForTabID(tabId: UserTab): React.ReactNode {
const subs = {
strong: (sub: string) => <strong>{sub}</strong>,
};
switch (tabId) {
case UserTab.General:
return _t("settings|general|dialog_title", undefined, subs);
case UserTab.SessionManager:
return _t("settings|sessions|dialog_title", undefined, subs);
case UserTab.Appearance:
return _t("settings|appearance|dialog_title", undefined, subs);
case UserTab.Notifications:
return _t("settings|notifications|dialog_title", undefined, subs);
case UserTab.Preferences:
return _t("settings|preferences|dialog_title", undefined, subs);
case UserTab.Keyboard:
return _t("settings|keyboard|dialog_title", undefined, subs);
case UserTab.Sidebar:
return _t("settings|sidebar|dialog_title", undefined, subs);
case UserTab.Voice:
return _t("settings|voip|dialog_title", undefined, subs);
case UserTab.Security:
return _t("settings|security|dialog_title", undefined, subs);
case UserTab.Labs:
return _t("settings|labs|dialog_title", undefined, subs);
case UserTab.Mjolnir:
return _t("settings|labs_mjolnir|dialog_title", undefined, subs);
case UserTab.Help:
return _t("setting|help_about|dialog_title", undefined, subs);
}
}

export default function UserSettingsDialog(props: IProps): JSX.Element {
const voipEnabled = useSettingValue<boolean>(UIFeature.Voip);
const mjolnirEnabled = useSettingValue<boolean>("feature_mjolnir");
Expand Down Expand Up @@ -184,7 +216,7 @@ export default function UserSettingsDialog(props: IProps): JSX.Element {
className="mx_UserSettingsDialog"
hasCancel={true}
onFinished={props.onFinished}
title={_t("common|settings")}
title={titleForTabID(activeTabId)}
>
<div className="mx_SettingsDialog_content">
<TabbedView
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ export default function NotificationSettings2(): JSX.Element {
)}
</SettingsBanner>
)}
<SettingsSection heading={_t("notifications|enable_prompt_toast_title")}>
<SettingsSection>
<div className="mx_SettingsSubsection_content mx_NotificationSettings2_flags">
<LabelledToggleSwitch
label={_t("settings|notifications|enable_notifications_account")}
Expand Down
25 changes: 17 additions & 8 deletions src/components/views/settings/shared/SettingsSection.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,25 @@ import React, { HTMLAttributes } from "react";
import Heading from "../../typography/Heading";

export interface SettingsSectionProps extends HTMLAttributes<HTMLDivElement> {
heading: string | React.ReactNode;
heading?: string | React.ReactNode;
children?: React.ReactNode;
}

function renderHeading(heading: string | React.ReactNode | undefined): React.ReactNode | undefined {
switch (typeof heading) {
case "string":
return (
<Heading as="h2" size="3">
{heading}
</Heading>
);
case "undefined":
return undefined;
default:
return heading;
}
}

/**
* A section of settings content
* A SettingsTab may contain one or more SettingsSections
Expand All @@ -43,13 +58,7 @@ export interface SettingsSectionProps extends HTMLAttributes<HTMLDivElement> {
*/
export const SettingsSection: React.FC<SettingsSectionProps> = ({ className, heading, children, ...rest }) => (
<div {...rest} className={classnames("mx_SettingsSection", className)}>
{typeof heading === "string" ? (
<Heading as="h2" size="3">
{heading}
</Heading>
) : (
<>{heading}</>
)}
{renderHeading(heading)}
<div className="mx_SettingsSection_subSections">{children}</div>
</div>
);
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import ThemeChoicePanel from "../../ThemeChoicePanel";
import ImageSizePanel from "../../ImageSizePanel";
import SettingsTab from "../SettingsTab";
import { SettingsSection } from "../../shared/SettingsSection";
import SettingsSubsection, { SettingsSubsectionText } from "../../shared/SettingsSubsection";
import SettingsSubsection from "../../shared/SettingsSubsection";
import MatrixClientContext from "../../../../../contexts/MatrixClientContext";

interface IProps {}
Expand Down Expand Up @@ -152,12 +152,9 @@ export default class AppearanceUserSettingsTab extends React.Component<IProps, I
}

public render(): React.ReactNode {
const brand = SdkConfig.get().brand;

return (
<SettingsTab data-testid="mx_AppearanceUserSettingsTab">
<SettingsSection heading={_t("settings|appearance|heading")}>
<SettingsSubsectionText>{_t("settings|appearance|subheading", { brand })}</SettingsSubsectionText>
<SettingsSection>
<ThemeChoicePanel />
<LayoutSwitcher
userId={this.state.userId}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -560,7 +560,7 @@ export default class GeneralUserSettingsTab extends React.Component<IProps, ISta

return (
<SettingsTab data-testid="mx_GeneralUserSettingsTab">
<SettingsSection heading={_t("common|general")}>
<SettingsSection>
<ProfileSettings />
{this.renderAccountSection()}
{this.renderLanguageSection()}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ export default class HelpUserSettingsTab extends React.Component<IProps, IState>

return (
<SettingsTab>
<SettingsSection heading={_t("setting|help_about|title")}>
<SettingsSection>
{bugReportingSection}
<SettingsSubsection heading={_t("common|faq")} description={faqText} />
<SettingsSubsection heading={_t("setting|help_about|versions")}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ const KeyboardShortcutSection: React.FC<IKeyboardShortcutSectionProps> = ({ cate
const KeyboardUserSettingsTab: React.FC = () => {
return (
<SettingsTab>
<SettingsSection heading={_t("settings|keyboard|title")}>
<SettingsSection>
{visibleCategories.map(([categoryName, category]) => {
return (
<KeyboardShortcutSection key={categoryName} categoryName={categoryName} category={category} />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ export default class MjolnirUserSettingsTab extends React.Component<{}, IState>

return (
<SettingsTab>
<SettingsSection heading={_t("labs_mjolnir|title")}>
<SettingsSection>
<SettingsSubsectionText>
<strong className="warning">{_t("labs_mjolnir|advanced_warning")}</strong>
<p>{_t("labs_mjolnir|explainer_1", { brand }, { code: (s) => <code>{s}</code> })}</p>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ limitations under the License.

import React from "react";

import { _t } from "../../../../../languageHandler";
import { Features } from "../../../../../settings/Settings";
import SettingsStore from "../../../../../settings/SettingsStore";
import Notifications from "../../Notifications";
Expand All @@ -33,7 +32,7 @@ export default class NotificationUserSettingsTab extends React.Component {
{newNotificationSettingsEnabled ? (
<NotificationSettings2 />
) : (
<SettingsSection heading={_t("notifications|enable_prompt_toast_title")}>
<SettingsSection>
<Notifications />
</SettingsSection>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ export default class PreferencesUserSettingsTab extends React.Component<IProps,

return (
<SettingsTab data-testid="mx_PreferencesUserSettingsTab">
<SettingsSection heading={_t("common|preferences")}>
<SettingsSection>
{roomListSettings.length > 0 && (
<SettingsSubsection heading={_t("settings|preferences|room_list_heading")}>
{this.renderGroup(roomListSettings)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ const SessionManagerTab: React.FC = () => {

return (
<SettingsTab>
<SettingsSection heading={_t("settings|sessions|title")}>
<SettingsSection>
<LoginWithQRSection
onShowQr={onShowQrClicked}
versions={clientVersions}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ const SidebarUserSettingsTab: React.FC = () => {

return (
<SettingsTab>
<SettingsSection heading={_t("settings|sidebar|title")}>
<SettingsSection>
<SettingsSubsection
heading={_t("settings|sidebar|metaspaces_subsection")}
description={_t("settings|sidebar|spaces_explainer")}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ export default class VoiceUserSettingsTab extends React.Component<{}, IState> {

return (
<SettingsTab>
<SettingsSection heading={_t("settings|voip|title")}>
<SettingsSection>
{requestButton}
<SettingsSubsection heading={_t("settings|voip|voice_section")} stretchContent>
{speakerDropdown}
Expand Down
18 changes: 16 additions & 2 deletions src/i18n/strings/en_EN.json
Original file line number Diff line number Diff line change
Expand Up @@ -2395,6 +2395,7 @@
"brand_version": "%(brand)s version:",
"clear_cache_reload": "Clear cache and reload",
"crypto_version": "Crypto version:",
"dialog_title": "<strong>Settings:</strong> Help & About",
"help_link": "For help with using %(brand)s, click <a>here</a>.",
"homeserver": "Homeserver is <code>%(homeserverUrl)s</code>",
"identity_server": "Identity server is <code>%(identityServerUrl)s</code>",
Expand All @@ -2417,15 +2418,14 @@
"custom_theme_invalid": "Invalid theme schema.",
"custom_theme_success": "Theme added!",
"custom_theme_url": "Custom theme URL",
"dialog_title": "<strong>Settings:</strong> Appearance",
"font_size": "Font size",
"font_size_default": "%(fontSize)s (default)",
"heading": "Customise your appearance",
"image_size_default": "Default",
"image_size_large": "Large",
"layout_bubbles": "Message bubbles",
"layout_irc": "IRC (Experimental)",
"match_system_theme": "Match system theme",
"subheading": "Appearance Settings only affect this %(brand)s session.",
"timeline_image_size": "Image size in the timeline",
"use_high_contrast": "Use high contrast"
},
Expand Down Expand Up @@ -2467,6 +2467,7 @@
"deactivate_confirm_erase_label": "Hide my messages from new joiners",
"deactivate_section": "Deactivate Account",
"deactivate_warning": "Deactivating your account is a permanent action — be careful!",
"dialog_title": "<strong>Settings:</strong> General",
"discovery_email_empty": "Discovery options will appear once you have added an email above.",
"discovery_email_verification_instructions": "Verify the link in your inbox",
"discovery_msisdn_empty": "Discovery options will appear once you have added a phone number above.",
Expand Down Expand Up @@ -2574,12 +2575,20 @@
"phrase_strong_enough": "Great! This passphrase looks strong enough"
},
"keyboard": {
"dialog_title": "<strong>Settings:</strong> Keyboard",
"title": "Keyboard"
},
"labs": {
"dialog_title": "<strong>Settings:</strong> Labs"
},
"labs_mjolnir": {
"dialog_title": "<strong>Settings:</strong> Ignored Users"
},
"notifications": {
"default_setting_description": "This setting will be applied by default to all your rooms.",
"default_setting_section": "I want to be notified for (Default Setting)",
"desktop_notification_message_preview": "Show message preview in desktop notification",
"dialog_title": "<strong>Settings:</strong> Notifications",
"email_description": "Receive an email summary of missed notifications",
"email_section": "Email summary",
"email_select": "Select which emails you want to send summaries to. Manage your emails in <button>General</button>.",
Expand Down Expand Up @@ -2638,6 +2647,7 @@
"code_blocks_heading": "Code blocks",
"compact_modern": "Use a more compact 'Modern' layout",
"composer_heading": "Composer",
"dialog_title": "<strong>Settings:</strong> Preferences",
"enable_hardware_acceleration": "Enable hardware acceleration",
"enable_tray_icon": "Show tray icon and minimise window to it on close",
"keyboard_heading": "Keyboard shortcuts",
Expand Down Expand Up @@ -2687,6 +2697,7 @@
"dehydrated_device_enabled": "Offline device enabled",
"delete_backup": "Delete Backup",
"delete_backup_confirm_description": "Are you sure? You will lose your encrypted messages if your keys are not backed up properly.",
"dialog_title": "<strong>Settings:</strong> Security & Privacy",
"e2ee_default_disabled_warning": "Your server admin has disabled end-to-end encryption by default in private rooms & Direct Messages.",
"enable_message_search": "Enable message search in encrypted rooms",
"encryption_individual_verification_mode": "Individually verify each session used by a user to mark it as trusted, not trusting cross-signed devices.",
Expand Down Expand Up @@ -2766,6 +2777,7 @@
"device_unverified_description_current": "Verify your current session for enhanced secure messaging.",
"device_verified_description": "This session is ready for secure messaging.",
"device_verified_description_current": "Your current session is ready for secure messaging.",
"dialog_title": "<strong>Settings:</strong> Sessions",
"error_pusher_state": "Failed to set pusher state",
"error_set_name": "Failed to set session name",
"filter_all": "All",
Expand Down Expand Up @@ -2849,6 +2861,7 @@
"show_typing_notifications": "Show typing notifications",
"showbold": "Show all activity in the room list (dots or number of unread messages)",
"sidebar": {
"dialog_title": "<strong>Settings:</strong> Sidebar",
"metaspaces_favourites_description": "Group all your favourite rooms and people in one place.",
"metaspaces_home_all_rooms": "Show all rooms",
"metaspaces_home_all_rooms_description": "Show all your rooms in Home, even if they're in a space.",
Expand Down Expand Up @@ -2878,6 +2891,7 @@
"audio_output_empty": "No Audio Outputs detected",
"auto_gain_control": "Automatic gain control",
"connection_section": "Connection",
"dialog_title": "<strong>Settings:</strong> Voice & Video",
"echo_cancellation": "Echo cancellation",
"enable_fallback_ice_server": "Allow fallback call assist server (%(server)s)",
"enable_fallback_ice_server_description": "Only applies if your homeserver does not offer one. Your IP address would be shared during a call.",
Expand Down
Loading

0 comments on commit 18ef971

Please sign in to comment.