Use semantic headings in space settings (#10751)

* split SettingsSection out of SettingsTab, replace usage

* correct copyright

* use semantic headings in GeneralRoomSettingsTab

* use SettingsTab and SettingsSubsection in room settings

* fix VoipRoomSettingsTab

* use SettingsSection components in space settings

res/css/_components.pcss

@@ -341,7 +341,6 @@
 @import "./views/settings/_UpdateCheckButton.pcss";
 @import "./views/settings/tabs/_SettingsSection.pcss";
 @import "./views/settings/tabs/_SettingsTab.pcss";
-@import "./views/settings/tabs/room/_GeneralRoomSettingsTab.pcss";
 @import "./views/settings/tabs/room/_NotificationSettingsTab.pcss";
 @import "./views/settings/tabs/room/_RolesRoomSettingsTab.pcss";
 @import "./views/settings/tabs/room/_SecurityRoomSettingsTab.pcss";

res/css/views/room_settings/_AliasSettings.pcss

@@ -27,20 +27,14 @@ limitations under the License.
     box-shadow: none;
 }
 
-.mx_AliasSettings {
+.mx_AliasSettings_localAddresses {
-    summary {
+    cursor: pointer;
-        cursor: pointer;
+    color: $accent;
-        color: $accent;
+    font-weight: var(--font-semi-bold);
-        font-weight: var(--font-semi-bold);
+    list-style: none;
-        list-style: none;
 
-        /* list-style doesn't do it for webkit */
+    /* list-style doesn't do it for webkit */
-        &::-webkit-details-marker {
+    &::-webkit-details-marker {
-            display: none;
+        display: none;
-        }
-    }
-
-    .mx_AliasSettings_localAliasHeader {
-        margin-top: 35px;
     }
 }

res/css/views/settings/tabs/room/_GeneralRoomSettingsTab.pcss

@@ -1,19 +0,0 @@
-/*
-Copyright 2019 New Vector Ltd
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-.mx_GeneralRoomSettingsTab_profileSection {
-    margin-top: 10px;
-}

src/components/views/room_settings/AliasSettings.tsx

@@ -1,5 +1,5 @@
 /*
-Copyright 2016 - 2021 The Matrix.org Foundation C.I.C.
+Copyright 2016 - 2023 The Matrix.org Foundation C.I.C.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -402,7 +402,7 @@ export default class AliasSettings extends React.Component<IProps, IState> {
         }
 
         return (
-            <div className="mx_AliasSettings">
+            <>
                 <SettingsFieldset
                     data-testid="published-address-fieldset"
                     legend={_t("Published Addresses")}
@@ -416,15 +416,6 @@ export default class AliasSettings extends React.Component<IProps, IState> {
                         </>
                     }
                 >
-                    {/*
-                <span className='mx_SettingsTab_subheading'>{ _t("Published Addresses") }</span>
-                <p>
-                    { isSpaceRoom
-                        ? _t("Published addresses can be used by anyone on any server to join your space.")
-                        : _t("Published addresses can be used by anyone on any server to join your room.") }
-                    &nbsp;
-                    { _t("To publish an address, it needs to be set as a local address first.") }
-                </p> */}
                     {canonicalAliasSection}
                     {this.props.hidePublishSetting ? null : (
                         <RoomPublishSetting
@@ -472,11 +463,13 @@ export default class AliasSettings extends React.Component<IProps, IState> {
                     }
                 >
                     <details onToggle={this.onLocalAliasesToggled} open={this.state.detailsOpen}>
-                        <summary>{this.state.detailsOpen ? _t("Show less") : _t("Show more")}</summary>
+                        <summary className="mx_AliasSettings_localAddresses">
+                            {this.state.detailsOpen ? _t("Show less") : _t("Show more")}
+                        </summary>
                         {localAliasesList}
                     </details>
                 </SettingsFieldset>
-            </div>
+            </>
         );
     }
 }

src/components/views/settings/tabs/room/GeneralRoomSettingsTab.tsx

@@ -86,9 +86,7 @@ export default class GeneralRoomSettingsTab extends React.Component<IProps, ISta
         return (
             <SettingsTab>
                 <SettingsSection heading={_t("General")}>
-                    <div className="mx_SettingsTab_section mx_GeneralRoomSettingsTab_profileSection">
+                    <RoomProfileSettings roomId={room.roomId} />
-                        <RoomProfileSettings roomId={room.roomId} />
-                    </div>
                 </SettingsSection>
 
                 <SettingsSection heading={_t("Room Addresses")}>

src/components/views/spaces/SpaceSettingsGeneralTab.tsx

@@ -27,6 +27,9 @@ import { avatarUrlForRoom } from "../../../Avatar";
 import { htmlSerializeFromMdIfNeeded } from "../../../editor/serialize";
 import { leaveSpace } from "../../../utils/leave-behaviour";
 import { getTopic } from "../../../hooks/room/useTopic";
+import SettingsTab from "../settings/tabs/SettingsTab";
+import { SettingsSection } from "../settings/shared/SettingsSection";
+import SettingsSubsection from "../settings/shared/SettingsSubsection";
 
 interface IProps {
     matrixClient: MatrixClient;
@@ -94,50 +97,49 @@ const SpaceSettingsGeneralTab: React.FC<IProps> = ({ matrixClient: cli, space })
     };
 
     return (
-        <div className="mx_SettingsTab">
+        <SettingsTab>
-            <div className="mx_SettingsTab_heading">{_t("General")}</div>
+            <SettingsSection heading={_t("General")}>
+                <div>
+                    <div>{_t("Edit settings relating to your space.")}</div>
 
-            <div>{_t("Edit settings relating to your space.")}</div>
+                    {error && <div className="mx_SpaceRoomView_errorText">{error}</div>}
 
-            {error && <div className="mx_SpaceRoomView_errorText">{error}</div>}
+                    <SpaceBasicSettings
+                        avatarUrl={avatarUrlForRoom(space, 80, 80, "crop") ?? undefined}
+                        avatarDisabled={busy || !canSetAvatar}
+                        setAvatar={setNewAvatar}
+                        name={name}
+                        nameDisabled={busy || !canSetName}
+                        setName={setName}
+                        topic={topic}
+                        topicDisabled={busy || !canSetTopic}
+                        setTopic={setTopic}
+                    />
 
-            <div className="mx_SettingsTab_section">
+                    <AccessibleButton
-                <SpaceBasicSettings
+                        onClick={onCancel}
-                    avatarUrl={avatarUrlForRoom(space, 80, 80, "crop") ?? undefined}
+                        disabled={busy || !(avatarChanged || nameChanged || topicChanged)}
-                    avatarDisabled={busy || !canSetAvatar}
+                        kind="link"
-                    setAvatar={setNewAvatar}
+                    >
-                    name={name}
+                        {_t("Cancel")}
-                    nameDisabled={busy || !canSetName}
+                    </AccessibleButton>
-                    setName={setName}
+                    <AccessibleButton onClick={onSave} disabled={busy} kind="primary">
-                    topic={topic}
+                        {busy ? _t("Saving…") : _t("Save Changes")}
-                    topicDisabled={busy || !canSetTopic}
+                    </AccessibleButton>
-                    setTopic={setTopic}
+                </div>
-                />
 
-                <AccessibleButton
+                <SettingsSubsection heading={_t("Leave Space")}>
-                    onClick={onCancel}
+                    <AccessibleButton
-                    disabled={busy || !(avatarChanged || nameChanged || topicChanged)}
+                        kind="danger"
-                    kind="link"
+                        onClick={() => {
-                >
+                            leaveSpace(space);
-                    {_t("Cancel")}
+                        }}
-                </AccessibleButton>
+                    >
-                <AccessibleButton onClick={onSave} disabled={busy} kind="primary">
+                        {_t("Leave Space")}
-                    {busy ? _t("Saving…") : _t("Save Changes")}
+                    </AccessibleButton>
-                </AccessibleButton>
+                </SettingsSubsection>
-            </div>
+            </SettingsSection>
-
+        </SettingsTab>
-            <span className="mx_SettingsTab_subheading">{_t("Leave Space")}</span>
-            <div className="mx_SettingsTab_section mx_SettingsTab_subsectionText">
-                <AccessibleButton
-                    kind="danger"
-                    onClick={() => {
-                        leaveSpace(space);
-                    }}
-                >
-                    {_t("Leave Space")}
-                </AccessibleButton>
-            </div>
-        </div>
     );
 };
 

src/components/views/spaces/SpaceSettingsVisibilityTab.tsx

@@ -1,5 +1,5 @@
 /*
-Copyright 2021 The Matrix.org Foundation C.I.C.
+Copyright 2021-2023 The Matrix.org Foundation C.I.C.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -30,6 +30,8 @@ import JoinRuleSettings from "../settings/JoinRuleSettings";
 import { useRoomState } from "../../../hooks/useRoomState";
 import SettingsFieldset from "../settings/SettingsFieldset";
 import { useAsyncMemo } from "../../../hooks/useAsyncMemo";
+import { SettingsSection } from "../settings/shared/SettingsSection";
+import SettingsTab from "../settings/tabs/SettingsTab";
 
 interface IProps {
     matrixClient: MatrixClient;
@@ -124,8 +126,7 @@ const SpaceSettingsVisibilityTab: React.FC<IProps> = ({ matrixClient: cli, space
     let addressesSection: JSX.Element | undefined;
     if (space.getJoinRule() === JoinRule.Public) {
         addressesSection = (
-            <>
+            <SettingsSection heading={_t("Address")}>
-                <span className="mx_SettingsTab_subheading">{_t("Address")}</span>
                 <AliasSettings
                     roomId={space.roomId}
                     canSetCanonicalAlias={canSetCanonical}
@@ -133,50 +134,52 @@ const SpaceSettingsVisibilityTab: React.FC<IProps> = ({ matrixClient: cli, space
                     canonicalAliasEvent={canonicalAliasEv ?? undefined}
                     hidePublishSetting={!serverSupportsExploringSpaces}
                 />
-            </>
+            </SettingsSection>
         );
     }
 
     return (
-        <div className="mx_SettingsTab">
+        <SettingsTab>
-            <div className="mx_SettingsTab_heading">{_t("Visibility")}</div>
+            <SettingsSection heading={_t("Visibility")}>
+                {error && (
+                    <div data-testid="space-settings-error" className="mx_SpaceRoomView_errorText">
+                        {error}
+                    </div>
+                )}
 
-            {error && (
+                <SettingsFieldset
-                <div data-testid="space-settings-error" className="mx_SpaceRoomView_errorText">
+                    data-testid="access-fieldset"
-                    {error}
+                    legend={_t("Access")}
-                </div>
+                    description={_t("Decide who can view and join %(spaceName)s.", { spaceName: space.name })}
-            )}
+                >
-
+                    <JoinRuleSettings
-            <SettingsFieldset
+                        room={space}
-                data-testid="access-fieldset"
+                        onError={(): void => setError(_t("Failed to update the visibility of this space"))}
-                legend={_t("Access")}
+                        closeSettingsFn={closeSettingsFn}
-                description={_t("Decide who can view and join %(spaceName)s.", { spaceName: space.name })}
-            >
-                <JoinRuleSettings
-                    room={space}
-                    onError={(): void => setError(_t("Failed to update the visibility of this space"))}
-                    closeSettingsFn={closeSettingsFn}
-                />
-                {advancedSection}
-                <div className="mx_SettingsTab_toggleWithDescription">
-                    <LabelledToggleSwitch
-                        value={historyVisibility === HistoryVisibility.WorldReadable}
-                        onChange={(checked: boolean): void => {
-                            setHistoryVisibility(checked ? HistoryVisibility.WorldReadable : HistoryVisibility.Shared);
-                        }}
-                        disabled={!canSetHistoryVisibility}
-                        label={_t("Preview Space")}
                     />
-                    <p>
+                    {advancedSection}
-                        {_t("Allow people to preview your space before they join.")}
+                    <div className="mx_SettingsTab_toggleWithDescription">
-                        <br />
+                        <LabelledToggleSwitch
-                        <b>{_t("Recommended for public spaces.")}</b>
+                            value={historyVisibility === HistoryVisibility.WorldReadable}
-                    </p>
+                            onChange={(checked: boolean): void => {
-                </div>
+                                setHistoryVisibility(
-            </SettingsFieldset>
+                                    checked ? HistoryVisibility.WorldReadable : HistoryVisibility.Shared,
+                                );
+                            }}
+                            disabled={!canSetHistoryVisibility}
+                            label={_t("Preview Space")}
+                        />
+                        <p>
+                            {_t("Allow people to preview your space before they join.")}
+                            <br />
+                            <b>{_t("Recommended for public spaces.")}</b>
+                        </p>
+                    </div>
+                </SettingsFieldset>
 
-            {addressesSection}
+                {addressesSection}
-        </div>
+            </SettingsSection>
+        </SettingsTab>
     );
 };
 

test/components/views/spaces/__snapshots__/SpaceSettingsVisibilityTab-test.tsx.snap

@@ -21,118 +21,130 @@ exports[`<SpaceSettingsVisibilityTab /> renders container 1`] = `
     class="mx_SettingsTab"
   >
     <div
-      class="mx_SettingsTab_heading"
+      class="mx_SettingsTab_sections"
     >
-      Visibility
-    </div>
-    <fieldset
-      class="mx_SettingsFieldset"
-      data-testid="access-fieldset"
-    >
-      <legend
-        class="mx_SettingsFieldset_legend"
-      >
-        Access
-      </legend>
       <div
-        class="mx_SettingsFieldset_description"
+        class="mx_SettingsSection"
       >
-        Decide who can view and join mock-space.
+        <h2
-      </div>
+          class="mx_Heading_h2"
-      <label
-        class="mx_StyledRadioButton mx_JoinRuleSettings_radioButton mx_StyledRadioButton_disabled mx_StyledRadioButton_checked"
-      >
-        <input
-          aria-describedby="joinRule-invite-description"
-          checked=""
-          disabled=""
-          id="joinRule-invite"
-          name="joinRule"
-          type="radio"
-          value="invite"
-        />
-        <div>
-          <div />
-        </div>
-        <div
-          class="mx_StyledRadioButton_content"
         >
-          Private (invite only)
+          Visibility
-        </div>
+        </h2>
         <div
-          class="mx_StyledRadioButton_spacer"
+          class="mx_SettingsSection_subSections"
-        />
-      </label>
-      <span
-        id="joinRule-invite-description"
-      >
-        Only invited people can join.
-      </span>
-      <label
-        class="mx_StyledRadioButton mx_JoinRuleSettings_radioButton mx_StyledRadioButton_disabled"
-      >
-        <input
-          aria-describedby="joinRule-public-description"
-          disabled=""
-          id="joinRule-public"
-          name="joinRule"
-          type="radio"
-          value="public"
-        />
-        <div>
-          <div />
-        </div>
-        <div
-          class="mx_StyledRadioButton_content"
         >
-          Public
+          <fieldset
-        </div>
+            class="mx_SettingsFieldset"
-        <div
+            data-testid="access-fieldset"
-          class="mx_StyledRadioButton_spacer"
-        />
-      </label>
-      <span
-        id="joinRule-public-description"
-      >
-        Anyone can find and join.
-      </span>
-      <div
-        class="mx_SettingsTab_toggleWithDescription"
-      >
-        <div
-          class="mx_SettingsFlag"
-        >
-          <span
-            class="mx_SettingsFlag_label"
           >
-            <div
+            <legend
-              id="mx_LabelledToggleSwitch_testid_0"
+              class="mx_SettingsFieldset_legend"
             >
-              Preview Space
+              Access
-            </div>
+            </legend>
-          </span>
-          <div
-            aria-checked="true"
-            aria-disabled="false"
-            aria-labelledby="mx_LabelledToggleSwitch_testid_0"
-            class="mx_AccessibleButton mx_ToggleSwitch mx_ToggleSwitch_on mx_ToggleSwitch_enabled"
-            role="switch"
-            tabindex="0"
-          >
             <div
-              class="mx_ToggleSwitch_ball"
+              class="mx_SettingsFieldset_description"
-            />
+            >
-          </div>
+              Decide who can view and join mock-space.
+            </div>
+            <label
+              class="mx_StyledRadioButton mx_JoinRuleSettings_radioButton mx_StyledRadioButton_disabled mx_StyledRadioButton_checked"
+            >
+              <input
+                aria-describedby="joinRule-invite-description"
+                checked=""
+                disabled=""
+                id="joinRule-invite"
+                name="joinRule"
+                type="radio"
+                value="invite"
+              />
+              <div>
+                <div />
+              </div>
+              <div
+                class="mx_StyledRadioButton_content"
+              >
+                Private (invite only)
+              </div>
+              <div
+                class="mx_StyledRadioButton_spacer"
+              />
+            </label>
+            <span
+              id="joinRule-invite-description"
+            >
+              Only invited people can join.
+            </span>
+            <label
+              class="mx_StyledRadioButton mx_JoinRuleSettings_radioButton mx_StyledRadioButton_disabled"
+            >
+              <input
+                aria-describedby="joinRule-public-description"
+                disabled=""
+                id="joinRule-public"
+                name="joinRule"
+                type="radio"
+                value="public"
+              />
+              <div>
+                <div />
+              </div>
+              <div
+                class="mx_StyledRadioButton_content"
+              >
+                Public
+              </div>
+              <div
+                class="mx_StyledRadioButton_spacer"
+              />
+            </label>
+            <span
+              id="joinRule-public-description"
+            >
+              Anyone can find and join.
+            </span>
+            <div
+              class="mx_SettingsTab_toggleWithDescription"
+            >
+              <div
+                class="mx_SettingsFlag"
+              >
+                <span
+                  class="mx_SettingsFlag_label"
+                >
+                  <div
+                    id="mx_LabelledToggleSwitch_testid_0"
+                  >
+                    Preview Space
+                  </div>
+                </span>
+                <div
+                  aria-checked="true"
+                  aria-disabled="false"
+                  aria-labelledby="mx_LabelledToggleSwitch_testid_0"
+                  class="mx_AccessibleButton mx_ToggleSwitch mx_ToggleSwitch_on mx_ToggleSwitch_enabled"
+                  role="switch"
+                  tabindex="0"
+                >
+                  <div
+                    class="mx_ToggleSwitch_ball"
+                  />
+                </div>
+              </div>
+              <p>
+                Allow people to preview your space before they join.
+                <br />
+                <b>
+                  Recommended for public spaces.
+                </b>
+              </p>
+            </div>
+          </fieldset>
         </div>
-        <p>
-          Allow people to preview your space before they join.
-          <br />
-          <b>
-            Recommended for public spaces.
-          </b>
-        </p>
       </div>
-    </fieldset>
+    </div>
   </div>
 </DocumentFragment>
 `;