From e5c73a617c4ae7291dd15f2aabac1127ed33f753 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Fri, 19 May 2023 16:35:19 +1200 Subject: [PATCH 1/3] use semantic headings in user notif settings --- .../settings/shared/_SettingsSubsection.pcss | 5 +- res/css/views/settings/_Notifications.pcss | 54 +++++++++---------- .../views/settings/Notifications.tsx | 40 +++++++------- .../settings/shared/SettingsSubsection.tsx | 3 +- .../tabs/user/NotificationUserSettingsTab.tsx | 11 ++-- .../__snapshots__/Notifications-test.tsx.snap | 52 +++++++++--------- 6 files changed, 81 insertions(+), 84 deletions(-) diff --git a/res/css/components/views/settings/shared/_SettingsSubsection.pcss b/res/css/components/views/settings/shared/_SettingsSubsection.pcss index 9ab33c93536..708331e4974 100644 --- a/res/css/components/views/settings/shared/_SettingsSubsection.pcss +++ b/res/css/components/views/settings/shared/_SettingsSubsection.pcss @@ -36,7 +36,6 @@ limitations under the License. grid-gap: $spacing-8; grid-template-columns: 1fr; justify-items: flex-start; - margin-top: $spacing-24; summary { color: $accent; @@ -50,4 +49,8 @@ limitations under the License. &.mx_SettingsSubsection_contentStretch { justify-items: stretch; } + + &.mx_SettingsSubsection_hasHeading { + margin-top: $spacing-24; + } } diff --git a/res/css/views/settings/_Notifications.pcss b/res/css/views/settings/_Notifications.pcss index 635627e0b02..c35181ddcaa 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,37 +70,33 @@ 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); - } + & > 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; + > table { + border-collapse: collapse; + border-spacing: 0; + margin-top: 8px; - tr > td:first-child { - /* Just for a bit of spacing */ - padding-right: 8px; - } + tr > td:first-child { + /* Just for a bit of spacing */ + padding-right: 8px; } } +} - .mx_UserNotifSettings_clearNotifsButton { - margin-top: 8px; - } +.mx_UserNotifSettings_clearNotifsButton { + margin-top: 8px; +} - .mx_TagComposer { - margin-top: 35px; /* lots of distance from the last line of the table */ - } +.mx_TagComposer { + margin-top: 35px; /* lots of distance from the last line of the table */ } .mx_AccessibleButton.mx_NotificationSound_browse { diff --git a/src/components/views/settings/Notifications.tsx b/src/components/views/settings/Notifications.tsx index 6c6c38c0b4f..e72396b1d9f 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 { private renderTopSection(): JSX.Element { const masterSwitch = ( - <> - - + ); // If all the rules are inhibited, don't show anything. @@ -680,7 +680,7 @@ export default class Notifications extends React.PureComponent { )); return ( - <> + {masterSwitch} { )} {emailSwitches} - + ); } @@ -814,7 +814,7 @@ export default class Notifications extends React.PureComponent { )); - 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 { } return ( - <> +
- - {sectionName} - + {VectorStateToLabel[VectorState.Off]} {VectorStateToLabel[VectorState.On]} {VectorStateToLabel[VectorState.Loud]} @@ -842,7 +840,7 @@ export default class Notifications extends React.PureComponent {
{clearNotifsButton} {keywordComposer} - +
); } @@ -877,13 +875,15 @@ export default class Notifications extends React.PureComponent { } return ( -
+ //
+ <> {this.renderTopSection()} {this.renderCategory(RuleClass.VectorGlobal)} {this.renderCategory(RuleClass.VectorMentions)} {this.renderCategory(RuleClass.VectorOther)} {this.renderTargets()} -
+ + //
); } } diff --git a/src/components/views/settings/shared/SettingsSubsection.tsx b/src/components/views/settings/shared/SettingsSubsection.tsx index eaf534cab0d..912c0a72209 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 { - 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 = ({
{children} diff --git a/src/components/views/settings/tabs/user/NotificationUserSettingsTab.tsx b/src/components/views/settings/tabs/user/NotificationUserSettingsTab.tsx index 5dd88b2c0f4..4e95220df1f 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 ( -
-
{_t("Notifications")}
-
+ + -
-
+ + ); } } diff --git a/test/components/views/settings/__snapshots__/Notifications-test.tsx.snap b/test/components/views/settings/__snapshots__/Notifications-test.tsx.snap index 6d9c127b5f8..bf6c01e50bb 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[` main notification switches renders only enable notifications switch when notifications are disabled 1`] = `
-
+
+ Enable notifications for this account +
-
- Enable notifications for this account -
- - Turn off to disable notifications on all your devices and sessions - + Turn off to disable notifications on all your devices and sessions + +
-
-
+ class="mx_ToggleSwitch_ball" + />
From 2e900fa6f60c565b4072dd4304a3de52badd5cd2 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Fri, 19 May 2023 16:46:26 +1200 Subject: [PATCH 2/3] unset margin for subsection content when no heading --- .../views/settings/shared/_SettingsSubsection.pcss | 5 +++-- src/components/views/settings/shared/SettingsSubsection.tsx | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/res/css/components/views/settings/shared/_SettingsSubsection.pcss b/res/css/components/views/settings/shared/_SettingsSubsection.pcss index 708331e4974..db58d08a442 100644 --- a/res/css/components/views/settings/shared/_SettingsSubsection.pcss +++ b/res/css/components/views/settings/shared/_SettingsSubsection.pcss @@ -36,6 +36,7 @@ limitations under the License. grid-gap: $spacing-8; grid-template-columns: 1fr; justify-items: flex-start; + margin-top: $spacing-24; summary { color: $accent; @@ -50,7 +51,7 @@ limitations under the License. justify-items: stretch; } - &.mx_SettingsSubsection_hasHeading { - margin-top: $spacing-24; + &.mx_SettingsSubsection_noHeading { + margin-top: 0; } } diff --git a/src/components/views/settings/shared/SettingsSubsection.tsx b/src/components/views/settings/shared/SettingsSubsection.tsx index 912c0a72209..ab36e9bfea4 100644 --- a/src/components/views/settings/shared/SettingsSubsection.tsx +++ b/src/components/views/settings/shared/SettingsSubsection.tsx @@ -50,7 +50,7 @@ export const SettingsSubsection: React.FC = ({
{children} From d1c0320a7af987b28dbe1a1fca99a45540d6d3ba Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Fri, 19 May 2023 17:20:54 +1200 Subject: [PATCH 3/3] remove debug --- src/components/views/settings/Notifications.tsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/components/views/settings/Notifications.tsx b/src/components/views/settings/Notifications.tsx index e72396b1d9f..18d5c34c10d 100644 --- a/src/components/views/settings/Notifications.tsx +++ b/src/components/views/settings/Notifications.tsx @@ -875,7 +875,6 @@ export default class Notifications extends React.PureComponent { } return ( - //
<> {this.renderTopSection()} {this.renderCategory(RuleClass.VectorGlobal)} @@ -883,7 +882,6 @@ export default class Notifications extends React.PureComponent { {this.renderCategory(RuleClass.VectorOther)} {this.renderTargets()} - //
); } }