From 520659242b799f0574ea535834e79dcd58cc4bab Mon Sep 17 00:00:00 2001
From: Kerry <kerrya@element.io>
Date: Mon, 29 May 2023 10:20:44 +1200
Subject: [PATCH] Fix spacing regression in user settings - roles & permissions
 (#10993)

* add content container to settingsfieldset

* fix spacing
---
 res/css/views/settings/_JoinRuleSettings.pcss |   1 -
 res/css/views/settings/_ProfileSettings.pcss  |   2 +
 res/css/views/settings/_SettingsFieldset.pcss |   6 +
 .../views/settings/SettingsFieldset.tsx       |   2 +-
 .../tabs/room/RolesRoomSettingsTab.tsx        |   2 +-
 .../SettingsFieldset-test.tsx.snap            |  24 ++-
 .../RolesRoomSettingsTab-test.tsx.snap        |  32 ++--
 .../SecurityRoomSettingsTab-test.tsx.snap     | 164 ++++++++---------
 .../SpaceSettingsVisibilityTab-test.tsx.snap  | 166 +++++++++---------
 9 files changed, 215 insertions(+), 184 deletions(-)

diff --git a/res/css/views/settings/_JoinRuleSettings.pcss b/res/css/views/settings/_JoinRuleSettings.pcss
index 18c4395efec..92721f3accc 100644
--- a/res/css/views/settings/_JoinRuleSettings.pcss
+++ b/res/css/views/settings/_JoinRuleSettings.pcss
@@ -57,7 +57,6 @@ limitations under the License.
 
 .mx_JoinRuleSettings_radioButton {
     padding-top: 16px;
-    margin-bottom: 8px;
 
     .mx_StyledRadioButton_content {
         margin-left: 14px;
diff --git a/res/css/views/settings/_ProfileSettings.pcss b/res/css/views/settings/_ProfileSettings.pcss
index 86b6835dc81..4dc81689455 100644
--- a/res/css/views/settings/_ProfileSettings.pcss
+++ b/res/css/views/settings/_ProfileSettings.pcss
@@ -34,6 +34,8 @@ limitations under the License.
             }
 
             .mx_ProfileSettings_profile_controls_topic {
+                margin-top: $spacing-8;
+
                 & > textarea {
                     font-family: inherit;
                     resize: vertical;
diff --git a/res/css/views/settings/_SettingsFieldset.pcss b/res/css/views/settings/_SettingsFieldset.pcss
index 5acf3754a75..3443ba79704 100644
--- a/res/css/views/settings/_SettingsFieldset.pcss
+++ b/res/css/views/settings/_SettingsFieldset.pcss
@@ -43,3 +43,9 @@ limitations under the License.
         }
     }
 }
+
+.mx_SettingsFieldset_content {
+    display: flex;
+    flex-direction: column;
+    gap: $spacing-8;
+}
diff --git a/src/components/views/settings/SettingsFieldset.tsx b/src/components/views/settings/SettingsFieldset.tsx
index 617564aed3d..d63b5bf787f 100644
--- a/src/components/views/settings/SettingsFieldset.tsx
+++ b/src/components/views/settings/SettingsFieldset.tsx
@@ -31,7 +31,7 @@ const SettingsFieldset: React.FC<Props> = ({ legend, className, children, descri
                 <SettingsSubsectionText>{description}</SettingsSubsectionText>
             </div>
         )}
-        {children}
+        <div className="mx_SettingsFieldset_content">{children}</div>
     </fieldset>
 );
 
diff --git a/src/components/views/settings/tabs/room/RolesRoomSettingsTab.tsx b/src/components/views/settings/tabs/room/RolesRoomSettingsTab.tsx
index 0ed44cf173c..eb8a4ec083a 100644
--- a/src/components/views/settings/tabs/room/RolesRoomSettingsTab.tsx
+++ b/src/components/views/settings/tabs/room/RolesRoomSettingsTab.tsx
@@ -466,7 +466,7 @@ export default class RolesRoomSettingsTab extends React.Component<IProps> {
                     label = _t("Send %(eventType)s events", { eventType });
                 }
                 return (
-                    <div className="" key={eventType}>
+                    <div key={eventType}>
                         <PowerSelector
                             label={label}
                             value={eventsLevels[eventType]}
diff --git a/test/components/views/settings/__snapshots__/SettingsFieldset-test.tsx.snap b/test/components/views/settings/__snapshots__/SettingsFieldset-test.tsx.snap
index 968308763a1..4ac3bbb1e12 100644
--- a/test/components/views/settings/__snapshots__/SettingsFieldset-test.tsx.snap
+++ b/test/components/views/settings/__snapshots__/SettingsFieldset-test.tsx.snap
@@ -20,8 +20,12 @@ exports[`<SettingsFieldset /> renders fieldset with plain text description 1`] =
         Changes to who can read history.
       </div>
     </div>
-    <div>
-      test
+    <div
+      class="mx_SettingsFieldset_content"
+    >
+      <div>
+        test
+      </div>
     </div>
   </fieldset>
 </DocumentFragment>
@@ -54,8 +58,12 @@ exports[`<SettingsFieldset /> renders fieldset with react description 1`] = `
         </a>
       </div>
     </div>
-    <div>
-      test
+    <div
+      class="mx_SettingsFieldset_content"
+    >
+      <div>
+        test
+      </div>
     </div>
   </fieldset>
 </DocumentFragment>
@@ -72,8 +80,12 @@ exports[`<SettingsFieldset /> renders fieldset without description 1`] = `
     >
       Who can read history?
     </legend>
-    <div>
-      test
+    <div
+      class="mx_SettingsFieldset_content"
+    >
+      <div>
+        test
+      </div>
     </div>
   </fieldset>
 </DocumentFragment>
diff --git a/test/components/views/settings/tabs/room/__snapshots__/RolesRoomSettingsTab-test.tsx.snap b/test/components/views/settings/tabs/room/__snapshots__/RolesRoomSettingsTab-test.tsx.snap
index aea48398946..e5893633090 100644
--- a/test/components/views/settings/tabs/room/__snapshots__/RolesRoomSettingsTab-test.tsx.snap
+++ b/test/components/views/settings/tabs/room/__snapshots__/RolesRoomSettingsTab-test.tsx.snap
@@ -9,20 +9,24 @@ exports[`RolesRoomSettingsTab Banned users renders banned users 1`] = `
   >
     Banned users
   </legend>
-  <ul
-    class="mx_RolesRoomSettingsTab_bannedList"
+  <div
+    class="mx_SettingsFieldset_content"
   >
-    <li>
-      <span
-        title="Banned by @alice:server.org"
-      >
-        <strong>
-          @bob:server.org
-        </strong>
-         
-         Reason: just testing
-      </span>
-    </li>
-  </ul>
+    <ul
+      class="mx_RolesRoomSettingsTab_bannedList"
+    >
+      <li>
+        <span
+          title="Banned by @alice:server.org"
+        >
+          <strong>
+            @bob:server.org
+          </strong>
+           
+           Reason: just testing
+        </span>
+      </li>
+    </ul>
+  </div>
 </fieldset>
 `;
diff --git a/test/components/views/settings/tabs/room/__snapshots__/SecurityRoomSettingsTab-test.tsx.snap b/test/components/views/settings/tabs/room/__snapshots__/SecurityRoomSettingsTab-test.tsx.snap
index 28e43821763..d3642a1a7e3 100644
--- a/test/components/views/settings/tabs/room/__snapshots__/SecurityRoomSettingsTab-test.tsx.snap
+++ b/test/components/views/settings/tabs/room/__snapshots__/SecurityRoomSettingsTab-test.tsx.snap
@@ -18,91 +18,95 @@ exports[`<SecurityRoomSettingsTab /> history visibility uses shared as default h
       Changes to who can read history will only apply to future messages in this room. The visibility of existing history will be unchanged.
     </div>
   </div>
-  <label
-    class="mx_StyledRadioButton mx_StyledRadioButton_enabled"
+  <div
+    class="mx_SettingsFieldset_content"
   >
-    <input
-      id="historyVis-world_readable"
-      name="historyVis"
-      type="radio"
-      value="world_readable"
-    />
-    <div>
-      <div />
-    </div>
-    <div
-      class="mx_StyledRadioButton_content"
+    <label
+      class="mx_StyledRadioButton mx_StyledRadioButton_enabled"
     >
-      Anyone
-    </div>
-    <div
-      class="mx_StyledRadioButton_spacer"
-    />
-  </label>
-  <label
-    class="mx_StyledRadioButton mx_StyledRadioButton_enabled mx_StyledRadioButton_checked"
-  >
-    <input
-      checked=""
-      id="historyVis-shared"
-      name="historyVis"
-      type="radio"
-      value="shared"
-    />
-    <div>
-      <div />
-    </div>
-    <div
-      class="mx_StyledRadioButton_content"
+      <input
+        id="historyVis-world_readable"
+        name="historyVis"
+        type="radio"
+        value="world_readable"
+      />
+      <div>
+        <div />
+      </div>
+      <div
+        class="mx_StyledRadioButton_content"
+      >
+        Anyone
+      </div>
+      <div
+        class="mx_StyledRadioButton_spacer"
+      />
+    </label>
+    <label
+      class="mx_StyledRadioButton mx_StyledRadioButton_enabled mx_StyledRadioButton_checked"
     >
-      Members only (since the point in time of selecting this option)
-    </div>
-    <div
-      class="mx_StyledRadioButton_spacer"
-    />
-  </label>
-  <label
-    class="mx_StyledRadioButton mx_StyledRadioButton_enabled"
-  >
-    <input
-      id="historyVis-invited"
-      name="historyVis"
-      type="radio"
-      value="invited"
-    />
-    <div>
-      <div />
-    </div>
-    <div
-      class="mx_StyledRadioButton_content"
+      <input
+        checked=""
+        id="historyVis-shared"
+        name="historyVis"
+        type="radio"
+        value="shared"
+      />
+      <div>
+        <div />
+      </div>
+      <div
+        class="mx_StyledRadioButton_content"
+      >
+        Members only (since the point in time of selecting this option)
+      </div>
+      <div
+        class="mx_StyledRadioButton_spacer"
+      />
+    </label>
+    <label
+      class="mx_StyledRadioButton mx_StyledRadioButton_enabled"
     >
-      Members only (since they were invited)
-    </div>
-    <div
-      class="mx_StyledRadioButton_spacer"
-    />
-  </label>
-  <label
-    class="mx_StyledRadioButton mx_StyledRadioButton_enabled"
-  >
-    <input
-      id="historyVis-joined"
-      name="historyVis"
-      type="radio"
-      value="joined"
-    />
-    <div>
-      <div />
-    </div>
-    <div
-      class="mx_StyledRadioButton_content"
+      <input
+        id="historyVis-invited"
+        name="historyVis"
+        type="radio"
+        value="invited"
+      />
+      <div>
+        <div />
+      </div>
+      <div
+        class="mx_StyledRadioButton_content"
+      >
+        Members only (since they were invited)
+      </div>
+      <div
+        class="mx_StyledRadioButton_spacer"
+      />
+    </label>
+    <label
+      class="mx_StyledRadioButton mx_StyledRadioButton_enabled"
     >
-      Members only (since they joined)
-    </div>
-    <div
-      class="mx_StyledRadioButton_spacer"
-    />
-  </label>
+      <input
+        id="historyVis-joined"
+        name="historyVis"
+        type="radio"
+        value="joined"
+      />
+      <div>
+        <div />
+      </div>
+      <div
+        class="mx_StyledRadioButton_content"
+      >
+        Members only (since they joined)
+      </div>
+      <div
+        class="mx_StyledRadioButton_spacer"
+      />
+    </label>
+  </div>
 </fieldset>
 `;
 
diff --git a/test/components/views/spaces/__snapshots__/SpaceSettingsVisibilityTab-test.tsx.snap b/test/components/views/spaces/__snapshots__/SpaceSettingsVisibilityTab-test.tsx.snap
index 9ce736cb817..b60e4fb9b31 100644
--- a/test/components/views/spaces/__snapshots__/SpaceSettingsVisibilityTab-test.tsx.snap
+++ b/test/components/views/spaces/__snapshots__/SpaceSettingsVisibilityTab-test.tsx.snap
@@ -52,98 +52,102 @@ exports[`<SpaceSettingsVisibilityTab /> renders container 1`] = `
                 Decide who can view and join mock-space.
               </div>
             </div>
-            <label
-              class="mx_StyledRadioButton mx_JoinRuleSettings_radioButton mx_StyledRadioButton_disabled mx_StyledRadioButton_checked"
+            <div
+              class="mx_SettingsFieldset_content"
             >
-              <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"
+              <label
+                class="mx_StyledRadioButton mx_JoinRuleSettings_radioButton mx_StyledRadioButton_disabled mx_StyledRadioButton_checked"
               >
-                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"
+                <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"
               >
-                Public
-              </div>
-              <div
-                class="mx_StyledRadioButton_spacer"
-              />
-            </label>
-            <span
-              id="joinRule-public-description"
-            >
-              Anyone can find and join.
-            </span>
-            <div
-              class="mx_SettingsTab_toggleWithDescription"
-            >
+                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_SettingsFlag"
+                class="mx_SettingsTab_toggleWithDescription"
               >
-                <span
-                  class="mx_SettingsFlag_label"
+                <div
+                  class="mx_SettingsFlag"
                 >
+                  <span
+                    class="mx_SettingsFlag_label"
+                  >
+                    <div
+                      id="mx_LabelledToggleSwitch_testid_0"
+                    >
+                      Preview Space
+                    </div>
+                  </span>
                   <div
-                    id="mx_LabelledToggleSwitch_testid_0"
+                    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"
                   >
-                    Preview Space
+                    <div
+                      class="mx_ToggleSwitch_ball"
+                    />
                   </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>
+                <p>
+                  Allow people to preview your space before they join.
+                  <br />
+                  <b>
+                    Recommended for public spaces.
+                  </b>
+                </p>
               </div>
-              <p>
-                Allow people to preview your space before they join.
-                <br />
-                <b>
-                  Recommended for public spaces.
-                </b>
-              </p>
             </div>
           </fieldset>
         </div>