Put room settings form elements in fieldsets (#7311)

* override default fieldset styles Signed-off-by: Kerry Archibald <kerrya@element.io> * SettingsFieldset component Signed-off-by: Kerry Archibald <kerrya@element.io> * test settings fieldset Signed-off-by: Kerry Archibald <kerrya@element.io> * refactor SettingsFlag styles * use SettingsFieldset in room > securit settings * use fieldset in urlpreviewsettings Signed-off-by: Kerry Archibald <kerrya@element.io> * use SettingsFieldset in AliasSettings Signed-off-by: Kerry Archibald <kerrya@element.io> * fieldset in room > roles settings Signed-off-by: Kerry Archibald <kerrya@element.io> * css lint Signed-off-by: Kerry Archibald <kerrya@element.io> * run i18n Signed-off-by: Kerry Archibald <kerrya@element.io> * fussy order Signed-off-by: Kerry Archibald <kerrya@element.io> * default export Signed-off-by: Kerry Archibald <kerrya@element.io> * fix copyright headers Signed-off-by: Kerry Archibald <kerrya@element.io>
2021-12-09 18:44:22 +01:00 · 2021-12-09 18:44:22 +01:00 · 2e3f225520
commit 2e3f225520
parent cba92c0e90
16 changed files with 385 additions and 147 deletions
--- a/src/components/views/elements/SettingsFlag.tsx
+++ b/src/components/views/elements/SettingsFlag.tsx
@ -93,16 +93,18 @@ export default class SettingsFlag extends React.Component<IProps, IState> {
        } else {
            return (
                <div className="mx_SettingsFlag">
-                    <span className="mx_SettingsFlag_label">{ label }</span>
+                    <label className="mx_SettingsFlag_label">
+                        <span className="mx_SettingsFlag_labelText">{ label }</span>
+                        { description && <span className="mx_SettingsFlag_microcopy">
+                            { description }
+                        </span> }
+                    </label>
                    <ToggleSwitch
                        checked={this.state.value}
                        onChange={this.onChange}
                        disabled={this.props.disabled || !canChange}
                        aria-label={label}
                    />
-                    { description && <div className="mx_SettingsFlag_microcopy">
-                        { description }
-                    </div> }
                </div>
            );
        }
--- a/src/components/views/room_settings/AliasSettings.tsx
+++ b/src/components/views/room_settings/AliasSettings.tsx
@ -28,6 +28,7 @@ import Modal from "../../../Modal";
 import RoomPublishSetting from "./RoomPublishSetting";
 import { replaceableComponent } from "../../../utils/replaceableComponent";
 import RoomAliasField from "../elements/RoomAliasField";
+import SettingsFieldset from "../settings/SettingsFieldset";

 import { logger } from "matrix-js-sdk/src/logger";

@ -380,6 +381,17 @@ export default class AliasSettings extends React.Component<IProps, IState> {

        return (
            <div className='mx_AliasSettings'>
+                <SettingsFieldset
+                    legend={_t("Published Addresses")}
+                    description={<>
+                        { isSpaceRoom
+                            ? _t("Published addresses can be used by anyone on any server to join your space.")
+                            : _t("Published addresses can be used by anyone on any server to join your room.") }
+                        &nbsp;
+                        { _t("To publish an address, it needs to be set as a local address first.") }
+                    </>}
+                >
+                    { /*
                <span className='mx_SettingsTab_subheading'>{ _t("Published Addresses") }</span>
                <p>
                    { isSpaceRoom
@ -387,47 +399,46 @@ export default class AliasSettings extends React.Component<IProps, IState> {
                        : _t("Published addresses can be used by anyone on any server to join your room.") }
                    &nbsp;
                    { _t("To publish an address, it needs to be set as a local address first.") }
-                </p>
-                { canonicalAliasSection }
-                { this.props.hidePublishSetting
-                    ? null
-                    : <RoomPublishSetting
-                        roomId={this.props.roomId}
-                        canSetCanonicalAlias={this.props.canSetCanonicalAlias}
-                    /> }
-                <datalist id="mx_AliasSettings_altRecommendations">
-                    { this.getLocalNonAltAliases().map(alias => {
-                        return <option value={alias} key={alias} />;
-                    }) };
-                </datalist>
-                <EditableAliasesList
-                    id="roomAltAliases"
-                    items={this.state.altAliases}
-                    newItem={this.state.newAltAlias}
-                    onNewItemChanged={this.onNewAltAliasChanged}
-                    canRemove={this.props.canSetCanonicalAlias}
-                    canEdit={this.props.canSetCanonicalAlias}
-                    onItemAdded={this.onAltAliasAdded}
-                    onItemRemoved={this.onAltAliasDeleted}
-                    suggestionsListId="mx_AliasSettings_altRecommendations"
-                    itemsLabel={_t('Other published addresses:')}
-                    noItemsLabel={_t('No other published addresses yet, add one below')}
-                    placeholder={_t('New published address (e.g. #alias:server)')}
-                />
-                <span className='mx_SettingsTab_subheading mx_AliasSettings_localAliasHeader'>
-                    { _t("Local Addresses") }
-                </span>
-                <p>
-                    { isSpaceRoom
+                </p> */ }
+                    { canonicalAliasSection }
+                    { this.props.hidePublishSetting
+                        ? null
+                        : <RoomPublishSetting
+                            roomId={this.props.roomId}
+                            canSetCanonicalAlias={this.props.canSetCanonicalAlias}
+                        /> }
+                    <datalist id="mx_AliasSettings_altRecommendations">
+                        { this.getLocalNonAltAliases().map(alias => {
+                            return <option value={alias} key={alias} />;
+                        }) };
+                    </datalist>
+                    <EditableAliasesList
+                        id="roomAltAliases"
+                        items={this.state.altAliases}
+                        newItem={this.state.newAltAlias}
+                        onNewItemChanged={this.onNewAltAliasChanged}
+                        canRemove={this.props.canSetCanonicalAlias}
+                        canEdit={this.props.canSetCanonicalAlias}
+                        onItemAdded={this.onAltAliasAdded}
+                        onItemRemoved={this.onAltAliasDeleted}
+                        suggestionsListId="mx_AliasSettings_altRecommendations"
+                        itemsLabel={_t('Other published addresses:')}
+                        noItemsLabel={_t('No other published addresses yet, add one below')}
+                        placeholder={_t('New published address (e.g. #alias:server)')}
+                    />
+                </SettingsFieldset>
+                <SettingsFieldset legend={_t("Local Addresses")}
+                    description={isSpaceRoom
                        ? _t("Set addresses for this space so users can find this space " +
                            "through your homeserver (%(localDomain)s)", { localDomain })
                        : _t("Set addresses for this room so users can find this room " +
-                        "through your homeserver (%(localDomain)s)", { localDomain }) }
-                </p>
-                <details onToggle={this.onLocalAliasesToggled} open={this.state.detailsOpen}>
-                    <summary>{ this.state.detailsOpen ? _t('Show less') : _t("Show more") }</summary>
-                    { localAliasesList }
-                </details>
+                        "through your homeserver (%(localDomain)s)", { localDomain })}>
+                    <details onToggle={this.onLocalAliasesToggled} open={this.state.detailsOpen}>
+                        <summary>{ this.state.detailsOpen ? _t('Show less') : _t("Show more") }</summary>
+                        { localAliasesList }
+                    </details>
+                </SettingsFieldset>
+
            </div>
        );
    }
--- a/src/components/views/room_settings/UrlPreviewSettings.tsx
+++ b/src/components/views/room_settings/UrlPreviewSettings.tsx
@ -27,6 +27,7 @@ import { SettingLevel } from "../../../settings/SettingLevel";
 import { replaceableComponent } from "../../../utils/replaceableComponent";
 import { Room } from "matrix-js-sdk/src/models/room";
 import SettingsFlag from "../elements/SettingsFlag";
+import SettingsFieldset from '../settings/SettingsFieldset';

 interface IProps {
    room: Room;
@ -96,18 +97,19 @@ export default class UrlPreviewSettings extends React.Component<IProps> {
                roomId={roomId} />
        );

-        return (
-            <div>
-                <div className='mx_SettingsTab_subsectionText'>
-                    { _t('When someone puts a URL in their message, a URL preview can be shown to give more ' +
+        const description = <>
+            <p>
+                { _t('When someone puts a URL in their message, a URL preview can be shown to give more ' +
                        'information about that link such as the title, description, and an image from the website.') }
-                </div>
-                <div className='mx_SettingsTab_subsectionText'>
-                    { previewsForAccount }
-                </div>
+            </p>
+            <p>{ previewsForAccount }</p>
+        </>;
+
+        return (
+            <SettingsFieldset legend={_t("URL Previews")} description={description}>
                { previewsForRoom }
                <label>{ previewsForRoomAccount }</label>
-            </div>
+            </SettingsFieldset>
        );
    }
 }
--- a/src/components/views/settings/SettingsFieldset.tsx
+++ b/src/components/views/settings/SettingsFieldset.tsx
@ -0,0 +1,31 @@
+/*
+Copyright 2021 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+    http://www.apache.org/licenses/LICENSE-2.0
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+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, { ReactNode, HTMLAttributes } from 'react';
+import classNames from 'classnames';
+
+interface Props extends HTMLAttributes<HTMLFieldSetElement> {
+    // section title
+    legend: string | ReactNode;
+    description?: string | ReactNode;
+}
+
+const SettingsFieldset: React.FC<Props> = ({ legend, className, children, description, ...rest }) =>
+    <fieldset {...rest} className={classNames('mx_SettingsFieldset', className)}>
+        <legend className='mx_SettingsFieldset_legend'>{ legend }</legend>
+        { description && <div className='mx_SettingsFieldset_description'>{ description }</div> }
+        { children }
+    </fieldset>;
+
+export default SettingsFieldset;
--- a/src/components/views/settings/tabs/room/GeneralRoomSettingsTab.tsx
+++ b/src/components/views/settings/tabs/room/GeneralRoomSettingsTab.tsx
@ -66,15 +66,9 @@ export default class GeneralRoomSettingsTab extends React.Component<IProps, ISta
        const canChangeGroups = room.currentState.mayClientSendStateEvent("m.room.related_groups", client);
        const groupsEvent = room.currentState.getStateEvents("m.room.related_groups", "");

-        let urlPreviewSettings = <>
-            <span className='mx_SettingsTab_subheading'>{ _t("URL Previews") }</span>
-            <div className='mx_SettingsTab_section'>
-                <UrlPreviewSettings room={room} />
-            </div>
-        </>;
-        if (!SettingsStore.getValue(UIFeature.URLPreviews)) {
-            urlPreviewSettings = null;
-        }
+        const urlPreviewSettings = SettingsStore.getValue(UIFeature.URLPreviews) ?
+            <UrlPreviewSettings room={room} /> :
+            null;

        let flairSection;
        if (SettingsStore.getValue(UIFeature.Flair)) {
@ -110,14 +104,12 @@ export default class GeneralRoomSettingsTab extends React.Component<IProps, ISta
                </div>

                <div className="mx_SettingsTab_heading">{ _t("Room Addresses") }</div>
-                <div className='mx_SettingsTab_section mx_SettingsTab_subsectionText'>
-                    <AliasSettings
-                        roomId={this.props.roomId}
-                        canSetCanonicalAlias={canSetCanonical}
-                        canSetAliases={canSetAliases}
-                        canonicalAliasEvent={canonicalAliasEv}
-                    />
-                </div>
+                <AliasSettings
+                    roomId={this.props.roomId}
+                    canSetCanonicalAlias={canSetCanonical}
+                    canSetAliases={canSetAliases}
+                    canonicalAliasEvent={canonicalAliasEv}
+                />
                <div className="mx_SettingsTab_heading">{ _t("Other") }</div>
                { flairSection }
                { urlPreviewSettings }
--- a/src/components/views/settings/tabs/room/RolesRoomSettingsTab.tsx
+++ b/src/components/views/settings/tabs/room/RolesRoomSettingsTab.tsx
@ -27,6 +27,7 @@ import { RoomState } from "matrix-js-sdk/src/models/room-state";
 import { compare } from "../../../../../utils/strings";
 import ErrorDialog from '../../../dialogs/ErrorDialog';
 import PowerSelector from "../../../elements/PowerSelector";
+import SettingsFieldset from '../../SettingsFieldset';

 import { logger } from "matrix-js-sdk/src/logger";
 import SettingsStore from "../../../../../settings/SettingsStore";
@ -345,17 +346,15 @@ export default class RolesRoomSettingsTab extends React.Component<IProps> {

            if (privilegedUsers.length) {
                privilegedUsersSection =
-                    <div className='mx_SettingsTab_section mx_SettingsTab_subsectionText'>
-                        <div className='mx_SettingsTab_subheading'>{ _t('Privileged Users') }</div>
-                        { privilegedUsers }
-                    </div>;
+                <SettingsFieldset legend={_t('Privileged Users')}>
+                    { privilegedUsers }
+                </SettingsFieldset>;
            }
            if (mutedUsers.length) {
                mutedUsersSection =
-                    <div className='mx_SettingsTab_section mx_SettingsTab_subsectionText'>
-                        <div className='mx_SettingsTab_subheading'>{ _t('Muted Users') }</div>
+                    <SettingsFieldset legend={_t('Muted Users')}>
                        { mutedUsers }
-                    </div>;
+                    </SettingsFieldset>;
            }
        }

@ -364,8 +363,7 @@ export default class RolesRoomSettingsTab extends React.Component<IProps> {
        if (banned.length) {
            const canBanUsers = currentUserLevel >= banLevel;
            bannedUsersSection =
-                <div className='mx_SettingsTab_section mx_SettingsTab_subsectionText'>
-                    <div className='mx_SettingsTab_subheading'>{ _t('Banned users') }</div>
+                <SettingsFieldset legend={_t('Banned users')}>
                    <ul>
                        { banned.map((member) => {
                            const banEvent = member.events.member.getContent();
@ -383,7 +381,7 @@ export default class RolesRoomSettingsTab extends React.Component<IProps> {
                            );
                        }) }
                    </ul>
-                </div>;
+                </SettingsFieldset>;
        }

        const powerSelectors = Object.keys(powerLevelDescriptors).map((key, index) => {
@ -452,15 +450,17 @@ export default class RolesRoomSettingsTab extends React.Component<IProps> {
                { privilegedUsersSection }
                { mutedUsersSection }
                { bannedUsersSection }
-                <div className='mx_SettingsTab_section mx_SettingsTab_subsectionText'>
-                    <span className='mx_SettingsTab_subheading'>{ _t("Permissions") }</span>
-                    <p>{ isSpaceRoom
-                        ? _t('Select the roles required to change various parts of the space')
-                        : _t('Select the roles required to change various parts of the room')
-                    }</p>
+                <SettingsFieldset
+                    legend={_t("Permissions")}
+                    description={
+                        isSpaceRoom
+                            ? _t('Select the roles required to change various parts of the space')
+                            : _t('Select the roles required to change various parts of the room')
+                    }
+                >
                    { powerSelectors }
                    { eventPowerSelectors }
-                </div>
+                </SettingsFieldset>
            </div>
        );
    }
--- a/src/components/views/settings/tabs/room/SecurityRoomSettingsTab.tsx
+++ b/src/components/views/settings/tabs/room/SecurityRoomSettingsTab.tsx
@ -35,6 +35,7 @@ import createRoom, { IOpts } from '../../../../../createRoom';
 import CreateRoomDialog from '../../../dialogs/CreateRoomDialog';
 import JoinRuleSettings from "../../JoinRuleSettings";
 import ErrorDialog from "../../../dialogs/ErrorDialog";
+import SettingsFieldset from '../../SettingsFieldset';

 import { logger } from "matrix-js-sdk/src/logger";

@ -265,14 +266,11 @@ export default class SecurityRoomSettingsTab extends React.Component<IProps, ISt
                </div>
            );
        }
+        const description = _t("Decide who can join %(roomName)s.", {
+            roomName: room?.name,
+        });

-        return <div className="mx_SecurityRoomSettingsTab_joinRule">
-            <div className="mx_SettingsTab_subsectionText">
-                <span>{ _t("Decide who can join %(roomName)s.", {
-                    roomName: room?.name,
-                }) }</span>
-            </div>
-
+        return <SettingsFieldset legend={_t("Access")} description={description}>
            { aliasWarning }

            <JoinRuleSettings
@ -282,7 +280,7 @@ export default class SecurityRoomSettingsTab extends React.Component<IProps, ISt
                closeSettingsFn={this.props.closeSettingsFn}
                promptUpgrade={true}
            />
-        </div>;
+        </SettingsFieldset>;
    }

    private onJoinRuleChangeError = (error: Error) => {
@ -330,6 +328,10 @@ export default class SecurityRoomSettingsTab extends React.Component<IProps, ISt
    };

    private renderHistory() {
+        if (!SettingsStore.getValue(UIFeature.RoomHistorySettings)) {
+            return null;
+        }
+
        const client = MatrixClientPeg.get();
        const history = this.state.history;
        const state = client.getRoom(this.props.roomId).currentState;
@ -358,21 +360,18 @@ export default class SecurityRoomSettingsTab extends React.Component<IProps, ISt
            });
        }

-        return (
-            <div>
-                <div>
-                    { _t('Changes to who can read history will only apply to future messages in this room. ' +
-                        'The visibility of existing history will be unchanged.') }
-                </div>
-                <StyledRadioGroup
-                    name="historyVis"
-                    value={history}
-                    onChange={this.onHistoryRadioToggle}
-                    disabled={!canChangeHistory}
-                    definitions={options}
-                />
-            </div>
-        );
+        const description = _t('Changes to who can read history will only apply to future messages in this room. ' +
+        'The visibility of existing history will be unchanged.');
+
+        return (<SettingsFieldset legend={_t("Who can read history?")} description={description}>
+            <StyledRadioGroup
+                name="historyVis"
+                value={history}
+                onChange={this.onHistoryRadioToggle}
+                disabled={!canChangeHistory}
+                definitions={options}
+            />
+        </SettingsFieldset>);
    }

    private toggleAdvancedSection = () => {
@ -416,15 +415,7 @@ export default class SecurityRoomSettingsTab extends React.Component<IProps, ISt
            />;
        }

-        let historySection = (<>
-            <span className='mx_SettingsTab_subheading'>{ _t("Who can read history?") }</span>
-            <div className='mx_SettingsTab_section mx_SettingsTab_subsectionText'>
-                { this.renderHistory() }
-            </div>
-        </>);
-        if (!SettingsStore.getValue(UIFeature.RoomHistorySettings)) {
-            historySection = null;
-        }
+        const historySection = this.renderHistory();

        let advanced;
        if (room.getJoinRule() === JoinRule.Public) {
@ -446,26 +437,17 @@ export default class SecurityRoomSettingsTab extends React.Component<IProps, ISt
            <div className="mx_SettingsTab mx_SecurityRoomSettingsTab">
                <div className="mx_SettingsTab_heading">{ _t("Security & Privacy") }</div>

-                <span className='mx_SettingsTab_subheading'>{ _t("Encryption") }</span>
-                <div className='mx_SettingsTab_section mx_SecurityRoomSettingsTab_encryptionSection'>
-                    <div>
-                        <div className='mx_SettingsTab_subsectionText'>
-                            <span>{ _t("Once enabled, encryption cannot be disabled.") }</span>
-                        </div>
-                        <LabelledToggleSwitch
-                            value={isEncrypted}
-                            onChange={this.onEncryptionChange}
-                            label={_t("Encrypted")}
-                            disabled={!canEnableEncryption}
-                        />
-                    </div>
+                <SettingsFieldset legend={_t("Encryption")} description={_t("Once enabled, encryption cannot be disabled.")}>
+                    <LabelledToggleSwitch
+                        value={isEncrypted}
+                        onChange={this.onEncryptionChange}
+                        label={_t("Encrypted")}
+                        disabled={!canEnableEncryption}
+                    />
                    { encryptionSettings }
-                </div>
+                </SettingsFieldset>

-                <span className='mx_SettingsTab_subheading'>{ _t("Access") }</span>
-                <div className='mx_SettingsTab_section mx_SettingsTab_subsectionText'>
-                    { this.renderJoinRule() }
-                </div>
+                { this.renderJoinRule() }

                { advanced }
                { historySection }
--- a/src/components/views/spaces/SpaceSettingsVisibilityTab.tsx
+++ b/src/components/views/spaces/SpaceSettingsVisibilityTab.tsx
@ -98,15 +98,13 @@ const SpaceSettingsVisibilityTab = ({ matrixClient: cli, space, closeSettingsFn
    if (space.getJoinRule() === JoinRule.Public) {
        addressesSection = <>
            <span className="mx_SettingsTab_subheading">{ _t("Address") }</span>
-            <div className="mx_SettingsTab_section mx_SettingsTab_subsectionText">
-                <AliasSettings
-                    roomId={space.roomId}
-                    canSetCanonicalAlias={canSetCanonical}
-                    canSetAliases={true}
-                    canonicalAliasEvent={canonicalAliasEv}
-                    hidePublishSetting={true}
-                />
-            </div>
+            <AliasSettings
+                roomId={space.roomId}
+                canSetCanonicalAlias={canSetCanonical}
+                canSetAliases={true}
+                canonicalAliasEvent={canonicalAliasEv}
+                hidePublishSetting={true}
+            />
        </>;
    }