From 40eefb0c674469c9d501a2dfb52dd5bf003ceeef Mon Sep 17 00:00:00 2001
From: Jay Sitter <jsit@users.noreply.github.com>
Date: Wed, 21 Jun 2023 08:27:27 -0400
Subject: [PATCH] fix(tabs): Fix tab semantics and a11y (#1382)

* fix: Fix tab semantics for Settings page

* fix: Use new tabpanel markup for admin settings

* fix: Remove unused currentTab behavior

* fix: Remove Bootstrap tab JS dependency

* fix: Add tabpanel role to rate limit tab panels

* fix: Fix style of tabs

---------

Co-authored-by: SleeplessOne1917 <abias1122@gmail.com>
Co-authored-by: Dessalines <dessalines@users.noreply.github.com>
---
 src/shared/components/common/tabs.tsx         |  26 ++--
 src/shared/components/home/admin-settings.tsx | 115 +++++++++++-------
 .../components/home/rate-limit-form.tsx       |  10 +-
 src/shared/components/person/settings.tsx     |  53 +++++---
 4 files changed, 135 insertions(+), 69 deletions(-)

diff --git a/src/shared/components/common/tabs.tsx b/src/shared/components/common/tabs.tsx
index 17980a47..3c2726a5 100644
--- a/src/shared/components/common/tabs.tsx
+++ b/src/shared/components/common/tabs.tsx
@@ -1,8 +1,9 @@
+import classNames from "classnames";
 import { Component, InfernoNode, linkEvent } from "inferno";
 
 interface TabItem {
   key: string;
-  getNode: () => InfernoNode;
+  getNode: (isSelected: boolean) => InfernoNode;
   label: string;
 }
 
@@ -30,24 +31,33 @@ export default class Tabs extends Component<TabsProps, TabsState> {
   render() {
     return (
       <div>
-        <ul className="nav nav-tabs mb-2">
+        <ul className="nav nav-tabs mb-2" role="tablist">
           {this.props.tabs.map(({ key, label }) => (
             <li key={key} className="nav-item">
               <button
                 type="button"
-                className={`nav-link btn${
-                  this.state?.currentTab === key ? " active" : ""
-                }`}
+                className={classNames("nav-link", {
+                  active: this.state?.currentTab === key,
+                })}
                 onClick={linkEvent({ ctx: this, tab: key }, handleSwitchTab)}
+                aria-controls={`${key}-tab-pane`}
+                {...(this.state?.currentTab === key && {
+                  ...{
+                    "aria-current": "page",
+                    "aria-selected": "true",
+                  },
+                })}
               >
                 {label}
               </button>
             </li>
           ))}
         </ul>
-        {this.props.tabs
-          .find(tab => tab.key === this.state?.currentTab)
-          ?.getNode()}
+        <div className="tab-content">
+          {this.props.tabs.map(({ key, getNode }) => {
+            return getNode(this.state?.currentTab === key);
+          })}
+        </div>
       </div>
     );
   }
diff --git a/src/shared/components/home/admin-settings.tsx b/src/shared/components/home/admin-settings.tsx
index 1de9f875..23454ab9 100644
--- a/src/shared/components/home/admin-settings.tsx
+++ b/src/shared/components/home/admin-settings.tsx
@@ -1,3 +1,4 @@
+import classNames from "classnames";
 import { Component, linkEvent } from "inferno";
 import {
   BannedPersonsResponse,
@@ -130,22 +131,30 @@ export class AdminSettings extends Component<any, AdminSettingsState> {
             {
               key: "site",
               label: i18n.t("site"),
-              getNode: () => (
-                <div className="row">
-                  <div className="col-12 col-md-6">
-                    <SiteForm
-                      showLocal={showLocal(this.isoData)}
-                      allowedInstances={federationData?.allowed}
-                      blockedInstances={federationData?.blocked}
-                      onSaveSite={this.handleEditSite}
-                      siteRes={this.state.siteRes}
-                      themeList={this.state.themeList}
-                      loading={this.state.loading}
-                    />
-                  </div>
-                  <div className="col-12 col-md-6">
-                    {this.admins()}
-                    {this.bannedUsers()}
+              getNode: isSelected => (
+                <div
+                  className={classNames("tab-pane show", {
+                    active: isSelected,
+                  })}
+                  role="tabpanel"
+                  id="site-tab-pane"
+                >
+                  <div className="row">
+                    <div className="col-12 col-md-6">
+                      <SiteForm
+                        showLocal={showLocal(this.isoData)}
+                        allowedInstances={federationData?.allowed}
+                        blockedInstances={federationData?.blocked}
+                        onSaveSite={this.handleEditSite}
+                        siteRes={this.state.siteRes}
+                        themeList={this.state.themeList}
+                        loading={this.state.loading}
+                      />
+                    </div>
+                    <div className="col-12 col-md-6">
+                      {this.admins()}
+                      {this.bannedUsers()}
+                    </div>
                   </div>
                 </div>
               ),
@@ -153,40 +162,64 @@ export class AdminSettings extends Component<any, AdminSettingsState> {
             {
               key: "rate_limiting",
               label: "Rate Limiting",
-              getNode: () => (
-                <RateLimitForm
-                  rateLimits={
-                    this.state.siteRes.site_view.local_site_rate_limit
-                  }
-                  onSaveSite={this.handleEditSite}
-                  loading={this.state.loading}
-                />
-              ),
-            },
-            {
-              key: "taglines",
-              label: i18n.t("taglines"),
-              getNode: () => (
-                <div className="row">
-                  <TaglineForm
-                    taglines={this.state.siteRes.taglines}
+              getNode: isSelected => (
+                <div
+                  className={classNames("tab-pane", {
+                    active: isSelected,
+                  })}
+                  role="tabpanel"
+                  id="rate_limiting-tab-pane"
+                >
+                  <RateLimitForm
+                    rateLimits={
+                      this.state.siteRes.site_view.local_site_rate_limit
+                    }
                     onSaveSite={this.handleEditSite}
                     loading={this.state.loading}
                   />
                 </div>
               ),
             },
+            {
+              key: "taglines",
+              label: i18n.t("taglines"),
+              getNode: isSelected => (
+                <div
+                  className={classNames("tab-pane", {
+                    active: isSelected,
+                  })}
+                  role="tabpanel"
+                  id="taglines-tab-pane"
+                >
+                  <div className="row">
+                    <TaglineForm
+                      taglines={this.state.siteRes.taglines}
+                      onSaveSite={this.handleEditSite}
+                      loading={this.state.loading}
+                    />
+                  </div>
+                </div>
+              ),
+            },
             {
               key: "emojis",
               label: i18n.t("emojis"),
-              getNode: () => (
-                <div className="row">
-                  <EmojiForm
-                    onCreate={this.handleCreateEmoji}
-                    onDelete={this.handleDeleteEmoji}
-                    onEdit={this.handleEditEmoji}
-                    loading={this.state.emojiLoading}
-                  />
+              getNode: isSelected => (
+                <div
+                  className={classNames("tab-pane", {
+                    active: isSelected,
+                  })}
+                  role="tabpanel"
+                  id="emojis-tab-pane"
+                >
+                  <div className="row">
+                    <EmojiForm
+                      onCreate={this.handleCreateEmoji}
+                      onDelete={this.handleDeleteEmoji}
+                      onEdit={this.handleEditEmoji}
+                      loading={this.state.emojiLoading}
+                    />
+                  </div>
                 </div>
               ),
             },
diff --git a/src/shared/components/home/rate-limit-form.tsx b/src/shared/components/home/rate-limit-form.tsx
index 1b2e281b..11c1a8e8 100644
--- a/src/shared/components/home/rate-limit-form.tsx
+++ b/src/shared/components/home/rate-limit-form.tsx
@@ -1,3 +1,4 @@
+import classNames from "classnames";
 import { Component, FormEventHandler, linkEvent } from "inferno";
 import { EditSite, LocalSiteRateLimit } from "lemmy-js-client";
 import { i18n } from "../../i18next";
@@ -19,6 +20,7 @@ interface RateLimitsProps {
   handleRateLimitPerSecond: FormEventHandler<HTMLInputElement>;
   rateLimitValue?: number;
   rateLimitPerSecondValue?: number;
+  className?: string;
 }
 
 interface RateLimitFormProps {
@@ -49,9 +51,10 @@ function RateLimits({
   handleRateLimitPerSecond,
   rateLimitPerSecondValue,
   rateLimitValue,
+  className,
 }: RateLimitsProps) {
   return (
-    <div className="mb-3 row">
+    <div role="tabpanel" className={classNames("mb-3 row", className)}>
       <div className="col-md-6">
         <label htmlFor="rate-limit">{i18n.t("rate_limit")}</label>
         <input
@@ -142,8 +145,11 @@ export default class RateLimitsForm extends Component<
           tabs={rateLimitTypes.map(rateLimitType => ({
             key: rateLimitType,
             label: i18n.t(`rate_limit_${rateLimitType}`),
-            getNode: () => (
+            getNode: isSelected => (
               <RateLimits
+                className={classNames("tab-pane show", {
+                  active: isSelected,
+                })}
                 handleRateLimit={linkEvent(
                   { rateLimitType, ctx: this },
                   handleRateLimitChange
diff --git a/src/shared/components/person/settings.tsx b/src/shared/components/person/settings.tsx
index 5c3fc345..9acba57a 100644
--- a/src/shared/components/person/settings.tsx
+++ b/src/shared/components/person/settings.tsx
@@ -1,4 +1,5 @@
 import { debounce } from "@utils/helpers";
+import classNames from "classnames";
 import { NoOptionI18nKeys } from "i18next";
 import { Component, linkEvent } from "inferno";
 import {
@@ -265,34 +266,50 @@ export class Settings extends Component<any, SettingsState> {
     );
   }
 
-  userSettings() {
+  userSettings(isSelected) {
     return (
-      <div className="row">
-        <div className="col-12 col-md-6">
-          <div className="card border-secondary mb-3">
-            <div className="card-body">{this.saveUserSettingsHtmlForm()}</div>
+      <div
+        className={classNames("tab-pane show", {
+          active: isSelected,
+        })}
+        role="tabpanel"
+        id="settings-tab-pane"
+      >
+        <div className="row">
+          <div className="col-12 col-md-6">
+            <div className="card border-secondary mb-3">
+              <div className="card-body">{this.saveUserSettingsHtmlForm()}</div>
+            </div>
           </div>
-        </div>
-        <div className="col-12 col-md-6">
-          <div className="card border-secondary mb-3">
-            <div className="card-body">{this.changePasswordHtmlForm()}</div>
+          <div className="col-12 col-md-6">
+            <div className="card border-secondary mb-3">
+              <div className="card-body">{this.changePasswordHtmlForm()}</div>
+            </div>
           </div>
         </div>
       </div>
     );
   }
 
-  blockCards() {
+  blockCards(isSelected) {
     return (
-      <div className="row">
-        <div className="col-12 col-md-6">
-          <div className="card border-secondary mb-3">
-            <div className="card-body">{this.blockUserCard()}</div>
+      <div
+        className={classNames("tab-pane", {
+          active: isSelected,
+        })}
+        role="tabpanel"
+        id="blocks-tab-pane"
+      >
+        <div className="row">
+          <div className="col-12 col-md-6">
+            <div className="card border-secondary mb-3">
+              <div className="card-body">{this.blockUserCard()}</div>
+            </div>
           </div>
-        </div>
-        <div className="col-12 col-md-6">
-          <div className="card border-secondary mb-3">
-            <div className="card-body">{this.blockCommunityCard()}</div>
+          <div className="col-12 col-md-6">
+            <div className="card border-secondary mb-3">
+              <div className="card-body">{this.blockCommunityCard()}</div>
+            </div>
           </div>
         </div>
       </div>