From 9211da20f42570654ddefb1d59a3e5ce12904fc1 Mon Sep 17 00:00:00 2001
From: Kerry <kerrya@element.io>
Date: Wed, 24 May 2023 11:36:09 +1200
Subject: [PATCH] Use semantic headings in user settings - Notifications
(#10948)
* use semantic headings in user notif settings
* unset margin for subsection content when no heading
* remove debug
---
.../settings/shared/_SettingsSubsection.pcss | 4 ++
res/css/views/settings/_Notifications.pcss | 56 +++++++++----------
.../views/settings/Notifications.tsx | 38 ++++++-------
.../settings/shared/SettingsSubsection.tsx | 3 +-
.../tabs/user/NotificationUserSettingsTab.tsx | 11 ++--
.../__snapshots__/Notifications-test.tsx.snap | 54 +++++++++---------
6 files changed, 81 insertions(+), 85 deletions(-)
diff --git a/res/css/components/views/settings/shared/_SettingsSubsection.pcss b/res/css/components/views/settings/shared/_SettingsSubsection.pcss
index 6d4b69e292..2d8894150f 100644
--- a/res/css/components/views/settings/shared/_SettingsSubsection.pcss
+++ b/res/css/components/views/settings/shared/_SettingsSubsection.pcss
@@ -52,4 +52,8 @@ limitations under the License.
&.mx_SettingsSubsection_contentStretch {
justify-items: stretch;
}
+
+ &.mx_SettingsSubsection_noHeading {
+ margin-top: 0;
+ }
}
diff --git a/res/css/views/settings/_Notifications.pcss b/res/css/views/settings/_Notifications.pcss
index 635627e0b0..c35181ddca 100644
--- a/res/css/views/settings/_Notifications.pcss
+++ b/res/css/views/settings/_Notifications.pcss
@@ -20,7 +20,6 @@ limitations under the License.
grid-template-columns: auto repeat(3, 62px);
place-items: center center;
grid-gap: 8px;
- margin-top: $spacing-40;
/* Override StyledRadioButton default styles */
.mx_StyledRadioButton {
@@ -34,6 +33,11 @@ limitations under the License.
display: none;
}
}
+
+ // left align section heading
+ .mx_SettingsSubsectionHeading {
+ justify-self: start;
+ }
}
.mx_UserNotifSettings_gridRowContainer {
@@ -51,10 +55,6 @@ limitations under the License.
/* force it inline using float */
float: left;
}
-.mx_UserNotifSettings_gridRowHeading {
- font-size: $font-18px;
- font-weight: var(--font-semi-bold);
-}
.mx_UserNotifSettings_gridColumnLabel {
color: $secondary-content;
@@ -70,39 +70,35 @@ limitations under the License.
margin-top: -$spacing-4;
}
-.mx_UserNotifSettings {
- color: $primary-content; /* override from default settings page styles */
+.mx_UserNotifSettings_floatingSection {
+ margin-top: 40px;
- .mx_UserNotifSettings_floatingSection {
- margin-top: 40px;
-
- & > div:first-child {
- /* section header */
- font-size: $font-18px;
- font-weight: var(--font-semi-bold);
- }
-
- > table {
- border-collapse: collapse;
- border-spacing: 0;
- margin-top: 8px;
-
- tr > td:first-child {
- /* Just for a bit of spacing */
- padding-right: 8px;
- }
- }
+ & > div:first-child {
+ /* section header */
+ font-size: $font-18px;
+ font-weight: var(--font-semi-bold);
}
- .mx_UserNotifSettings_clearNotifsButton {
+ > table {
+ border-collapse: collapse;
+ border-spacing: 0;
margin-top: 8px;
- }
- .mx_TagComposer {
- margin-top: 35px; /* lots of distance from the last line of the table */
+ tr > td:first-child {
+ /* Just for a bit of spacing */
+ padding-right: 8px;
+ }
}
}
+.mx_UserNotifSettings_clearNotifsButton {
+ margin-top: 8px;
+}
+
+.mx_TagComposer {
+ margin-top: 35px; /* lots of distance from the last line of the table */
+}
+
.mx_AccessibleButton.mx_NotificationSound_browse {
margin-right: 10px;
}
diff --git a/src/components/views/settings/Notifications.tsx b/src/components/views/settings/Notifications.tsx
index 6c6c38c0b4..18d5c34c10 100644
--- a/src/components/views/settings/Notifications.tsx
+++ b/src/components/views/settings/Notifications.tsx
@@ -48,6 +48,8 @@ import {
updatePushRuleActions,
} from "../../../utils/pushRules/updatePushRuleActions";
import { Caption } from "../typography/Caption";
+import { SettingsSubsectionHeading } from "./shared/SettingsSubsectionHeading";
+import SettingsSubsection from "./shared/SettingsSubsection";
// TODO: this "view" component still has far too much application logic in it,
// which should be factored out to other files.
@@ -649,16 +651,14 @@ export default class Notifications extends React.PureComponent<IProps, IState> {
private renderTopSection(): JSX.Element {
const masterSwitch = (
- <>
- <LabelledToggleSwitch
- data-testid="notif-master-switch"
- value={!this.isInhibited}
- label={_t("Enable notifications for this account")}
- caption={_t("Turn off to disable notifications on all your devices and sessions")}
- onChange={this.onMasterRuleChanged}
- disabled={this.state.phase === Phase.Persisting}
- />
- </>
+ <LabelledToggleSwitch
+ data-testid="notif-master-switch"
+ value={!this.isInhibited}
+ label={_t("Enable notifications for this account")}
+ caption={_t("Turn off to disable notifications on all your devices and sessions")}
+ onChange={this.onMasterRuleChanged}
+ disabled={this.state.phase === Phase.Persisting}
+ />
);
// If all the rules are inhibited, don't show anything.
@@ -680,7 +680,7 @@ export default class Notifications extends React.PureComponent<IProps, IState> {
));
return (
- <>
+ <SettingsSubsection>
{masterSwitch}
<LabelledToggleSwitch
@@ -718,7 +718,7 @@ export default class Notifications extends React.PureComponent<IProps, IState> {
)}
{emailSwitches}
- </>
+ </SettingsSubsection>
);
}
@@ -814,7 +814,7 @@ export default class Notifications extends React.PureComponent<IProps, IState> {
</fieldset>
));
- let sectionName: TranslatedString;
+ let sectionName: string;
switch (category) {
case RuleClass.VectorGlobal:
sectionName = _t("Global");
@@ -830,11 +830,9 @@ export default class Notifications extends React.PureComponent<IProps, IState> {
}
return (
- <>
+ <div>
<div data-testid={`notif-section-${category}`} className="mx_UserNotifSettings_grid">
- <span className="mx_UserNotifSettings_gridRowLabel mx_UserNotifSettings_gridRowHeading">
- {sectionName}
- </span>
+ <SettingsSubsectionHeading heading={sectionName} />
<span className="mx_UserNotifSettings_gridColumnLabel">{VectorStateToLabel[VectorState.Off]}</span>
<span className="mx_UserNotifSettings_gridColumnLabel">{VectorStateToLabel[VectorState.On]}</span>
<span className="mx_UserNotifSettings_gridColumnLabel">{VectorStateToLabel[VectorState.Loud]}</span>
@@ -842,7 +840,7 @@ export default class Notifications extends React.PureComponent<IProps, IState> {
</div>
{clearNotifsButton}
{keywordComposer}
- </>
+ </div>
);
}
@@ -877,13 +875,13 @@ export default class Notifications extends React.PureComponent<IProps, IState> {
}
return (
- <div className="mx_UserNotifSettings">
+ <>
{this.renderTopSection()}
{this.renderCategory(RuleClass.VectorGlobal)}
{this.renderCategory(RuleClass.VectorMentions)}
{this.renderCategory(RuleClass.VectorOther)}
{this.renderTargets()}
- </div>
+ </>
);
}
}
diff --git a/src/components/views/settings/shared/SettingsSubsection.tsx b/src/components/views/settings/shared/SettingsSubsection.tsx
index eaf534cab0..ab36e9bfea 100644
--- a/src/components/views/settings/shared/SettingsSubsection.tsx
+++ b/src/components/views/settings/shared/SettingsSubsection.tsx
@@ -20,7 +20,7 @@ import React, { HTMLAttributes } from "react";
import { SettingsSubsectionHeading } from "./SettingsSubsectionHeading";
export interface SettingsSubsectionProps extends HTMLAttributes<HTMLDivElement> {
- heading: string | React.ReactNode;
+ heading?: string | React.ReactNode;
description?: string | React.ReactNode;
children?: React.ReactNode;
// when true content will be justify-items: stretch, which will make items within the section stretch to full width.
@@ -50,6 +50,7 @@ export const SettingsSubsection: React.FC<SettingsSubsectionProps> = ({
<div
className={classNames("mx_SettingsSubsection_content", {
mx_SettingsSubsection_contentStretch: !!stretchContent,
+ mx_SettingsSubsection_noHeading: !heading && !description,
})}
>
{children}
diff --git a/src/components/views/settings/tabs/user/NotificationUserSettingsTab.tsx b/src/components/views/settings/tabs/user/NotificationUserSettingsTab.tsx
index 5dd88b2c0f..4e95220df1 100644
--- a/src/components/views/settings/tabs/user/NotificationUserSettingsTab.tsx
+++ b/src/components/views/settings/tabs/user/NotificationUserSettingsTab.tsx
@@ -18,16 +18,17 @@ import React from "react";
import { _t } from "../../../../../languageHandler";
import Notifications from "../../Notifications";
+import { SettingsSection } from "../../shared/SettingsSection";
+import SettingsTab from "../SettingsTab";
export default class NotificationUserSettingsTab extends React.Component {
public render(): React.ReactNode {
return (
- <div className="mx_SettingsTab">
- <div className="mx_SettingsTab_heading">{_t("Notifications")}</div>
- <div className="mx_SettingsTab_section mx_SettingsTab_subsectionText">
+ <SettingsTab>
+ <SettingsSection heading={_t("Notifications")}>
<Notifications />
- </div>
- </div>
+ </SettingsSection>
+ </SettingsTab>
);
}
}
diff --git a/test/components/views/settings/__snapshots__/Notifications-test.tsx.snap b/test/components/views/settings/__snapshots__/Notifications-test.tsx.snap
index 6d9c127b5f..bf6c01e50b 100644
--- a/test/components/views/settings/__snapshots__/Notifications-test.tsx.snap
+++ b/test/components/views/settings/__snapshots__/Notifications-test.tsx.snap
@@ -3,40 +3,36 @@
exports[`<Notifications /> main notification switches renders only enable notifications switch when notifications are disabled 1`] = `
<div>
<div
- class="mx_UserNotifSettings"
+ class="mx_SettingsFlag"
+ data-testid="notif-master-switch"
>
- <div
- class="mx_SettingsFlag"
- data-testid="notif-master-switch"
+ <span
+ class="mx_SettingsFlag_label"
>
- <span
- class="mx_SettingsFlag_label"
- >
- <div
- id="mx_LabelledToggleSwitch_testid_0"
- >
- Enable notifications for this account
- </div>
- <span
- class="mx_Caption"
- id="mx_LabelledToggleSwitch_testid_0_caption"
- >
- Turn off to disable notifications on all your devices and sessions
- </span>
- </span>
<div
- aria-checked="false"
- aria-describedby="mx_LabelledToggleSwitch_testid_0_caption"
- aria-disabled="false"
- aria-labelledby="mx_LabelledToggleSwitch_testid_0"
- class="mx_AccessibleButton mx_ToggleSwitch mx_ToggleSwitch_enabled"
- role="switch"
- tabindex="0"
+ id="mx_LabelledToggleSwitch_testid_0"
>
- <div
- class="mx_ToggleSwitch_ball"
- />
+ Enable notifications for this account
</div>
+ <span
+ class="mx_Caption"
+ id="mx_LabelledToggleSwitch_testid_0_caption"
+ >
+ Turn off to disable notifications on all your devices and sessions
+ </span>
+ </span>
+ <div
+ aria-checked="false"
+ aria-describedby="mx_LabelledToggleSwitch_testid_0_caption"
+ aria-disabled="false"
+ aria-labelledby="mx_LabelledToggleSwitch_testid_0"
+ class="mx_AccessibleButton mx_ToggleSwitch mx_ToggleSwitch_enabled"
+ role="switch"
+ tabindex="0"
+ >
+ <div
+ class="mx_ToggleSwitch_ball"
+ />
</div>
</div>
</div>