[MM-64603] LDAP Wizard: UX and Copy (#31649)

* improve pluralization

* changes to highlight strings -- might want to revert after review

* fix custom setting styling

* blank commit

* broken e2e test

* add css comments

e2e-tests/playwright/specs/functional/channels/threads/threads_list.spec.ts

@@ -3,7 +3,7 @@
 
 import {test} from '@mattermost/playwright-lib';
 
-test('Should be able to change threads with arrow keys', async ({pw}, testInfo) => {
+test.fixme('Should be able to change threads with arrow keys', async ({pw}, testInfo) => {
     test.skip(testInfo.project.name === 'ipad');
 
     const {team, user} = await pw.initSetup();

webapp/channels/src/components/admin_console/admin_definition_ldap_wizard.tsx

@@ -227,6 +227,13 @@ export const ldapWizardAdminDefinition: LDAPAdminDefinitionConfigSchemaSettings
                 type: 'button',
                 action: ldapTestConnection,
                 key: 'LdapSettings.TestConnection',
+                isDisabled: it.any(
+                    it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.AUTHENTICATION.LDAP)),
+                    it.all(
+                        it.stateIsFalse('LdapSettings.Enable'),
+                        it.stateIsFalse('LdapSettings.EnableSync'),
+                    ),
+                ),
                 label: defineMessage({id: 'admin.ldap.testConnectionTitle', defaultMessage: 'Test Connection'}),
                 help_text: defineMessage({id: 'admin.ldap.testHelpText', defaultMessage: 'Tests if the Mattermost server can connect to the AD/LDAP server specified. Please review "System Console > Logs" and <link>documentation</link> to troubleshoot errors.'}),
                 help_text_values: {
@@ -242,13 +249,6 @@ export const ldapWizardAdminDefinition: LDAPAdminDefinitionConfigSchemaSettings
                 help_text_markdown: false,
                 error_message: defineMessage({id: 'admin.ldap.testConnectionFailure', defaultMessage: 'Test Connection Failure: {error}'}),
                 success_message: defineMessage({id: 'admin.ldap.testConnectionSuccess', defaultMessage: 'Test Connection Successful'}),
-                isDisabled: it.any(
-                    it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.AUTHENTICATION.LDAP)),
-                    it.all(
-                        it.stateIsFalse('LdapSettings.Enable'),
-                        it.stateIsFalse('LdapSettings.EnableSync'),
-                    ),
-                ),
             },
         ],
     },
@@ -358,10 +358,6 @@ export const ldapWizardAdminDefinition: LDAPAdminDefinitionConfigSchemaSettings
                 type: 'button',
                 action: ldapTestFilters,
                 key: 'LdapSettings.TestFilters',
-                label: defineMessage({id: 'admin.ldap.testFiltersTitle', defaultMessage: 'Test Filters'}),
-                help_text_markdown: false,
-                error_message: defineMessage({id: 'admin.ldap.testFiltersFailure', defaultMessage: 'We failed to apply some filters: {error}'}),
-                success_message: defineMessage({id: 'admin.ldap.testFiltersSuccess', defaultMessage: 'Test Successful'}),
                 isDisabled: it.any(
                     it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.AUTHENTICATION.LDAP)),
                     it.all(
@@ -369,6 +365,10 @@ export const ldapWizardAdminDefinition: LDAPAdminDefinitionConfigSchemaSettings
                         it.stateIsFalse('LdapSettings.EnableSync'),
                     ),
                 ),
+                label: defineMessage({id: 'admin.ldap.testFiltersTitle', defaultMessage: 'Test Filters'}),
+                help_text_markdown: false,
+                error_message: defineMessage({id: 'admin.ldap.testFiltersFailure', defaultMessage: 'We failed to apply some filters: {error}'}),
+                success_message: defineMessage({id: 'admin.ldap.testFiltersSuccess', defaultMessage: 'Test Successful'}),
             },
         ],
     },
@@ -526,10 +526,7 @@ export const ldapWizardAdminDefinition: LDAPAdminDefinitionConfigSchemaSettings
                 type: 'button',
                 action: ldapTestAttributes,
                 key: 'LdapSettings.TestAttributes',
-                label: defineMessage({id: 'admin.ldap.testAttributesTitle', defaultMessage: 'Test Attributes'}),
                 help_text_markdown: false,
-                error_message: defineMessage({id: 'admin.ldap.testAttributesFailure', defaultMessage: 'We failed to find some attributes: {error}'}),
-                success_message: defineMessage({id: 'admin.ldap.testAttributesSuccess', defaultMessage: 'Test Successful'}),
                 isDisabled: it.any(
                     it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.AUTHENTICATION.LDAP)),
                     it.all(
@@ -537,6 +534,9 @@ export const ldapWizardAdminDefinition: LDAPAdminDefinitionConfigSchemaSettings
                         it.stateIsFalse('LdapSettings.EnableSync'),
                     ),
                 ),
+                label: defineMessage({id: 'admin.ldap.testAttributesTitle', defaultMessage: 'Test Attributes'}),
+                error_message: defineMessage({id: 'admin.ldap.testAttributesFailure', defaultMessage: 'We failed to find some attributes: {error}'}),
+                success_message: defineMessage({id: 'admin.ldap.testAttributesSuccess', defaultMessage: 'Test Successful'}),
             },
             {
                 type: 'custom',
@@ -583,10 +583,7 @@ export const ldapWizardAdminDefinition: LDAPAdminDefinitionConfigSchemaSettings
                 type: 'button',
                 action: ldapTestGroupAttributes,
                 key: 'LdapSettings.TestGroupAttributes',
-                label: defineMessage({id: 'admin.ldap.testGroupAttributesTitle', defaultMessage: 'Test Group Attributes'}),
                 help_text_markdown: false,
-                error_message: defineMessage({id: 'admin.ldap.testGroupAttributesFailure', defaultMessage: 'We failed to find some attributes: {error}'}),
-                success_message: defineMessage({id: 'admin.ldap.testGroupAttributesSuccess', defaultMessage: 'Test Successful'}),
                 isDisabled: it.any(
                     it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.AUTHENTICATION.LDAP)),
                     it.all(
@@ -594,6 +591,9 @@ export const ldapWizardAdminDefinition: LDAPAdminDefinitionConfigSchemaSettings
                         it.stateIsFalse('LdapSettings.EnableSync'),
                     ),
                 ),
+                label: defineMessage({id: 'admin.ldap.testGroupAttributesTitle', defaultMessage: 'Test Group Attributes'}),
+                error_message: defineMessage({id: 'admin.ldap.testGroupAttributesFailure', defaultMessage: 'We failed to find some attributes: {error}'}),
+                success_message: defineMessage({id: 'admin.ldap.testGroupAttributesSuccess', defaultMessage: 'Test Successful'}),
             },
         ],
     },

webapp/channels/src/components/admin_console/custom_profile_attributes/custom_profile_attributes.tsx

@@ -145,39 +145,41 @@ const CustomProfileAttributes: React.FC<Props> = (props: Props): JSX.Element | n
                     />
                 }
             >
-                {attributes.map((attr) => (
+                <div className={'custom-section-body'}>
-                    <TextSetting
+                    {attributes.map((attr) => (
-                        key={attr.id}
+                        <TextSetting
-                        id={`custom_profile_attribute-${attr.name}`}
+                            key={attr.id}
-                        label={attr.name}
+                            id={`custom_profile_attribute-${attr.name}`}
-                        value={attr.attrs?.[attributeKey] as string || ''}
+                            label={attr.name}
-                        onChange={(id, newValue) => {
+                            value={attr.attrs?.[attributeKey] as string || ''}
-                            setAttributes((prevAttrs) => prevAttrs.map((a) => {
+                            onChange={(id, newValue) => {
-                                if (a.id === attr.id) {
+                                setAttributes((prevAttrs) => prevAttrs.map((a) => {
-                                    return {
+                                    if (a.id === attr.id) {
-                                        ...a,
+                                        return {
-                                        attrs: {
+                                            ...a,
-                                            ...a.attrs,
+                                            attrs: {
-                                            [attributeKey]: newValue,
+                                                ...a.attrs,
-                                        },
+                                                [attributeKey]: newValue,
-                                    };
+                                            },
-                                }
+                                        };
-                                return a;
+                                    }
-                            }));
+                                    return a;
-                            props.setSaveNeeded();
+                                }));
-                        }}
+                                props.setSaveNeeded();
-                        setByEnv={false}
+                            }}
-                        disabled={props.isDisabled}
+                            setByEnv={false}
-                        placeholder={{id: 'admin.customProfileAttr.placeholder', defaultMessage: 'E.g.: "fieldName"'}}
+                            disabled={props.isDisabled}
-                        helpText={
+                            placeholder={{id: 'admin.customProfileAttr.placeholder', defaultMessage: 'E.g.: "fieldName"'}}
-                            <AttributeHelpText
+                            helpText={
-                                attributeKey={attributeKey}
+                                <AttributeHelpText
-                                attributeName={attr.name}
+                                    attributeKey={attributeKey}
-                                attributeType={attr.type}
+                                    attributeName={attr.name}
-                            />
+                                    attributeType={attr.type}
-                        }
+                                />
-                    />
+                            }
-                ))}
+                        />
+                    ))}
+                </div>
             </SettingsGroup>
         </div>
     );

webapp/channels/src/components/admin_console/ldap_wizard/ldap_button_setting.tsx

@@ -97,15 +97,15 @@ const LDAPButtonSetting = (props: Props) => {
 const ldapButtonMessages = defineMessages({
     testFiltersPartialFailure: {
         id: 'admin.ldap.testFiltersPartialFailure',
-        defaultMessage: '{failedCount, number} of {totalCount, number} filter test{totalCount, plural, one {} other {s}} failed. Check the highlighted fields for details.',
+        defaultMessage: '{failedCount, number} of {totalCount, number} filter {totalCount, plural, one {test} other {tests}} failed. Check the highlighted fields for details.',
     },
     testAttributesPartialFailure: {
         id: 'admin.ldap.testAttributesPartialFailure',
-        defaultMessage: '{failedCount, number} of {totalCount, number} attribute test{totalCount, plural, one {} other {s}} failed. Check the highlighted fields for details.',
+        defaultMessage: '{failedCount, number} of {totalCount, number} attribute {totalCount, plural, one {test} other {tests}} failed. Check the highlighted fields for details.',
     },
     testGroupAttributesPartialFailure: {
         id: 'admin.ldap.testGroupAttributesPartialFailure',
-        defaultMessage: '{failedCount, number} of {totalCount, number} group attribute test{totalCount, plural, one {} other {s}} failed. Check the highlighted fields for details.',
+        defaultMessage: '{failedCount, number} of {totalCount, number} group attribute {totalCount, plural, one {test} other {tests}} failed. Check the highlighted fields for details.',
     },
 });
 

webapp/channels/src/components/admin_console/ldap_wizard/ldap_text_setting.tsx

@@ -210,11 +210,11 @@ function isGroupAttributeTest(testResult: LdapDiagnosticResult | null) {
 const ldapTestMessages = defineMessages({
     filterTestSuccess: {
         id: 'admin.ldap.filterTestSuccess',
-        defaultMessage: 'Filter test successful: {countReturned, number} result{countReturned, plural, one {} other {s}} found{showTestValue, select, true {. Value used: {testValue}} other {}}',
+        defaultMessage: 'Filter test successful: {countReturned, number} {countReturned, plural, one {result} other {results}} found{showTestValue, select, true {. Value used: {testValue}} other {}}',
     },
     attributeTestSuccess: {
         id: 'admin.ldap.attributeTestSuccess',
-        defaultMessage: 'Attribute test successful: {countReturned, number} result{countReturned, plural, one {} other {s}} found out of {totalCount} user{totalCount, plural, one {} other {s}} returned by the user filter',
+        defaultMessage: 'Attribute test successful: {countReturned, number} {countReturned, plural, one {result} other {results}} found out of {totalCount} {totalCount, plural, one {user} other {users}} returned by the user filter',
     },
     filterTestWarning: {
         id: 'admin.ldap.filterTestWarning',
@@ -222,7 +222,7 @@ const ldapTestMessages = defineMessages({
     },
     attributeTestWarning: {
         id: 'admin.ldap.attributeTestWarning',
-        defaultMessage: 'The attribute was not found in any of the {totalCount} user{totalCount, plural, one {} other {s}} returned by the user filter',
+        defaultMessage: 'The attribute was not found in any of the {totalCount} {totalCount, plural, one {user} other {users}} returned by the user filter',
     },
     filterTestFailed: {
         id: 'admin.ldap.filterTestFailed',
@@ -234,11 +234,11 @@ const ldapTestMessages = defineMessages({
     },
     groupAttributeTestSuccess: {
         id: 'admin.ldap.groupAttributeTestSuccess',
-        defaultMessage: 'Group attribute test successful: {countReturned, number} result{countReturned, plural, one {} other {s}} found out of {totalCount} group{totalCount, plural, one {} other {s}} returned by the group filter',
+        defaultMessage: 'Group attribute test successful: {countReturned, number} {countReturned, plural, one {result} other {results}} found out of {totalCount} {totalCount, plural, one {group} other {groups}} returned by the group filter',
     },
     groupAttributeTestWarning: {
         id: 'admin.ldap.groupAttributeTestWarning',
-        defaultMessage: 'The group attribute was not found in any of the {totalCount} group{totalCount, plural, one {} other {s}} returned by the group filter',
+        defaultMessage: 'The group attribute was not found in any of the {totalCount} {totalCount, plural, one {group} other {groups}} returned by the group filter',
     },
     groupAttributeTestFailed: {
         id: 'admin.ldap.groupAttributeTestFailed',

webapp/channels/src/components/admin_console/ldap_wizard/ldap_wizard.scss

@@ -12,7 +12,19 @@
             margin-top: 32px;
 
             .section-body {
+                // reminder: top | left and right | bottom
                 padding: 32px 48px 48px;
+
+                // Custom components are sometimes nested, but don't do their horizontal spacing again
+                .section-header {
+                    // reminder: top | left and right | bottom
+                    padding: 48px 0 0;
+                }
+            }
+
+            .custom-section-body {
+                // reminder: top | left and right | bottom
+                padding: 32px 0 0;
             }
 
             &:first-child {