Use semantic headings in user settings Labs (#10773)

* allow testids in settings sections

* use semantic headings in LabsUserSettingsTab

* put back margin var

* explicit cast to boolean

* Update src/components/views/settings/shared/SettingsSubsection.tsx

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

---------

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

res/css/_components.pcss

@@ -344,7 +344,6 @@
 @import "./views/settings/tabs/user/_GeneralUserSettingsTab.pcss";
 @import "./views/settings/tabs/user/_HelpUserSettingsTab.pcss";
 @import "./views/settings/tabs/user/_KeyboardUserSettingsTab.pcss";
-@import "./views/settings/tabs/user/_LabsUserSettingsTab.pcss";
 @import "./views/settings/tabs/user/_MjolnirUserSettingsTab.pcss";
 @import "./views/settings/tabs/user/_PreferencesUserSettingsTab.pcss";
 @import "./views/settings/tabs/user/_SecurityUserSettingsTab.pcss";

res/css/components/views/settings/shared/_SettingsSubsection.pcss

@@ -46,4 +46,8 @@ limitations under the License.
             margin-bottom: $spacing-8;
         }
     }
+
+    &.mx_SettingsSubsection_contentStretch {
+        justify-items: stretch;
+    }
 }

res/css/views/beta/_BetaCard.pcss

@@ -15,7 +15,6 @@ limitations under the License.
 */
 
 .mx_BetaCard {
-    margin-bottom: $spacing-20;
     padding: $spacing-24;
     background-color: $system;
     border-radius: 8px;
@@ -114,10 +113,6 @@ limitations under the License.
             }
         }
     }
-
-    &:last-child {
-        margin-bottom: 0;
-    }
 }
 
 .mx_BetaCard_betaPill {

res/css/views/elements/_SettingsFlag.pcss

@@ -20,6 +20,7 @@ limitations under the License.
     align-items: flex-start;
     justify-content: space-between;
     margin-bottom: 4px;
+    width: 100%;
 
     .mx_ToggleSwitch {
         flex: 0 0 auto;

res/css/views/settings/tabs/user/_LabsUserSettingsTab.pcss

@@ -1,27 +0,0 @@
-/*
-Copyright 2021 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.
-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_LabsUserSettingsTab {
-    .mx_SettingsTab_subsectionText,
-    .mx_SettingsTab_section {
-        margin-bottom: var(--SettingsTab_section-margin-bottom-preferences-labs);
-    }
-
-    .mx_SettingsFlag {
-        margin-right: 0; /* remove right margin to align with beta cards */
-        align-items: center;
-    }
-}

src/components/views/settings/shared/SettingsSubsection.tsx

@@ -14,6 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
+import classNames from "classnames";
 import React, { HTMLAttributes } from "react";
 
 import { SettingsSubsectionHeading } from "./SettingsSubsectionHeading";
@@ -22,6 +23,8 @@ export interface SettingsSubsectionProps extends HTMLAttributes<HTMLDivElement>
     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.
+    stretchContent?: boolean;
 }
 
 export const SettingsSubsectionText: React.FC<HTMLAttributes<HTMLDivElement>> = ({ children, ...rest }) => (
@@ -30,7 +33,13 @@ export const SettingsSubsectionText: React.FC<HTMLAttributes<HTMLDivElement>> =
     </div>
 );
 
-export const SettingsSubsection: React.FC<SettingsSubsectionProps> = ({ heading, description, children, ...rest }) => (
+export const SettingsSubsection: React.FC<SettingsSubsectionProps> = ({
+    heading,
+    description,
+    children,
+    stretchContent,
+    ...rest
+}) => (
     <div {...rest} className="mx_SettingsSubsection">
         {typeof heading === "string" ? <SettingsSubsectionHeading heading={heading} /> : <>{heading}</>}
         {!!description && (
@@ -38,7 +47,13 @@ export const SettingsSubsection: React.FC<SettingsSubsectionProps> = ({ heading,
                 <SettingsSubsectionText>{description}</SettingsSubsectionText>
             </div>
         )}
-        <div className="mx_SettingsSubsection_content">{children}</div>
+        <div
+            className={classNames("mx_SettingsSubsection_content", {
+                mx_SettingsSubsection_contentStretch: !!stretchContent,
+            })}
+        >
+            {children}
+        </div>
     </div>
 );
 

src/components/views/settings/tabs/SettingsTab.tsx

@@ -13,9 +13,9 @@ 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.
 */
-import React from "react";
+import React, { HTMLAttributes } from "react";
 
-export interface SettingsTabProps {
+export interface SettingsTabProps extends Omit<HTMLAttributes<HTMLDivElement>, "className"> {
     children?: React.ReactNode;
 }
 
@@ -37,8 +37,8 @@ export interface SettingsTabProps {
  * </SettingsTab>
  * ```
  */
-const SettingsTab: React.FC<SettingsTabProps> = ({ children }) => (
+const SettingsTab: React.FC<SettingsTabProps> = ({ children, ...rest }) => (
-    <div className="mx_SettingsTab">
+    <div {...rest} className="mx_SettingsTab">
         <div className="mx_SettingsTab_sections">{children}</div>
     </div>
 );

src/components/views/settings/tabs/user/LabsUserSettingsTab.tsx

@@ -25,6 +25,9 @@ import BetaCard from "../../../beta/BetaCard";
 import SettingsFlag from "../../../elements/SettingsFlag";
 import { LabGroup, labGroupNames } from "../../../../../settings/Settings";
 import { EnhancedMap } from "../../../../../utils/maps";
+import { SettingsSection } from "../../shared/SettingsSection";
+import SettingsSubsection, { SettingsSubsectionText } from "../../shared/SettingsSubsection";
+import SettingsTab from "../SettingsTab";
 
 export default class LabsUserSettingsTab extends React.Component<{}> {
     private readonly labs: string[];
@@ -54,11 +57,11 @@ export default class LabsUserSettingsTab extends React.Component<{}> {
         let betaSection: JSX.Element | undefined;
         if (this.betas.length) {
             betaSection = (
-                <div data-testid="labs-beta-section" className="mx_SettingsTab_section">
+                <>
                     {this.betas.map((f) => (
                         <BetaCard key={f} featureId={f} />
                     ))}
-                </div>
+                </>
             );
         }
 
@@ -93,31 +96,35 @@ export default class LabsUserSettingsTab extends React.Component<{}> {
             labsSections = (
                 <>
                     {sortBy(Array.from(groups.entries()), "0").map(([group, flags]) => (
-                        <div className="mx_SettingsTab_section" key={group} data-testid={`labs-group-${group}`}>
+                        <SettingsSubsection
-                            <span className="mx_SettingsTab_subheading">{_t(labGroupNames[group])}</span>
+                            key={group}
+                            data-testid={`labs-group-${group}`}
+                            heading={_t(labGroupNames[group])}
+                        >
                             {flags}
-                        </div>
+                        </SettingsSubsection>
                     ))}
                 </>
             );
         }
 
         return (
-            <div className="mx_SettingsTab mx_LabsUserSettingsTab">
+            <SettingsTab>
-                <div className="mx_SettingsTab_heading">{_t("Upcoming features")}</div>
+                <SettingsSection heading={_t("Upcoming features")}>
-                <div className="mx_SettingsTab_subsectionText">
+                    <SettingsSubsectionText>
-                    {_t(
+                        {_t(
-                        "What's next for %(brand)s? " +
+                            "What's next for %(brand)s? " +
-                            "Labs are the best way to get things early, " +
+                                "Labs are the best way to get things early, " +
-                            "test out new features and help shape them before they actually launch.",
+                                "test out new features and help shape them before they actually launch.",
-                        { brand: SdkConfig.get("brand") },
+                            { brand: SdkConfig.get("brand") },
-                    )}
+                        )}
-                </div>
+                    </SettingsSubsectionText>
-                {betaSection}
+                    {betaSection}
+                </SettingsSection>
+
                 {labsSections && (
-                    <>
+                    <SettingsSection heading={_t("Early previews")}>
-                        <div className="mx_SettingsTab_heading">{_t("Early previews")}</div>
+                        <SettingsSubsectionText>
-                        <div className="mx_SettingsTab_subsectionText">
                             {_t(
                                 "Feeling experimental? " +
                                     "Try out our latest ideas in development. " +
@@ -139,11 +146,11 @@ export default class LabsUserSettingsTab extends React.Component<{}> {
                                     },
                                 },
                             )}
-                        </div>
+                        </SettingsSubsectionText>
                         {labsSections}
-                    </>
+                    </SettingsSection>
                 )}
-            </div>
+            </SettingsTab>
         );
     }
 }

test/components/views/settings/tabs/user/LabsUserSettingsTab-test.tsx

@@ -15,7 +15,7 @@ limitations under the License.
 */
 
 import React from "react";
-import { render, waitFor } from "@testing-library/react";
+import { render, screen, waitFor } from "@testing-library/react";
 import { defer } from "matrix-js-sdk/src/utils";
 
 import LabsUserSettingsTab from "../../../../../../src/components/views/settings/tabs/user/LabsUserSettingsTab";
@@ -51,17 +51,16 @@ describe("<LabsUserSettingsTab />", () => {
     });
 
     it("renders settings marked as beta as beta cards", () => {
-        const { getByTestId } = render(getComponent());
+        render(getComponent());
-        expect(getByTestId("labs-beta-section")).toMatchSnapshot();
+        expect(screen.getByText("Upcoming features").parentElement!).toMatchSnapshot();
     });
 
     it("does not render non-beta labs settings when disabled in config", () => {
-        const { container } = render(getComponent());
+        render(getComponent());
         expect(sdkConfigSpy).toHaveBeenCalledWith("show_labs_settings");
 
-        const labsSections = container.getElementsByClassName("mx_SettingsTab_section");
         // only section is beta section
-        expect(labsSections.length).toEqual(1);
+        expect(screen.queryByText("Early previews")).not.toBeInTheDocument();
     });
 
     it("renders non-beta labs settings when enabled in config", () => {
@@ -69,8 +68,10 @@ describe("<LabsUserSettingsTab />", () => {
         sdkConfigSpy.mockImplementation((configName) => configName === "show_labs_settings");
         const { container } = render(getComponent());
 
-        const labsSections = container.getElementsByClassName("mx_SettingsTab_section");
+        // non-beta labs section
-        expect(labsSections).toHaveLength(11);
+        expect(screen.getByText("Early previews")).toBeInTheDocument();
+        const labsSections = container.getElementsByClassName("mx_SettingsSubsection");
+        expect(labsSections).toHaveLength(10);
     });
 
     it("allow setting a labs flag which requires unstable support once support is confirmed", async () => {

test/components/views/settings/tabs/user/__snapshots__/LabsUserSettingsTab-test.tsx.snap

@@ -2,121 +2,134 @@
 
 exports[`<LabsUserSettingsTab /> renders settings marked as beta as beta cards 1`] = `
 <div
-  class="mx_SettingsTab_section"
+  class="mx_SettingsSection"
-  data-testid="labs-beta-section"
 >
+  <h2
+    class="mx_Heading_h2"
+  >
+    Upcoming features
+  </h2>
   <div
-    class="mx_BetaCard"
+    class="mx_SettingsSection_subSections"
   >
     <div
-      class="mx_BetaCard_columns"
+      class="mx_SettingsSubsection_text"
+    >
+      What's next for false? Labs are the best way to get things early, test out new features and help shape them before they actually launch.
+    </div>
+    <div
+      class="mx_BetaCard"
     >
       <div
-        class="mx_BetaCard_columns_description"
+        class="mx_BetaCard_columns"
       >
-        <h3
+        <div
-          class="mx_BetaCard_title"
+          class="mx_BetaCard_columns_description"
         >
-          <span>
+          <h3
-            Video rooms
+            class="mx_BetaCard_title"
-          </span>
-          <span
-            class="mx_BetaCard_betaPill"
           >
-            Beta
+            <span>
-          </span>
+              Video rooms
-        </h3>
+            </span>
-        <div
+            <span
-          class="mx_BetaCard_caption"
+              class="mx_BetaCard_betaPill"
-        >
+            >
-          <p>
+              Beta
-            A new way to chat over voice and video in .
+            </span>
-          </p>
+          </h3>
-          <p>
-            Video rooms are always-on VoIP channels embedded within a room in .
-          </p>
-        </div>
-        <div
-          class="mx_BetaCard_buttons"
-        >
           <div
-            class="mx_AccessibleButton mx_AccessibleButton_hasKind mx_AccessibleButton_kind_primary"
+            class="mx_BetaCard_caption"
-            role="button"
-            tabindex="0"
           >
-            Join the beta
+            <p>
+              A new way to chat over voice and video in .
+            </p>
+            <p>
+              Video rooms are always-on VoIP channels embedded within a room in .
+            </p>
           </div>
+          <div
+            class="mx_BetaCard_buttons"
+          >
+            <div
+              class="mx_AccessibleButton mx_AccessibleButton_hasKind mx_AccessibleButton_kind_primary"
+              role="button"
+              tabindex="0"
+            >
+              Join the beta
+            </div>
+          </div>
+          <div
+            class="mx_BetaCard_refreshWarning"
+          >
+            Joining the beta will reload .
+          </div>
+          <div
+            class="mx_BetaCard_faq"
+          />
         </div>
         <div
-          class="mx_BetaCard_refreshWarning"
+          class="mx_BetaCard_columns_image_wrapper"
         >
-          Joining the beta will reload .
+          <img
+            alt=""
+            class="mx_BetaCard_columns_image"
+            src="image-file-stub"
+          />
         </div>
-        <div
-          class="mx_BetaCard_faq"
-        />
-      </div>
-      <div
-        class="mx_BetaCard_columns_image_wrapper"
-      >
-        <img
-          alt=""
-          class="mx_BetaCard_columns_image"
-          src="image-file-stub"
-        />
       </div>
     </div>
-  </div>
-  <div
-    class="mx_BetaCard"
-  >
     <div
-      class="mx_BetaCard_columns"
+      class="mx_BetaCard"
     >
       <div
-        class="mx_BetaCard_columns_description"
+        class="mx_BetaCard_columns"
       >
-        <h3
+        <div
-          class="mx_BetaCard_title"
+          class="mx_BetaCard_columns_description"
         >
-          <span>
+          <h3
-            New session manager
+            class="mx_BetaCard_title"
-          </span>
-          <span
-            class="mx_BetaCard_betaPill"
           >
-            Beta
+            <span>
-          </span>
+              New session manager
-        </h3>
+            </span>
-        <div
+            <span
-          class="mx_BetaCard_caption"
+              class="mx_BetaCard_betaPill"
-        >
+            >
-          <p>
+              Beta
-            Have greater visibility and control over all your sessions.
+            </span>
-          </p>
+          </h3>
-          <p>
-            Our new sessions manager provides better visibility of all your sessions, and greater control over them including the ability to remotely toggle push notifications.
-          </p>
-        </div>
-        <div
-          class="mx_BetaCard_buttons"
-        >
           <div
-            class="mx_AccessibleButton mx_AccessibleButton_hasKind mx_AccessibleButton_kind_primary"
+            class="mx_BetaCard_caption"
-            role="button"
-            tabindex="0"
           >
-            Join the beta
+            <p>
+              Have greater visibility and control over all your sessions.
+            </p>
+            <p>
+              Our new sessions manager provides better visibility of all your sessions, and greater control over them including the ability to remotely toggle push notifications.
+            </p>
+          </div>
+          <div
+            class="mx_BetaCard_buttons"
+          >
+            <div
+              class="mx_AccessibleButton mx_AccessibleButton_hasKind mx_AccessibleButton_kind_primary"
+              role="button"
+              tabindex="0"
+            >
+              Join the beta
+            </div>
           </div>
         </div>
-      </div>
+        <div
-      <div
+          class="mx_BetaCard_columns_image_wrapper"
-        class="mx_BetaCard_columns_image_wrapper"
+        >
-      >
+          <img
-        <img
+            alt=""
-          alt=""
+            class="mx_BetaCard_columns_image"
-          class="mx_BetaCard_columns_image"
+          />
-        />
+        </div>
       </div>
     </div>
   </div>