MM-62045 Enable eslint-plugin-jsx-a11y and fix occurrences of jsx-a11…

…y/anchor-has-content (mattermost#29453)

* Enable eslint-plugin-jsx-a11y and add standard rules as warnings or errors

* Fix jsx-a11y/anchor-has-content in BooleanSetting and add SettingSet

The invisible anchor was presumably supposed to let people jump from one
setting's help text to another setting. That didn't work for boolean
settings because there's no element with the ID of the setting. Instead
of an empty anchor, we needed to give something else that ID.

To improve the semantics of those settings, I also put those settings
into a fieldset with a legend containing the setting's label text
instead of using an actual label element as per how it's done on the
MDN. We'll probably end up using the SettingSet for other settings as
well.

Finally, I also fixed the link used by admin.service.useLetsEncryptDescription.disabled
to point to the right place and made it so that the link would open in
the current tab. Ideally, I'd also remove the Markdown from that help
text, but there's a lot here already.

* Fix jsx-a11y/anchor-has-content in CheckboxSetting

* Use SettingSet in PasswordSettings to improve semantics and layout

The main reason for doing this is to use a proper legend and fieldset
for these settings, but it also has the benefit of removing the extra
spacing from in between these settings.

* Change CopyText to use a button and require an aria-label for it

This fixes two ESLint warnings:
- This was failing jsx-a11y/anchor-has-content because it was an empty
  anchor element. I fixed that by giving it an aria-label matching the
tooltip of the CopyText, but that required that the label was a string,
so I had to change how CopyText is used. I also made the label required
so that people give an accessible explanation of what is being copied.
- This was also failing jsx-a11y/anchor-is-valid because it should be a
  button instead of a link. I applied the btn-link class and made it so
that that doesn't override the link's height which hopefully doesn't
cause problems anywhere else.

This is to fix jsx-a11y/anchor-has-content and jsx-a11y/anchor-is-valid
in the component. The ESLint plugin is complaining that this component
is using an anchor instead of a button element, so I changed that

* Turn jsx-a11y/anchor-has-content to be an error

* Update snapshots

* Remove lingering reference to nested prop which has been removed

* Changing system console password settings to a list and update padding for alerts in it

* Fix typo in SettingSet

* Update E2E tests to enable Elasticsearch by ID and remove duplicate enableElasticSearch helper

* Update E2E test to look for a legend instead of a label

* Fix typo in snapshot

e2e-tests/cypress/tests/integration/channels/autocomplete/helpers.ts

@@ -36,43 +36,6 @@ function createSearchData(prefix: string) {
     });
 }
 
-function enableElasticSearch() {
-    // # Enable elastic search via the API
-    cy.apiUpdateConfig({
-        ElasticsearchSettings: {
-            EnableIndexing: true,
-            EnableSearching: true,
-        },
-    } as Cypress.AdminConfig);
-
-    // # Navigate to the elastic search setting page
-    cy.visit('/admin_console/environment/elasticsearch');
-    cy.get('[data-testid="enableIndexing"] > .col-sm-8 > :nth-child(2)').click();
-
-    // * Test the connection and verify that we are successful
-    cy.contains('button', 'Test Connection').click();
-    cy.get('.alert-success').should('have.text', 'Test successful. Configuration saved.');
-
-    // # Index so we are up to date
-    cy.contains('button', 'Index Now').click();
-
-    // # Small wait to ensure new row is added
-    cy.wait(TIMEOUTS.ONE_SEC).get('.job-table__table').find('tbody > tr').eq(0).as('firstRow');
-
-    // * Newest row should eventually result in Success
-    const checkFirstRow = () => {
-        return cy.get('@firstRow').then((el) => {
-            return el.find('.status-icon-success').length > 0;
-        });
-    };
-    const options = {
-        timeout: TIMEOUTS.TWO_MIN,
-        interval: TIMEOUTS.TWO_SEC,
-        errorMsg: 'Reindex did not succeed in time',
-    };
-    cy.waitUntil(checkFirstRow, options);
-}
-
 function getTestUsers(prefix = ''): Record<string, SimpleUser> {
     if (Cypress.env('searchTestUsers')) {
         return JSON.parse(Cypress.env('searchTestUsers'));
@@ -281,7 +244,6 @@ export {
     createPrivateChannel,
     createPublicChannel,
     createSearchData,
-    enableElasticSearch,
     getTestUsers,
     getPostTextboxInput,
     getQuickChannelSwitcherInput,

e2e-tests/cypress/tests/integration/channels/enterprise/elasticsearch_autocomplete/helpers.ts

@@ -82,7 +82,7 @@ export function enableElasticSearch() {
 
     // # Navigate to the elastic search setting page
     cy.visit('/admin_console/environment/elasticsearch');
-    cy.get('[data-testid="enableIndexing"] > .col-sm-8 > :nth-child(2)').click();
+    cy.get('#enableIndexingtrue').click();
 
     // * Test the connection and verify that we are successful
     cy.contains('button', 'Test Connection').click();

e2e-tests/cypress/tests/integration/channels/enterprise/elasticsearch_autocomplete/system_console_spec.ts

@@ -29,7 +29,7 @@ describe('Elasticsearch system console', () => {
 
         // # Visit the Elasticsearch settings page
         cy.visit('/admin_console/environment/elasticsearch');
-        cy.get('[data-testid="enableIndexing"] > .col-sm-8 > :nth-child(2)').click();
+        cy.get('#enableIndexingtrue').click();
 
         // # Enable Auto complete
         cy.get('#enableAutocompletetrue').check().should('be.checked');

e2e-tests/cypress/tests/integration/channels/enterprise/elasticsearch_autocomplete/users_in_channel_switcher_spec.ts

@@ -12,7 +12,8 @@
 
 import {getRandomLetter} from '../../../../utils';
 import {doTestQuickChannelSwitcher} from '../../autocomplete/common_test';
-import {createSearchData, enableElasticSearch, SimpleUser} from '../../autocomplete/helpers';
+import {createSearchData, SimpleUser} from '../../autocomplete/helpers';
+import {enableElasticSearch} from './helpers';
 
 describe('Autocomplete with Elasticsearch - Users', () => {
     const prefix = getRandomLetter(3);

e2e-tests/cypress/tests/integration/channels/enterprise/elasticsearch_autocomplete/users_in_message_input_box_spec.ts

@@ -12,7 +12,8 @@
 
 import {getRandomLetter} from '../../../../utils';
 import {doTestPostextbox} from '../../autocomplete/common_test';
-import {createSearchData, enableElasticSearch, SimpleUser} from '../../autocomplete/helpers';
+import {createSearchData, SimpleUser} from '../../autocomplete/helpers';
+import {enableElasticSearch} from './helpers';
 
 describe('Autocomplete with Elasticsearch - Users', () => {
     const prefix = getRandomLetter(3);

e2e-tests/cypress/tests/integration/channels/enterprise/elasticsearch_autocomplete/users_spec.ts

@@ -13,7 +13,8 @@
 import {Team} from '@mattermost/types/teams';
 import {getRandomLetter} from '../../../../utils';
 import {doTestDMChannelSidebar, doTestUserChannelSection} from '../../autocomplete/common_test';
-import {createSearchData, enableElasticSearch, SimpleUser} from '../../autocomplete/helpers';
+import {createSearchData, SimpleUser} from '../../autocomplete/helpers';
+import {enableElasticSearch} from './helpers';
 
 describe('Autocomplete with Elasticsearch - Users', () => {
     const prefix = getRandomLetter(3);

e2e-tests/cypress/tests/integration/channels/enterprise/integrations/incoming_webhook_spec.ts

@@ -11,7 +11,7 @@
 // Group: @channels @enterprise @elasticsearch @incoming_webhook @not_cloud
 
 import * as TIMEOUTS from '../../../../fixtures/timeouts';
-import {enableElasticSearch} from '../../autocomplete/helpers';
+import {enableElasticSearch} from '../elasticsearch_autocomplete/helpers';
 
 describe('Incoming webhook', () => {
     let testTeam;

e2e-tests/cypress/tests/integration/channels/system_console/ui_and_api/customization_spec.js

@@ -138,7 +138,7 @@ describe('Customization', () => {
 
         cy.findByTestId('TeamSettings.EnableCustomBrand').should('be.visible').within(() => {
             // * Verify that setting is visible and matches text content
-            cy.get('label:first').should('be.visible').and('have.text', 'Enable Custom Branding: ');
+            cy.get('legend:contains("Enable Custom Branding: ")').should('be.visible');
 
             // * Verify that help setting is visible and matches text content
             const content = 'Enable custom branding to show an image of your choice, uploaded below, and some help text, written below, on the login page.';

webapp/channels/src/components/admin_console/__snapshots__/checkbox_setting.test.tsx.snap

@@ -2,32 +2,18 @@
 
 exports[`components/admin_console/CheckboxSetting should match snapshot 1`] = `
 <div>
-  <div
-    class="form-group"
-    data-testid="string.id"
-  >
-    <div
-      class="col-sm-12"
+  <div>
+    <label
+      class="checkbox-inline"
     >
-      <a
+      <input
+        data-testid="string.id"
         id="string.id"
+        name="string.id"
+        type="checkbox"
       />
-      <label
-        class="checkbox-inline"
-      >
-        <input
-          data-testid="string.id"
-          id="string.id"
-          name="string.id"
-          type="checkbox"
-        />
-        some label
-      </label>
-      <div
-        class="help-text"
-        data-testid="string.idhelp-text"
-      />
-    </div>
+      some label
+    </label>
   </div>
 </div>
 `;

webapp/channels/src/components/admin_console/__snapshots__/custom_enable_disable_guest_accounts_setting.test.tsx.snap

@@ -2,22 +2,19 @@
 
 exports[`components/AdminConsole/CustomEnableDisableGuestAccountsSetting renders correctly when disabled 1`] = `
 <div>
-  <div
+  <fieldset
     class="form-group"
     data-testid="MySetting"
+    id="MySetting"
   >
-    <label
-      class="control-label col-sm-4"
-      for="MySetting"
+    <legend
+      class="control-label form-legend col-sm-4"
     >
       Enable Guest Access: 
-    </label>
+    </legend>
     <div
       class="col-sm-8"
     >
-      <a
-        id="MySetting"
-      />
       <label
         class="Label-eIDBis glIJuw"
       >
@@ -56,28 +53,25 @@ exports[`components/AdminConsole/CustomEnableDisableGuestAccountsSetting renders
          for which roles can invite guests.
       </div>
     </div>
-  </div>
+  </fieldset>
 </div>
 `;
 
 exports[`components/AdminConsole/CustomEnableDisableGuestAccountsSetting renders correctly when enabled 1`] = `
 <div>
-  <div
+  <fieldset
     class="form-group"
     data-testid="MySetting"
+    id="MySetting"
   >
-    <label
-      class="control-label col-sm-4"
-      for="MySetting"
+    <legend
+      class="control-label form-legend col-sm-4"
     >
       Enable Guest Access: 
-    </label>
+    </legend>
     <div
       class="col-sm-8"
     >
-      <a
-        id="MySetting"
-      />
       <label
         class="Label-eIDBis glIJuw"
       >
@@ -116,6 +110,6 @@ exports[`components/AdminConsole/CustomEnableDisableGuestAccountsSetting renders
          for which roles can invite guests.
       </div>
     </div>
-  </div>
+  </fieldset>
 </div>
 `;

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

@@ -761,7 +761,7 @@ const AdminDefinition: AdminDefinitionType = {
                             key: 'ServiceSettings.UseLetsEncrypt',
                             label: defineMessage({id: 'admin.service.useLetsEncrypt', defaultMessage: 'Use Let\'s Encrypt:'}),
                             help_text: defineMessage({id: 'admin.service.useLetsEncryptDescription', defaultMessage: 'Enable the automatic retrieval of certificates from Let\'s Encrypt. The certificate will be retrieved when a client attempts to connect from a new domain. This will work with multiple domains.'}),
-                            disabled_help_text: defineMessage({id: 'admin.service.useLetsEncryptDescription.disabled', defaultMessage: "Enable the automatic retrieval of certificates from Let's Encrypt. The certificate will be retrieved when a client attempts to connect from a new domain. This will work with multiple domains. This setting cannot be enabled unless the [Forward port 80 to 443](#SystemSettings.Forward80To443) setting is set to true."}),
+                            disabled_help_text: defineMessage({id: 'admin.service.useLetsEncryptDescription.disabled', defaultMessage: "Enable the automatic retrieval of certificates from Let's Encrypt. The certificate will be retrieved when a client attempts to connect from a new domain. This will work with multiple domains. This setting cannot be enabled unless the [Forward port 80 to 443](#ServiceSettings.Forward80To443) setting is set to true."}),
                             disabled_help_text_markdown: true,
                             isDisabled: it.any(
                                 it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.ENVIRONMENT.WEB_SERVER)),

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

@@ -7,7 +7,7 @@ import styled from 'styled-components';
 
 import * as Utils from 'utils/utils';
 
-import Setting from './setting';
+import SettingSet from './setting_set';
 
 const Label = styled.label<{isDisabled: boolean}>`
     display: inline-flex;
@@ -121,13 +121,12 @@ const BooleanSetting = ({
     }, [id, onChange]);
 
     return (
-        <Setting
+        <SettingSet
+            helpText={helptext}
             inputId={id}
             label={label}
-            helpText={helptext}
             setByEnv={setByEnv}
         >
-            <a id={id}/>
             <Label isDisabled={disabled || setByEnv}>
                 <input
                     data-testid={id + 'true'}
@@ -154,7 +153,7 @@ const BooleanSetting = ({
                 />
                 {falseText}
             </Label>
-        </Setting>
+        </SettingSet>
     );
 };
 

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

@@ -3,7 +3,7 @@
 
 import React from 'react';
 
-import Setting from './setting';
+import SetByEnv from './set_by_env';
 
 type Props = {
     id: string;
@@ -12,8 +12,6 @@ type Props = {
     onChange: (id: string, foo: boolean) => void;
     disabled: boolean;
     setByEnv: boolean;
-    disabledText?: React.ReactNode;
-    helpText?: React.ReactNode;
 }
 
 export default class CheckboxSetting extends React.PureComponent<Props> {
@@ -26,29 +24,8 @@ export default class CheckboxSetting extends React.PureComponent<Props> {
     };
 
     public render() {
-        let helpText;
-        if (this.props.disabled && this.props.disabledText) {
-            helpText = (
-                <div>
-                    <span className='admin-console__disabled-text'>
-                        {this.props.disabledText}
-                    </span>
-                    {this.props.helpText}
-                </div>
-            );
-        } else {
-            helpText = this.props.helpText;
-        }
-
         return (
-            <Setting
-                inputId={this.props.id}
-                label={this.props.label}
-                helpText={helpText}
-                setByEnv={this.props.setByEnv}
-                nested={true}
-            >
-                <a id={this.props.id}/>
+            <div>
                 <label className='checkbox-inline'>
                     <input
                         data-testid={this.props.id}
@@ -61,7 +38,8 @@ export default class CheckboxSetting extends React.PureComponent<Props> {
                     />
                     {this.props.label}
                 </label>
-            </Setting>
+                {this.props.setByEnv ? <SetByEnv/> : null}
+            </div>
         );
     }
 }