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 <[email protected]> --------- Co-authored-by: Richard van der Hoff <[email protected]>
matrix-org · May 16, 2023 · 9bab356 · 9bab356
1 parent 38c1350
commit 9bab356
Show file tree

Hide file tree

Showing 10 changed files with 164 additions and 156 deletions.
diff --git a/res/css/_components.pcss b/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";

diff --git a/res/css/components/views/settings/shared/_SettingsSubsection.pcss b/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;
+    }
 }
diff --git a/res/css/views/beta/_BetaCard.pcss b/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 {

diff --git a/res/css/views/elements/_SettingsFlag.pcss b/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;

diff --git a/res/css/views/settings/tabs/user/_LabsUserSettingsTab.pcss b/res/css/views/settings/tabs/user/_LabsUserSettingsTab.pcss
diff --git a/src/components/views/settings/shared/SettingsSubsection.tsx b/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,15 +33,27 @@ 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 && (
             <div className="mx_SettingsSubsection_description">
                 <SettingsSubsectionText>{description}</SettingsSubsectionText>
             </div>
         )}
-        <div className="mx_SettingsSubsection_content">{children}</div>
+        <div
+            className={classNames("mx_SettingsSubsection_content", {
+                mx_SettingsSubsection_contentStretch: !!stretchContent,
+            })}
+        >
+            {children}
+        </div>
     </div>
 );
 

diff --git a/src/components/views/settings/tabs/SettingsTab.tsx b/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 }) => (
-    <div className="mx_SettingsTab">
+const SettingsTab: React.FC<SettingsTabProps> = ({ children, ...rest }) => (
+    <div {...rest} className="mx_SettingsTab">
         <div className="mx_SettingsTab_sections">{children}</div>
     </div>
 );

diff --git a/src/components/views/settings/tabs/user/LabsUserSettingsTab.tsx b/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}`}>
-                            <span className="mx_SettingsTab_subheading">{_t(labGroupNames[group])}</span>
+                        <SettingsSubsection
+                            key={group}
+                            data-testid={`labs-group-${group}`}
+                            heading={_t(labGroupNames[group])}
+                        >
                             {flags}
-                        </div>
+                        </SettingsSubsection>
                     ))}
                 </>
             );
         }
 
         return (
-            <div className="mx_SettingsTab mx_LabsUserSettingsTab">
-                <div className="mx_SettingsTab_heading">{_t("Upcoming features")}</div>
-                <div className="mx_SettingsTab_subsectionText">
-                    {_t(
-                        "What's next for %(brand)s? " +
-                            "Labs are the best way to get things early, " +
-                            "test out new features and help shape them before they actually launch.",
-                        { brand: SdkConfig.get("brand") },
-                    )}
-                </div>
-                {betaSection}
+            <SettingsTab>
+                <SettingsSection heading={_t("Upcoming features")}>
+                    <SettingsSubsectionText>
+                        {_t(
+                            "What's next for %(brand)s? " +
+                                "Labs are the best way to get things early, " +
+                                "test out new features and help shape them before they actually launch.",
+                            { brand: SdkConfig.get("brand") },
+                        )}
+                    </SettingsSubsectionText>
+                    {betaSection}
+                </SettingsSection>
+
                 {labsSections && (
-                    <>
-                        <div className="mx_SettingsTab_heading">{_t("Early previews")}</div>
-                        <div className="mx_SettingsTab_subsectionText">
+                    <SettingsSection heading={_t("Early previews")}>
+                        <SettingsSubsectionText>
                             {_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>
         );
     }
 }
diff --git a/test/components/views/settings/tabs/user/LabsUserSettingsTab-test.tsx b/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,26 +51,27 @@ describe("<LabsUserSettingsTab />", () => {
     });
 
     it("renders settings marked as beta as beta cards", () => {
-        const { getByTestId } = render(getComponent());
-        expect(getByTestId("labs-beta-section")).toMatchSnapshot();
+        render(getComponent());
+        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", () => {
         // enable labs
         sdkConfigSpy.mockImplementation((configName) => configName === "show_labs_settings");
         const { container } = render(getComponent());
 
-        const labsSections = container.getElementsByClassName("mx_SettingsTab_section");
-        expect(labsSections).toHaveLength(11);
+        // non-beta labs section
+        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 () => {