[Search][a11y] Fixing connectors pageHeader hierarchy content

[JoseLuisGJ]

Replace the existing ConnectorNameAndDescription component with separate ConnectorName and ConnectorDescription components for improved accessibility as pointed out in this issue #198001 . Now only the H1 wraps the Title and the Descriptions is out of it.

Before:

After:

[elasticmachine]

Pinging @elastic/kibana-accessibility (Project:Accessibility)

[navarone-feekery]

Small request; can you provide a gif of you interacting with the title/description just to confirm the functionality still works?

[JoseLuisGJ]

Sure @navarone-feekery :

CleanShot.2024-11-22.at.15.13.48.mp4

[jedrazb]

could you expand the h1 tag to make sure no buttons are wrapped within h1 tag?

[JoseLuisGJ]

Good point @jedrazb there are still buttons within the h1 tag reading the expected behaviour from the issue ticket #198001:

Expected Result

Heading level is only announced one time (for one element, in this case title of connector). Heading level is not announced for description, nor for Save, Cancel buttons.

As commented here in the original issue ticket: we, from Search POV, can solve the issue regarding H1 not wrapping the description.

But bearing in mind that:

<EnterpriseSearchContentPageTemplate
      pageChrome={[...connectorsBreadcrumbs, connector?.name ?? '...']}
      pageViewTelemetry={tabId}
      isLoading={isLoading}
      pageHeader={{
        description: connector ? <ConnectorDescription connector={connector} /> : '...',
        pageTitle: connector ? <ConnectorName connector={connector} /> : '...',
        rightSideGroupProps: {
          gutterSize: 's',
          responsive: false,
          wrap: false,
        },
        rightSideItems: getHeaderActions(index, connector),
        tabs: tabs as Array<EuiTabProps & { label: React.ReactNode }>,
      }}
    >
      {selectedTab?.content || null}
</EnterpriseSearchContentPageTemplate>

1. PageHeader.PageTitle renders the content wrapped in a H1 by default
2. Within the ConnectorName we provide. We use and consume this EUI component EuiInlineEditTitle which renders these two buttons. So this part would be so hard to fix due to is part of the global PageTemplate we use. So we should decide if that's OK leaving this buttons within H1 oterwise we should think about remove completely the edit inline feature for titles to get the accessibility compliance.

[jedrazb]

@JoseLuisGJ got it thank you for sharing context, agreed that the fix for the h1 wrap is non-obvious and would involve adapting how page header and edit inline components are designed. If we can navigate the website with keyboard only and have reasonable voice over notifications I guess this should be fine

[JoseLuisGJ]

@elasticmachine merge upstream

[jedrazb]

The code duplication is the issue for me. I don't like that we duplicate most of logic between the _description and _name files, I asked copilot to refactor those files and create a HOC, Im sharing the suggestion as I like the structyre (though please validate! as I see that it lost e.g. heading="h1" prop for title):

HOC component suggestion

import React, { ChangeEvent, useEffect, useState } from 'react';
import { EuiFlexItem, EuiInlineEditText, EuiInlineEditTitle } from '@elastic/eui';
import { useActions, useValues } from 'kea';
import { i18n } from '@kbn/i18n';
import { Connector } from '@kbn/search-connectors';
import { ConnectorNameAndDescriptionLogic } from './connector_name_and_description_logic';

interface ConnectorFieldProps {
  connector: Connector;
  field: 'name' | 'description'; // The field to edit
  isTitle?: boolean; // Whether to render a title (`EuiInlineEditTitle`) or text (`EuiInlineEditText`)
}

export const ConnectorField: React.FC<ConnectorFieldProps> = ({ connector, field, isTitle }) => {
  const [value, setValue] = useState<string | null>(connector[field]);
  const [resolverObject, setResolverObject] = useState({
    rej: () => {},
    res: () => {},
  });
  const { saveNameAndDescription, setConnector } = useActions(ConnectorNameAndDescriptionLogic);
  const { status, isLoading, isFailed, isSuccess } = useValues(ConnectorNameAndDescriptionLogic);

  useEffect(() => {
    setConnector(connector);
  }, [connector]);

  useEffect(() => {
    if (isSuccess) resolverObject.res(true);
    if (isFailed) resolverObject.rej();
  }, [status]);

  const getValidationPromiseResolvers = () => {
    const resolvers = {
      rej: () => {},
      res: () => {},
    };
    const promise = new Promise((resolve, reject) => {
      resolvers.res = resolve;
      resolvers.rej = reject;
    });
    setResolverObject(resolvers);
    return promise;
  };

  const handleSave = async (newValue: string) => {
    setValue(newValue);
    saveNameAndDescription({ ...connector, [field]: newValue });
    return getValidationPromiseResolvers();
  };

  const handleCancel = (previousValue: string) => {
    setValue(previousValue);
  };

  const Component = isTitle ? EuiInlineEditTitle : EuiInlineEditText;

  return (
    <EuiFlexItem grow={false}>
      <Component
        inputAriaLabel={i18n.translate(
          `xpack.enterpriseSearch.content.connectors.nameAndDescription.${field}.ariaLabel`,
          {
            defaultMessage: `Edit connector ${field}`,
          }
        )}
        placeholder={i18n.translate(
          `xpack.enterpriseSearch.content.connectors.nameAndDescription.${field}.placeholder`,
          {
            defaultMessage: field === 'name' ? 'Add a name to your connector' : 'Add a description',
          }
        )}
        value={value || ''}
        isLoading={isLoading}
        isInvalid={field === 'name' && !value?.trim()}
        size="m"
        editModeProps={{
          cancelButtonProps: { onClick: () => handleCancel(connector[field] || '') },
          formRowProps: field === 'name' && !value?.trim() ? {
            error: [
              i18n.translate(
                'xpack.enterpriseSearch.content.nameAndDescription.name.error.empty',
                { defaultMessage: 'Connector name cannot be empty' }
              ),
            ],
          } : undefined,
          inputProps: { readOnly: isLoading },
        }}
        onSave={handleSave}
        onChange={(e: ChangeEvent<HTMLInputElement>) => setValue(e.target.value)}
        onCancel={() => handleCancel(connector[field] || '')}
      />
    </EuiFlexItem>
  );
};

and the e.g. connector name def:

import React from 'react';
import { Connector } from '@kbn/search-connectors';
import { ConnectorField } from './ConnectorField';

export const ConnectorName: React.FC<{ connector: Connector }> = ({ connector }) => (
  <ConnectorField connector={connector} field="name" isTitle />
);

and description

import React from 'react';
import { Connector } from '@kbn/search-connectors';
import { ConnectorField } from './ConnectorField';

export const ConnectorDescription: React.FC<{ connector: Connector }> = ({ connector }) => (
  <ConnectorField connector={connector} field="description" />
);

[artem-shelkovnikov]

I am also really away from accessibility/Kibana and do not fully understand what happens here.

Why does the accessibility improve? Is it because we now have 2 separate HTML sections that are handled better by the tools that do text-to-voice?

[jedrazb]

Why does the accessibility improve?

We no longer wrap description in h1 (page title) tag from what i understand

[JoseLuisGJ]

Thanks @jedrazb and @artem-shelkovnikov , indeed now H1 tag only renders the Title content and the description inline edit element it below it.

@jedrazb I refactored these components using a HOC and sharing code as your suggestion and with the help of Copilot as well.

[jedrazb]

LGTM, small comment

[jedrazb]

LGTM, thank you for helping with the a11y issues!

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: 8aeef56

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
enterpriseSearch	2313	2315	+2

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
enterpriseSearch	2.6MB	2.6MB	-324.0B

History

• 💔 Build #254354 failed 74b1fc1
• 💛 Build #253538 was flaky 9c72b9e

cc @JoseLuisGJ

[kibanamachine]

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/12033090516

[kibanamachine]

💚 All backports created successfully

Status	Branch	Result
✅	8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation