Several UX touchups:

- Fix double EuiSelectable focus ring styles - Fix issue where toolbar buttons get cut off on smaller screen sizes - Add tooltips to toolbar buttons so users can see the full labels when truncated - Tweak toolbar button menu designs to more closely match mockups (full-width menu items, compressed search bar, show all time interval menu items without scrolling) - Add middle truncation to toolbar button menus for long values, and update search bar placeholder to "Search" to match mockups - Autofocus on toolbar button menu selectables when popover is opened - Update toolbar button menu "no results" display to put message and search term on one line - Remove extra 2 pixels below Lens suggestion selector caused by `display: inline-block`, and update tooltip delay to "long" to match the toolbar button tooltips
jughosta · Jan 11, 2024 · 8560564 · 8560564
1 parent 125e9d7
commit 8560564
Show file tree

Hide file tree

Showing 6 changed files with 144 additions and 86 deletions.
diff --git a/packages/shared-ux/button_toolbar/src/buttons/toolbar_button/toolbar_button.test.tsx b/packages/shared-ux/button_toolbar/src/buttons/toolbar_button/toolbar_button.test.tsx
@@ -45,6 +45,13 @@ describe('<ToolbarButton />', () => {
       component.find('button').simulate('click');
       expect(mockHandler).toHaveBeenCalled();
     });
+
+    test('accepts an onBlur handler', () => {
+      const mockHandler = jest.fn();
+      const component = mountWithIntl(<ToolbarButton label="Create chart" onBlur={mockHandler} />);
+      component.find('button').simulate('blur');
+      expect(mockHandler).toHaveBeenCalled();
+    });
   });
 
   describe('iconButton', () => {

diff --git a/packages/shared-ux/button_toolbar/src/buttons/toolbar_button/toolbar_button.tsx b/packages/shared-ux/button_toolbar/src/buttons/toolbar_button/toolbar_button.tsx
@@ -23,7 +23,7 @@ type ButtonRenderStyle = 'standard' | 'iconButton';
 interface ToolbarButtonCommonProps
   extends Pick<
     EuiButtonPropsForButton,
-    'onClick' | 'iconType' | 'size' | 'data-test-subj' | 'isDisabled' | 'aria-label'
+    'onClick' | 'onBlur' | 'iconType' | 'size' | 'data-test-subj' | 'isDisabled' | 'aria-label'
   > {
   /**
    * Render style of the toolbar button

diff --git a/src/core/public/styles/_base.scss b/src/core/public/styles/_base.scss
@@ -9,7 +9,11 @@
 // focus states in Kibana.
 :focus {
   &:not([class^='eui']):not(.kbn-resetFocusState) {
-    @include euiFocusRing;
+    // The focus policy causes double focus rings to appear on EuiSelectableList
+    // since the focusable element does not contain a class starting with "eui".
+    &:not(.euiSelectableList__list > ul) {
+      @include euiFocusRing;
+    }
   }
 }
 

diff --git a/src/plugins/unified_histogram/public/chart/chart.tsx b/src/plugins/unified_histogram/public/chart/chart.tsx
@@ -277,58 +277,68 @@ export function Chart({
       responsive={false}
     >
       <EuiFlexItem grow={false} css={chartToolbarCss}>
-        <EuiFlexGroup direction="row" gutterSize="s" responsive={false} alignItems="center">
-          <EuiFlexItem grow={false}>
-            {renderCustomChartToggleActions ? (
-              renderCustomChartToggleActions()
-            ) : (
-              <IconButtonGroup
-                legend={i18n.translate('unifiedHistogram.hideChartButtongroupLegend', {
-                  defaultMessage: 'Chart visibility',
-                })}
-                buttonSize="s"
-                buttons={[
-                  {
-                    label: chartVisible
-                      ? i18n.translate('unifiedHistogram.hideChartButton', {
-                          defaultMessage: 'Hide chart',
-                        })
-                      : i18n.translate('unifiedHistogram.showChartButton', {
-                          defaultMessage: 'Show chart',
-                        }),
-                    iconType: chartVisible ? 'transitionTopOut' : 'transitionTopIn',
-                    'data-test-subj': 'unifiedHistogramToggleChartButton',
-                    onClick: toggleHideChart,
-                  },
-                ]}
-              />
-            )}
-          </EuiFlexItem>
-          {chartVisible && !isPlainRecord && !!onTimeIntervalChange && (
-            <EuiFlexItem grow={false}>
-              <TimeIntervalSelector chart={chart} onTimeIntervalChange={onTimeIntervalChange} />
-            </EuiFlexItem>
-          )}
-          <EuiFlexItem>
-            <div>
-              {chartVisible && breakdown && (
-                <BreakdownFieldSelector
-                  dataView={dataView}
-                  breakdown={breakdown}
-                  onBreakdownFieldChange={onBreakdownFieldChange}
-                />
-              )}
-              {chartVisible &&
-                currentSuggestion &&
-                allSuggestions &&
-                allSuggestions?.length > 1 && (
-                  <SuggestionSelector
-                    suggestions={allSuggestions}
-                    activeSuggestion={currentSuggestion}
-                    onSuggestionChange={onSuggestionSelectorChange}
+        <EuiFlexGroup
+          direction="row"
+          gutterSize="s"
+          responsive={false}
+          alignItems="center"
+          justifyContent="spaceBetween"
+        >
+          <EuiFlexItem grow={false} css={{ minWidth: 0 }}>
+            <EuiFlexGroup direction="row" gutterSize="s" responsive={false} alignItems="center">
+              <EuiFlexItem grow={false}>
+                {renderCustomChartToggleActions ? (
+                  renderCustomChartToggleActions()
+                ) : (
+                  <IconButtonGroup
+                    legend={i18n.translate('unifiedHistogram.hideChartButtongroupLegend', {
+                      defaultMessage: 'Chart visibility',
+                    })}
+                    buttonSize="s"
+                    buttons={[
+                      {
+                        label: chartVisible
+                          ? i18n.translate('unifiedHistogram.hideChartButton', {
+                              defaultMessage: 'Hide chart',
+                            })
+                          : i18n.translate('unifiedHistogram.showChartButton', {
+                              defaultMessage: 'Show chart',
+                            }),
+                        iconType: chartVisible ? 'transitionTopOut' : 'transitionTopIn',
+                        'data-test-subj': 'unifiedHistogramToggleChartButton',
+                        onClick: toggleHideChart,
+                      },
+                    ]}
                   />
                 )}
-            </div>
+              </EuiFlexItem>
+              {chartVisible && !isPlainRecord && !!onTimeIntervalChange && (
+                <EuiFlexItem grow={false} css={{ minWidth: 0 }}>
+                  <TimeIntervalSelector chart={chart} onTimeIntervalChange={onTimeIntervalChange} />
+                </EuiFlexItem>
+              )}
+              <EuiFlexItem grow={false} css={{ minWidth: 0 }}>
+                <div>
+                  {chartVisible && breakdown && (
+                    <BreakdownFieldSelector
+                      dataView={dataView}
+                      breakdown={breakdown}
+                      onBreakdownFieldChange={onBreakdownFieldChange}
+                    />
+                  )}
+                  {chartVisible &&
+                    currentSuggestion &&
+                    allSuggestions &&
+                    allSuggestions?.length > 1 && (
+                      <SuggestionSelector
+                        suggestions={allSuggestions}
+                        activeSuggestion={currentSuggestion}
+                        onSuggestionChange={onSuggestionSelectorChange}
+                      />
+                    )}
+                </div>
+              </EuiFlexItem>
+            </EuiFlexGroup>
           </EuiFlexItem>
           {chartVisible && actions.length > 0 && (
             <EuiFlexItem grow={false}>

diff --git a/src/plugins/unified_histogram/public/chart/suggestion_selector.tsx b/src/plugins/unified_histogram/public/chart/suggestion_selector.tsx
@@ -75,6 +75,8 @@ export const SuggestionSelector = ({
       position="top"
       content={suggestionsPopoverDisabled ? undefined : activeSuggestion?.title}
       anchorProps={{ css: suggestionComboCss }}
+      display="block"
+      delay="long"
     >
       <EuiComboBox
         data-test-subj="unifiedHistogramSuggestionSelector"

diff --git a/src/plugins/unified_histogram/public/chart/toolbar_selector.tsx b/src/plugins/unified_histogram/public/chart/toolbar_selector.tsx
@@ -8,19 +8,20 @@
 
 import React, { useCallback, ReactElement, useState, useMemo } from 'react';
 import {
-  EuiFlexGroup,
-  EuiFlexItem,
   EuiPopover,
   EuiPopoverTitle,
   EuiSelectable,
   EuiSelectableProps,
   EuiSelectableOption,
   useEuiTheme,
+  EuiPanel,
+  EuiToolTip,
 } from '@elastic/eui';
 import { ToolbarButton } from '@kbn/shared-ux-button-toolbar';
 import { FormattedMessage } from '@kbn/i18n-react';
 import { css } from '@emotion/react';
 import { calculateWidthFromEntries } from '@kbn/calculate-width-from-char-count';
+import { i18n } from '@kbn/i18n';
 
 export const EMPTY_OPTION = '__EMPTY_SELECTOR_OPTION__';
 
@@ -48,6 +49,14 @@ export const ToolbarSelector: React.FC<ToolbarSelectorProps> = ({
   const { euiTheme } = useEuiTheme();
   const [isOpen, setIsOpen] = useState<boolean>(false);
   const [searchTerm, setSearchTerm] = useState<string>();
+  const [labelPopoverDisabled, setLabelPopoverDisabled] = useState(false);
+
+  const disableLabelPopover = useCallback(() => setLabelPopoverDisabled(true), []);
+
+  const enableLabelPopover = useCallback(
+    () => setTimeout(() => setLabelPopoverDisabled(false)),
+    []
+  );
 
   const onSelectionChange = useCallback(
     (newOptions) => {
@@ -57,15 +66,24 @@ export const ToolbarSelector: React.FC<ToolbarSelectorProps> = ({
         chosenOption?.value && chosenOption?.value !== EMPTY_OPTION ? chosenOption : undefined
       );
       setIsOpen(false);
+      disableLabelPopover();
     },
-    [onChange, setIsOpen]
+    [disableLabelPopover, onChange]
   );
 
   const searchProps: EuiSelectableProps['searchProps'] = useMemo(
     () =>
       searchable
         ? {
+            id: `${dataTestSubj}SelectableInput`,
             'data-test-subj': `${dataTestSubj}SelectorSearch`,
+            compressed: true,
+            placeholder: i18n.translate(
+              'unifiedHistogram.toolbarSelectorPopover.searchPlaceholder',
+              {
+                defaultMessage: 'Search',
+              }
+            ),
             onChange: (value) => setSearchTerm(value),
           }
         : undefined,
@@ -78,62 +96,79 @@ export const ToolbarSelector: React.FC<ToolbarSelectorProps> = ({
     <EuiPopover
       id={dataTestSubj}
       ownFocus
-      initialFocus={`.${dataTestSubj}__popoverPanel`}
-      panelClassName={`${dataTestSubj}__popoverPanel`}
+      initialFocus={
+        searchable ? `#${dataTestSubj}SelectableInput` : `#${dataTestSubj}Selectable_listbox`
+      }
       panelProps={{
-        css: css`
-          min-width: ${panelMinWidth}px;
-        `,
+        css: searchable
+          ? css`
+              min-width: ${panelMinWidth}px;
+            `
+          : css`
+              width: ${panelMinWidth}px;
+            `,
       }}
-      panelPaddingSize="s"
+      panelPaddingSize="none"
       button={
-        <ToolbarButton
-          size="s"
-          css={css`
-            font-weight: ${euiTheme.font.weight.medium};
-            max-width: ${euiTheme.base * 20}px;
-          `}
-          data-test-subj={`${dataTestSubj}Button`}
-          data-selected-value={dataSelectedValue}
-          aria-label={popoverTitle}
-          label={buttonLabel}
-          onClick={() => setIsOpen(!isOpen)}
-        />
+        <EuiToolTip content={labelPopoverDisabled ? undefined : buttonLabel} delay="long">
+          <ToolbarButton
+            size="s"
+            css={css`
+              font-weight: ${euiTheme.font.weight.medium};
+              width: 100%;
+              min-width: 0;
+              max-width: ${euiTheme.base * 20}px;
+            `}
+            data-test-subj={`${dataTestSubj}Button`}
+            data-selected-value={dataSelectedValue}
+            aria-label={popoverTitle}
+            label={buttonLabel}
+            onClick={() => setIsOpen(!isOpen)}
+            onBlur={enableLabelPopover}
+          />
+        </EuiToolTip>
       }
       isOpen={isOpen}
       closePopover={() => setIsOpen(false)}
       anchorPosition="downLeft"
     >
-      <EuiPopoverTitle>
-        <EuiFlexGroup alignItems="center" responsive={false}>
-          <EuiFlexItem>{popoverTitle}</EuiFlexItem>
-        </EuiFlexGroup>
-      </EuiPopoverTitle>
+      <EuiPopoverTitle paddingSize="s">{popoverTitle}</EuiPopoverTitle>
       <EuiSelectable
+        id={`${dataTestSubj}Selectable`}
         singleSelection
         aria-label={popoverTitle}
         data-test-subj={`${dataTestSubj}Selectable`}
         options={options}
         onChange={onSelectionChange}
+        listProps={{
+          truncationProps: { truncation: 'middle' },
+          isVirtualized: searchable,
+        }}
         {...(searchable
           ? {
               searchable,
               searchProps,
               noMatchesMessage: (
-                <FormattedMessage
-                  id="unifiedHistogram.toolbarSelectorPopover.noResults"
-                  defaultMessage="No results found for {term}"
-                  values={{
-                    term: <strong>{searchTerm}</strong>,
-                  }}
-                />
+                <p>
+                  <FormattedMessage
+                    id="unifiedHistogram.toolbarSelectorPopover.noResults"
+                    defaultMessage="No results found for {term}"
+                    values={{
+                      term: <strong>{searchTerm}</strong>,
+                    }}
+                  />
+                </p>
               ),
             }
           : {})}
       >
         {(list, search) => (
           <>
-            {search}
+            {search && (
+              <EuiPanel paddingSize="s" hasShadow={false} css={{ paddingBottom: 0 }}>
+                {search}
+              </EuiPanel>
+            )}
             {list}
           </>
         )}