Wrap inputs in fieldsets in Space visibility settings (#7350)

Kerry · web-flow · commit b1066a56994c · 2022-01-06T10:09:26.000Z
* use fieldset in space settings &gt; visibility

Signed-off-by: Kerry Archibald &lt;kerrya@element.io&gt;

* add basic tests for space setttings visibility

Signed-off-by: Kerry Archibald &lt;kerrya@element.io&gt;

* i18n

Signed-off-by: Kerry Archibald &lt;kerrya@element.io&gt;

* fix toggle button placement

Signed-off-by: Kerry Archibald &lt;kerrya@element.io&gt;

* i18n

Signed-off-by: Kerry Archibald &lt;kerrya@element.io&gt;

* update settings separator color to quinary

Signed-off-by: Kerry Archibald &lt;kerrya@element.io&gt;
diff --git a/res/css/views/dialogs/_SpaceSettingsDialog.scss b/res/css/views/dialogs/_SpaceSettingsDialog.scss
@@ -38,7 +38,7 @@ limitations under the License.
         }
 
         & + .mx_SettingsTab_subheading {
-            border-top: 1px solid $quinary-content;
+            border-top: 1px solid $menu-border-color;
             margin-top: 0;
             padding-top: 24px;
         }
@@ -60,15 +60,6 @@ limitations under the License.
                 margin-left: 26px;
             }
         }
-
-        .mx_SettingsTab_showAdvanced {
-            margin: 16px 0;
-            padding: 0;
-        }
-
-        .mx_SettingsFlag {
-            margin-top: 24px;
-        }
     }
 
     .mx_SpaceSettingsDialog_buttons {
@@ -86,6 +77,11 @@ limitations under the License.
 
     .mx_AccessibleButton_hasKind {
         padding: 8px 22px;
+
+        &.mx_SettingsTab_showAdvanced {
+            margin: 16px 0;
+            padding: 0;
+        }
     }
 
     .mx_TabbedView_tabLabel {
diff --git a/res/css/views/settings/_JoinRuleSettings.scss b/res/css/views/settings/_JoinRuleSettings.scss
@@ -77,7 +77,7 @@ limitations under the License.
         color: $secondary-content;
 
         & + .mx_StyledRadioButton {
-            border-top: 1px solid $menu-border-color;
+            border-top: 1px solid $quinary-content;
         }
     }
 }
diff --git a/res/css/views/settings/_ProfileSettings.scss b/res/css/views/settings/_ProfileSettings.scss
@@ -54,7 +54,6 @@ limitations under the License.
 
 .mx_ProfileSettings_profileForm {
     @mixin mx_Settings_fullWidthField;
-    border-bottom: 1px solid $menu-border-color;
 }
 
 .mx_ProfileSettings_buttons {
diff --git a/res/css/views/settings/tabs/_SettingsTab.scss b/res/css/views/settings/tabs/_SettingsTab.scss
@@ -98,3 +98,7 @@ limitations under the License.
 .mx_SettingsTab a {
     color: $accent-alt;
 }
+
+.mx_SettingsTab_toggleWithDescription {
+    margin-top: 24px;
+}
diff --git a/src/components/views/room_settings/AliasSettings.tsx b/src/components/views/room_settings/AliasSettings.tsx
@@ -384,6 +384,7 @@ export default class AliasSettings extends React.Component<IProps, IState> {
         return (
             <div className='mx_AliasSettings'>
                 <SettingsFieldset
+                    data-test-id='published-address-fieldset'
                     legend={_t("Published Addresses")}
                     description={<>
                         { isSpaceRoom
@@ -429,7 +430,9 @@ export default class AliasSettings extends React.Component<IProps, IState> {
                         placeholder={_t('New published address (e.g. #alias:server)')}
                     />
                 </SettingsFieldset>
-                <SettingsFieldset legend={_t("Local Addresses")}
+                <SettingsFieldset
+                    data-test-id='local-address-fieldset'
+                    legend={_t("Local Addresses")}
                     description={isSpaceRoom
                         ? _t("Set addresses for this space so users can find this space " +
                             "through your homeserver (%(localDomain)s)", { localDomain })
diff --git a/src/components/views/spaces/SpaceSettingsVisibilityTab.tsx b/src/components/views/spaces/SpaceSettingsVisibilityTab.tsx
@@ -28,6 +28,7 @@ import LabelledToggleSwitch from "../elements/LabelledToggleSwitch";
 import { useLocalEcho } from "../../../hooks/useLocalEcho";
 import JoinRuleSettings from "../settings/JoinRuleSettings";
 import { useRoomState } from "../../../hooks/useRoomState";
+import SettingsFieldset from "../settings/SettingsFieldset";
 
 interface IProps {
     matrixClient: MatrixClient;
@@ -67,12 +68,16 @@ const SpaceSettingsVisibilityTab = ({ matrixClient: cli, space, closeSettingsFn
 
     let advancedSection;
     if (joinRule === JoinRule.Public) {
-        if (showAdvancedSection) {
-            advancedSection = <>
-                <AccessibleButton onClick={toggleAdvancedSection} kind="link" className="mx_SettingsTab_showAdvanced">
-                    { _t("Hide advanced") }
-                </AccessibleButton>
-
+        advancedSection = <div>
+            <AccessibleButton
+                data-test-id='toggle-guest-access-btn'
+                onClick={toggleAdvancedSection}
+                kind="link"
+                className="mx_SettingsTab_showAdvanced">
+                { showAdvancedSection ? _t("Hide advanced") : _t("Show advanced") }
+            </AccessibleButton>
+
+            { showAdvancedSection && <div className="mx_SettingsTab_toggleWithDescription">
                 <LabelledToggleSwitch
                     value={guestAccessEnabled}
                     onChange={setGuestAccessEnabled}
@@ -84,14 +89,9 @@ const SpaceSettingsVisibilityTab = ({ matrixClient: cli, space, closeSettingsFn
                     <br />
                     { _t("This may be useful for public spaces.") }
                 </p>
-            </>;
-        } else {
-            advancedSection = <>
-                <AccessibleButton onClick={toggleAdvancedSection} kind="link" className="mx_SettingsTab_showAdvanced">
-                    { _t("Show advanced") }
-                </AccessibleButton>
-            </>;
-        }
+            </div>
+            }
+        </div>;
     }
 
     let addressesSection;
@@ -111,34 +111,34 @@ const SpaceSettingsVisibilityTab = ({ matrixClient: cli, space, closeSettingsFn
     return <div className="mx_SettingsTab">
         <div className="mx_SettingsTab_heading">{ _t("Visibility") }</div>
 
-        { error && <div className="mx_SpaceRoomView_errorText">{ error }</div> }
-
-        <div className="mx_SettingsTab_section">
-            <div className="mx_SettingsTab_section_caption">
-                { _t("Decide who can view and join %(spaceName)s.", { spaceName: space.name }) }
-            </div>
-
-            <div>
-                <JoinRuleSettings
-                    room={space}
-                    onError={() => setError(_t("Failed to update the visibility of this space"))}
-                    closeSettingsFn={closeSettingsFn}
+        { error && <div data-test-id='space-settings-error' className="mx_SpaceRoomView_errorText">{ error }</div> }
+
+        <SettingsFieldset
+            data-test-id='access-fieldset'
+            legend={_t("Access")}
+            description={_t("Decide who can view and join %(spaceName)s.", { spaceName: space.name })}
+        >
+            <JoinRuleSettings
+                room={space}
+                onError={() => setError(_t("Failed to update the visibility of this space"))}
+                closeSettingsFn={closeSettingsFn}
+            />
+            { advancedSection }
+            <div className="mx_SettingsTab_toggleWithDescription">
+                <LabelledToggleSwitch
+                    value={historyVisibility === HistoryVisibility.WorldReadable}
+                    onChange={(checked: boolean) => {
+                        setHistoryVisibility(checked ? HistoryVisibility.WorldReadable : HistoryVisibility.Shared);
+                    }}
+                    disabled={!canSetHistoryVisibility}
+                    label={_t("Preview Space")}
                 />
+                <p>
+                    { _t("Allow people to preview your space before they join.") }<br />
+                    <b>{ _t("Recommended for public spaces.") }</b>
+                </p>
             </div>
-
-            { advancedSection }
-
-            <LabelledToggleSwitch
-                value={historyVisibility === HistoryVisibility.WorldReadable}
-                onChange={(checked: boolean) => {
-                    setHistoryVisibility(checked ? HistoryVisibility.WorldReadable : HistoryVisibility.Shared);
-                }}
-                disabled={!canSetHistoryVisibility}
-                label={_t("Preview Space")}
-            />
-            <div>{ _t("Allow people to preview your space before they join.") }</div>
-            <b>{ _t("Recommended for public spaces.") }</b>
-        </div>
+        </SettingsFieldset>
 
         { addressesSection }
     </div>;
diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json
@@ -1143,11 +1143,12 @@
     "Failed to update the guest access of this space": "Failed to update the guest access of this space",
     "Failed to update the history visibility of this space": "Failed to update the history visibility of this space",
     "Hide advanced": "Hide advanced",
+    "Show advanced": "Show advanced",
     "Enable guest access": "Enable guest access",
     "Guests can join a space without having an account.": "Guests can join a space without having an account.",
     "This may be useful for public spaces.": "This may be useful for public spaces.",
-    "Show advanced": "Show advanced",
     "Visibility": "Visibility",
+    "Access": "Access",
     "Decide who can view and join %(spaceName)s.": "Decide who can view and join %(spaceName)s.",
     "Failed to update the visibility of this space": "Failed to update the visibility of this space",
     "Preview Space": "Preview Space",
@@ -1580,7 +1581,6 @@
     "Once enabled, encryption for a room cannot be disabled. Messages sent in an encrypted room cannot be seen by the server, only by the participants of the room. Enabling encryption may prevent many bots and bridges from working correctly. <a>Learn more about encryption.</a>": "Once enabled, encryption for a room cannot be disabled. Messages sent in an encrypted room cannot be seen by the server, only by the participants of the room. Enabling encryption may prevent many bots and bridges from working correctly. <a>Learn more about encryption.</a>",
     "To link to this room, please add an address.": "To link to this room, please add an address.",
     "Decide who can join %(roomName)s.": "Decide who can join %(roomName)s.",
-    "Access": "Access",
     "Failed to update the join rules": "Failed to update the join rules",
     "Unknown failure": "Unknown failure",
     "Are you sure you want to make this encrypted room public?": "Are you sure you want to make this encrypted room public?",
diff --git a/test/components/views/spaces/SpaceSettingsVisibilityTab-test.tsx b/test/components/views/spaces/SpaceSettingsVisibilityTab-test.tsx
diff --git a/test/components/views/spaces/__snapshots__/SpaceSettingsVisibilityTab-test.tsx.snap b/test/components/views/spaces/__snapshots__/SpaceSettingsVisibilityTab-test.tsx.snap
diff --git a/test/test-utils.js b/test/test-utils.js

Original file line number	Diff line number	Diff line change
`@@ -38,7 +38,7 @@ limitations under the License.`
`38`	`38`	`}`
`39`	`39`
`40`	`40`	`& + .mx_SettingsTab_subheading {`
`41`		`- border-top: 1px solid $quinary-content;`
	`41`	`+ border-top: 1px solid $menu-border-color;`
`42`	`42`	`margin-top: 0;`
`43`	`43`	`padding-top: 24px;`
`44`	`44`	`}`
`@@ -60,15 +60,6 @@ limitations under the License.`
`60`	`60`	`margin-left: 26px;`
`61`	`61`	`}`
`62`	`62`	`}`
`63`		`-`
`64`		`- .mx_SettingsTab_showAdvanced {`
`65`		`- margin: 16px 0;`
`66`		`- padding: 0;`
`67`		`- }`
`68`		`-`
`69`		`- .mx_SettingsFlag {`
`70`		`- margin-top: 24px;`
`71`		`- }`
`72`	`63`	`}`
`73`	`64`
`74`	`65`	`.mx_SpaceSettingsDialog_buttons {`
`@@ -86,6 +77,11 @@ limitations under the License.`
`86`	`77`
`87`	`78`	`.mx_AccessibleButton_hasKind {`
`88`	`79`	`padding: 8px 22px;`
	`80`	`+`
	`81`	`+ &.mx_SettingsTab_showAdvanced {`
	`82`	`+ margin: 16px 0;`
	`83`	`+ padding: 0;`
	`84`	`+ }`
`89`	`85`	`}`
`90`	`86`
`91`	`87`	`.mx_TabbedView_tabLabel {`
Original file line number	Diff line number	Diff line change
`@@ -77,7 +77,7 @@ limitations under the License.`
`77`	`77`	`color: $secondary-content;`
`78`	`78`
`79`	`79`	`& + .mx_StyledRadioButton {`
`80`		`- border-top: 1px solid $menu-border-color;`
	`80`	`+ border-top: 1px solid $quinary-content;`
`81`	`81`	`}`
`82`	`82`	`}`
`83`	`83`	`}`
Original file line number	Diff line number	Diff line change
`@@ -54,7 +54,6 @@ limitations under the License.`
`54`	`54`
`55`	`55`	`.mx_ProfileSettings_profileForm {`
`56`	`56`	`@mixin mx_Settings_fullWidthField;`
`57`		`- border-bottom: 1px solid $menu-border-color;`
`58`	`57`	`}`
`59`	`58`
`60`	`59`	`.mx_ProfileSettings_buttons {`
Original file line number	Diff line number	Diff line change
`@@ -98,3 +98,7 @@ limitations under the License.`
`98`	`98`	`.mx_SettingsTab a {`
`99`	`99`	`color: $accent-alt;`
`100`	`100`	`}`
	`101`	`+`
	`102`	`+.mx_SettingsTab_toggleWithDescription {`
	`103`	`+ margin-top: 24px;`
	`104`	`+}`