Use semantic headings in user settings Keyboard

cypress/e2e/settings/preferences-user-settings-tab.spec.ts

@@ -35,19 +35,16 @@ describe("Preferences user settings tab", () => {
     it("should be rendered properly", () => {
         cy.openUserSettings("Preferences");
 
-        cy.get(".mx_SettingsTab.mx_PreferencesUserSettingsTab").within(() => {
+        cy.findByTestId("mx_PreferencesUserSettingsTab").within(() => {
             // Assert that the top heading is rendered
-            cy.findByTestId("preferences").should("have.text", "Preferences").should("be.visible");
+            cy.contains("Preferences").should("be.visible");
         });
 
-        cy.get(".mx_SettingsTab.mx_PreferencesUserSettingsTab").percySnapshotElement(
-            "User settings tab - Preferences",
-            {
-                // Emulate TabbedView's actual min and max widths
-                // 580: '.mx_UserSettingsDialog .mx_TabbedView' min-width
-                // 796: 1036 (mx_TabbedView_tabsOnLeft actual width) - 240 (mx_TabbedView_tabPanel margin-right)
-                widths: [580, 796],
-            },
-        );
+        cy.findByTestId("mx_PreferencesUserSettingsTab").percySnapshotElement("User settings tab - Preferences", {
+            // Emulate TabbedView's actual min and max widths
+            // 580: '.mx_UserSettingsDialog .mx_TabbedView' min-width
+            // 796: 1036 (mx_TabbedView_tabsOnLeft actual width) - 240 (mx_TabbedView_tabPanel margin-right)
+            widths: [580, 796],
+        });
     });
 });

res/css/_components.pcss

@@ -343,9 +343,7 @@
 @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";
 @import "./views/settings/tabs/user/_SidebarUserSettingsTab.pcss";
 @import "./views/settings/tabs/user/_VoiceUserSettingsTab.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/_KeyboardUserSettingsTab.pcss

@@ -15,31 +15,26 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-.mx_KeyboardUserSettingsTab .mx_SettingsTab_section {
-    ul {
-        margin: 0;
-        padding: 0;
-    }
-
-    .mx_KeyboardShortcut_shortcutRow,
-    .mx_KeyboardShortcut {
-        display: flex;
-        justify-content: space-between;
-        align-items: center;
-    }
+.mx_KeyboardShortcut_shortcutList {
+    margin: 0;
+    padding: 0;
+    width: 100%;
+    display: grid;
+    grid-gap: $spacing-4;
+}
 
-    .mx_KeyboardShortcut_shortcutRow {
-        column-gap: $spacing-8;
-        margin-bottom: $spacing-4;
+.mx_KeyboardShortcut_shortcutRow,
+.mx_KeyboardShortcut {
+    display: flex;
+    justify-content: space-between;
+    align-items: center;
+}
 
-        /* TODO: Use flexbox */
-        &:last-of-type {
-            margin-bottom: 0;
-        }
+.mx_KeyboardShortcut_shortcutRow {
+    column-gap: $spacing-8;
+}
 
-        .mx_KeyboardShortcut {
-            flex-wrap: nowrap;
-            column-gap: 5px; /* TODO: Use a spacing variable */
-        }
-    }
+.mx_KeyboardShortcut {
+    flex-wrap: nowrap;
+    column-gap: $spacing-4;
 }

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
+    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>
 );
 

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>
 );

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

@@ -25,6 +25,9 @@ import {
     getKeyboardShortcutValue,
 } from "../../../../../accessibility/KeyboardShortcutUtils";
 import { KeyboardShortcut } from "../../KeyboardShortcut";
+import SettingsTab from "../SettingsTab";
+import { SettingsSection } from "../../shared/SettingsSection";
+import SettingsSubsection from "../../shared/SettingsSubsection";
 
 interface IKeyboardShortcutRowProps {
     name: KeyBindingAction;
@@ -57,26 +60,27 @@ const KeyboardShortcutSection: React.FC<IKeyboardShortcutSectionProps> = ({ cate
     if (!category.categoryLabel) return null;
 
     return (
-        <div className="mx_SettingsTab_section" key={categoryName}>
-            <div className="mx_SettingsTab_subheading">{_t(category.categoryLabel)}</div>
-            <ul>
-                {" "}
+        <SettingsSubsection heading={_t(category.categoryLabel)} key={categoryName}>
+            <ul className="mx_KeyboardShortcut_shortcutList">
                 {category.settingNames.map((shortcutName) => {
                     return <KeyboardShortcutRow key={shortcutName} name={shortcutName} />;
-                })}{" "}
+                })}
             </ul>
-        </div>
+        </SettingsSubsection>
     );
 };
 
 const KeyboardUserSettingsTab: React.FC = () => {
     return (
-        <div className="mx_SettingsTab mx_KeyboardUserSettingsTab">
-            <div className="mx_SettingsTab_heading">{_t("Keyboard")}</div>
-            {visibleCategories.map(([categoryName, category]: [CategoryName, ICategory]) => {
-                return <KeyboardShortcutSection key={categoryName} categoryName={categoryName} category={category} />;
-            })}
-        </div>
+        <SettingsTab>
+            <SettingsSection heading={_t("Keyboard")}>
+                {visibleCategories.map(([categoryName, category]: [CategoryName, ICategory]) => {
+                    return (
+                        <KeyboardShortcutSection key={categoryName} categoryName={categoryName} category={category} />
+                    );
+                })}
+            </SettingsSection>
+        </SettingsTab>
     );
 };
 

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>
         );
     }
 }