Suggestions: Add keyboard support (#114517)

* Suggestions: hashes on suggestions, update logic to select first suggestion

* fix types

* Suggestions: New UI style updates

* update some styles

* getting styles just right

* remove grouping when not on flag

* adjust minimum width for sidebar

* CI cleanups

* updates from ad hoc review

* add loading and error states to suggestions

* remove unused import

* update header ui for panel editor

* restore back button to vizpicker

* fix e2e test

* fix e2e

* add i18n update

* use new util for setVisualization operation

* Apply suggestions from code review

Co-authored-by: Torkel Ödegaard <torkel@grafana.com>

* comments from review

* updates from review

* Suggestions: Add keyboard support

* fix selector for PluginVisualization.item

---------

Co-authored-by: Torkel Ödegaard <torkel@grafana.com>

eslint-suppressions.json

@@ -2882,11 +2882,6 @@
       "count": 1
     }
   },
-  "public/app/features/panel/components/VizTypePicker/PanelTypeCard.tsx": {
-    "@grafana/no-aria-label-selectors": {
-      "count": 1
-    }
-  },
   "public/app/features/panel/panellinks/linkSuppliers.ts": {
     "@typescript-eslint/no-explicit-any": {
       "count": 1

packages/grafana-e2e-selectors/src/selectors/components.ts

@@ -1057,6 +1057,7 @@ export const versionedComponents = {
   },
   PluginVisualization: {
     item: {
+      '12.4.0': (title: string) => `data-testid Plugin visualization item ${title}`,
       [MIN_GRAFANA_VERSION]: (title: string) => `Plugin visualization item ${title}`,
     },
     current: {

packages/grafana-schema/src/raw/composable/table/panelcfg/x/TablePanelCfg_types.gen.ts

@@ -17,6 +17,10 @@ export interface Options {
    * Controls the height of the rows
    */
   cellHeight?: ui.TableCellHeight;
+  /**
+   * If true, disables all keyboard events in the table. this is used when previewing a table (i.e. suggestions)
+   */
+  disableKeyboardEvents?: boolean;
   /**
    * Enable pagination on the table
    */

packages/grafana-schema/src/raw/composable/timeseries/panelcfg/x/TimeSeriesPanelCfg_types.gen.ts

@@ -13,6 +13,7 @@ import * as common from '@grafana/schema';
 export const pluginVersion = "12.4.0-pre";
 
 export interface Options extends common.OptionsWithTimezones, common.OptionsWithAnnotations {
+  disableKeyboardEvents?: boolean;
   legend: common.VizLegendOptions;
   orientation?: common.VizOrientation;
   timeCompare?: common.TimeCompareOptions;

packages/grafana-ui/src/components/Table/TableNG/TableNG.tsx

@@ -105,6 +105,7 @@ export function TableNG(props: TableNGProps) {
   const {
     cellHeight,
     data,
+    disableKeyboardEvents,
     disableSanitizeHtml,
     enablePagination = false,
     enableSharedCrosshair = false,
@@ -819,9 +820,9 @@ export function TableNG(props: TableNGProps) {
           }
         }}
         onCellKeyDown={
-          hasNestedFrames
+          hasNestedFrames || disableKeyboardEvents
             ? (_, event) => {
-                if (event.isDefaultPrevented()) {
+                if (disableKeyboardEvents || event.isDefaultPrevented()) {
                   // skip parent grid keyboard navigation if nested grid handled it
                   event.preventGridDefault();
                 }

packages/grafana-ui/src/components/Table/TableNG/types.ts

@@ -138,6 +138,8 @@ export interface BaseTableProps {
   enableVirtualization?: boolean;
   // for MarkdownCell, this flag disables sanitization of HTML content. Configured via config.ini.
   disableSanitizeHtml?: boolean;
+  // if true, disables all keyboard events in the table. this is used when previewing a table (i.e. suggestions)
+  disableKeyboardEvents?: boolean;
 }
 
 /* ---------------------------- Table cell props ---------------------------- */

public/app/features/dashboard-scene/panel-edit/PanelVizTypePicker.tsx

@@ -1,6 +1,6 @@
 import { css } from '@emotion/css';
 import { debounce } from 'lodash';
-import { useCallback, useMemo, useState } from 'react';
+import { useCallback, useId, useMemo, useState } from 'react';
 import { useSessionStorage } from 'react-use';
 
 import { GrafanaTheme2, PanelData } from '@grafana/data';
@@ -8,7 +8,7 @@ import { selectors } from '@grafana/e2e-selectors';
 import { t, Trans } from '@grafana/i18n';
 import { config, reportInteraction } from '@grafana/runtime';
 import { VizPanel } from '@grafana/scenes';
-import { Button, FilterInput, ScrollContainer, Stack, Tab, TabContent, TabsBar, useStyles2 } from '@grafana/ui';
+import { Button, Field, FilterInput, ScrollContainer, Stack, Tab, TabContent, TabsBar, useStyles2 } from '@grafana/ui';
 import { LS_VISUALIZATION_SELECT_TAB_KEY } from 'app/core/constants';
 import { VisualizationSelectPaneTab } from 'app/features/dashboard/components/PanelEditor/types';
 import { VisualizationSuggestions } from 'app/features/panel/components/VizTypePicker/VisualizationSuggestions';
@@ -44,6 +44,7 @@ const getTabs = (): Array<{ label: string; value: VisualizationSelectPaneTab }>
 export function PanelVizTypePicker({ panel, data, onChange, onClose, showBackButton }: Props) {
   const styles = useStyles2(getStyles);
   const panelModel = useMemo(() => new PanelModelCompatibilityWrapper(panel), [panel]);
+  const filterId = useId();
 
   /** SEARCH */
   const [searchQuery, setSearchQuery] = useState('');
@@ -101,26 +102,36 @@ export function PanelVizTypePicker({ panel, data, onChange, onClose, showBackBut
           )}
           {listMode === VisualizationSelectPaneTab.Visualizations && (
             <Stack gap={1} direction="column">
-              <Stack direction="row" gap={1}>
-                {showBackButton && (
-                  <Button
-                    aria-label={t('dashboard-scene.panel-viz-type-picker.title-close', 'Close')}
-                    fill="text"
-                    variant="secondary"
-                    icon="arrow-left"
-                    data-testid={selectors.components.PanelEditor.toggleVizPicker}
-                    onClick={onClose}
-                  >
-                    <Trans i18nKey="dashboard-scene.panel-viz-type-picker.button.close">Back</Trans>
-                  </Button>
-                )}
-                <FilterInput
-                  className={styles.filter}
-                  value={searchQuery}
-                  onChange={setSearchQuery}
-                  placeholder={t('dashboard-scene.panel-viz-type-picker.placeholder-search-for', 'Search for...')}
-                />
-              </Stack>
+              <Field
+                tabIndex={0}
+                className={styles.searchField}
+                noMargin
+                htmlFor={filterId}
+                aria-label={t('dashboard-scene.panel-viz-type-picker.placeholder-search-for', 'Search for...')}
+              >
+                <Stack direction="row" gap={1}>
+                  {showBackButton && (
+                    <Button
+                      aria-label={t('dashboard-scene.panel-viz-type-picker.title-close', 'Close')}
+                      fill="text"
+                      variant="secondary"
+                      icon="arrow-left"
+                      data-testid={selectors.components.PanelEditor.toggleVizPicker}
+                      onClick={onClose}
+                    >
+                      <Trans i18nKey="dashboard-scene.panel-viz-type-picker.button.close">Back</Trans>
+                    </Button>
+                  )}
+                  <FilterInput
+                    id={filterId}
+                    className={styles.filter}
+                    value={searchQuery}
+                    onChange={setSearchQuery}
+                    placeholder={t('dashboard-scene.panel-viz-type-picker.placeholder-search-for', 'Search for...')}
+                  />
+                </Stack>
+              </Field>
+
               <VizTypePicker
                 pluginId={panel.state.pluginId}
                 searchQuery={searchQuery}
@@ -143,9 +154,8 @@ const getStyles = (theme: GrafanaTheme2) => ({
     height: '100%',
     gap: theme.spacing(2),
   }),
-  searchRow: css({
-    display: 'flex',
-    marginBottom: theme.spacing(2),
+  searchField: css({
+    marginTop: theme.spacing(0.5), // input glow with the boundary without this
   }),
   tabs: css({
     width: '100%',

public/app/features/library-panels/components/LibraryPanelCard/LibraryPanelCard.tsx

@@ -39,7 +39,7 @@ const LibraryPanelCardComponent = ({ libraryPanel, onClick, onDelete, showSecond
         title={libraryPanel.name}
         description={libraryPanel.description}
         plugin={panelPlugin}
-        onClick={() => onClick?.(libraryPanel)}
+        onSelect={() => onClick?.(libraryPanel)}
         onDelete={showSecondaryActions ? () => setShowDeletionModal(true) : undefined}
       >
         <FolderLink libraryPanel={libraryPanel} />

public/app/features/library-panels/components/LibraryPanelsSearch/LibraryPanelsSearch.test.tsx

@@ -252,7 +252,7 @@ describe('LibraryPanelsSearch', () => {
         }
       );
 
-      const card = () => screen.getByLabelText(/plugin visualization item time series/i);
+      const card = () => screen.getByTestId(/plugin visualization item time series/i);
 
       expect(screen.queryByText(/you haven\'t created any library panels yet/i)).not.toBeInTheDocument();
       expect(card()).toBeInTheDocument();
@@ -293,7 +293,7 @@ describe('LibraryPanelsSearch', () => {
         }
       );
 
-      const card = () => screen.getByLabelText(/plugin visualization item time series/i);
+      const card = () => screen.getByTestId(/plugin visualization item time series/i);
 
       expect(screen.queryByText(/you haven\'t created any library panels yet/i)).not.toBeInTheDocument();
       expect(card()).toBeInTheDocument();

public/app/features/panel/components/VizTypePicker/PanelTypeCard.tsx

@@ -1,5 +1,4 @@
 import { css, cx } from '@emotion/css';
-import { MouseEventHandler } from 'react';
 import * as React from 'react';
 import Skeleton from 'react-loading-skeleton';
 
@@ -14,11 +13,12 @@ interface Props {
   isCurrent: boolean;
   plugin: PanelPluginMeta;
   title: string;
-  onClick: MouseEventHandler<HTMLDivElement>;
+  onSelect: (withModKey?: boolean) => void;
   onDelete?: () => void;
   disabled?: boolean;
   showBadge?: boolean;
   description?: string;
+  tabIndex?: number;
 }
 
 const IMAGE_SIZE = 38;
@@ -27,12 +27,13 @@ const PanelTypeCardComponent = ({
   isCurrent,
   title,
   plugin,
-  onClick,
+  onSelect,
   onDelete,
   disabled,
   showBadge,
   description,
   children,
+  tabIndex = 0,
 }: React.PropsWithChildren<Props>) => {
   const styles = useStyles2(getStyles);
 
@@ -44,13 +45,22 @@ const PanelTypeCardComponent = ({
   });
 
   return (
-    // TODO: fix keyboard a11y
-    // eslint-disable-next-line jsx-a11y/click-events-have-key-events
     <div
       className={cssClass}
-      aria-label={selectors.components.PluginVisualization.item(plugin.name)}
       data-testid={selectors.components.PluginVisualization.item(plugin.name)}
-      onClick={isDisabled ? undefined : onClick}
+      onClick={isDisabled ? undefined : (ev) => onSelect(ev.metaKey || ev.ctrlKey || ev.altKey)}
+      role="button"
+      tabIndex={0}
+      onKeyDown={
+        isDisabled
+          ? undefined
+          : (ev) => {
+              if (ev.key === 'Enter' || ev.key === ' ') {
+                ev.preventDefault();
+                onSelect(ev.metaKey || ev.ctrlKey || ev.altKey);
+              }
+            }
+      }
       title={
         isCurrent ? t('panel.panel-type-card.title-click-to-close', 'Click again to close this section') : plugin.name
       }

public/app/features/panel/components/VizTypePicker/VisualizationSuggestionCard.tsx

@@ -34,8 +34,9 @@ export function VisualizationSuggestionCard({
     className: cx(className, styles.vizBox),
     'data-testid': selectors.components.VisualizationPreview.card(suggestion.name),
     style: outerStyles,
+    tabIndex: -1, // selection is handled by parent container
     ...restProps,
-  };
+  } satisfies HTMLAttributes<HTMLButtonElement> & { 'data-testid': string };
 
   let content: ReactNode;
 

public/app/features/panel/components/VizTypePicker/VisualizationSuggestions.tsx

@@ -147,10 +147,21 @@ export function VisualizationSuggestions({ onChange, data, panel }: Props) {
                   <div
                     key={suggestion.hash}
                     className={styles.cardContainer}
+                    tabIndex={0}
+                    role="button"
+                    aria-pressed={isCardSelected}
+                    onKeyDown={(ev) => {
+                      if (ev.key === 'Enter' || ev.key === ' ') {
+                        ev.preventDefault();
+                        applySuggestion(suggestion, isNewVizSuggestionsEnabled && !isCardSelected);
+                      }
+                    }}
                     ref={index === 0 ? firstCardRef : undefined}
                   >
                     {isCardSelected && (
                       <Button
+                        // rather than allow direct focus, we handle ketboard events in the card.
+                        tabIndex={-1}
                         variant="primary"
                         size={'md'}
                         className={styles.applySuggestionButton}
@@ -174,7 +185,6 @@ export function VisualizationSuggestions({ onChange, data, panel }: Props) {
                       suggestion={suggestion}
                       width={width}
                       isSelected={isCardSelected}
-                      tabIndex={index}
                       onClick={() => applySuggestion(suggestion, true)}
                     />
                   </div>

public/app/features/panel/components/VizTypePicker/VizTypePicker.tsx

@@ -41,16 +41,16 @@ export function VizTypePicker({ pluginId, searchQuery, onChange, trackSearch }:
 
   return (
     <div className={styles.grid}>
-      {filteredPluginTypes.map((plugin) => (
+      {filteredPluginTypes.map((plugin, idx) => (
         <VizTypePickerPlugin
           disabled={false}
           key={plugin.id}
           isCurrent={plugin.id === pluginId}
           plugin={plugin}
-          onClick={(e) =>
+          onSelect={(withModKey) =>
             onChange({
               pluginId: plugin.id,
-              withModKey: e.metaKey || e.ctrlKey || e.altKey,
+              withModKey,
             })
           }
         />

public/app/features/panel/components/VizTypePicker/VizTypePickerPlugin.tsx

@@ -1,5 +1,3 @@
-import { MouseEventHandler } from 'react';
-
 import { PanelPluginMeta } from '@grafana/data';
 
 import { PanelTypeCard } from './PanelTypeCard';
@@ -7,17 +5,17 @@ import { PanelTypeCard } from './PanelTypeCard';
 interface Props {
   isCurrent: boolean;
   plugin: PanelPluginMeta;
-  onClick: MouseEventHandler<HTMLDivElement>;
+  onSelect: (withModKey?: boolean) => void;
   disabled: boolean;
 }
 
-export const VizTypePickerPlugin = ({ isCurrent, plugin, onClick, disabled }: Props) => {
+export const VizTypePickerPlugin = ({ isCurrent, plugin, onSelect, disabled }: Props) => {
   return (
     <PanelTypeCard
       title={plugin.name}
       plugin={plugin}
       description={plugin.info.description}
-      onClick={onClick}
+      onSelect={onSelect}
       isCurrent={isCurrent}
       disabled={disabled}
       showBadge={true}

public/app/plugins/panel/heatmap/suggestions.ts

@@ -69,5 +69,14 @@ export const heatmapSuggestionsSupplier: VisualizationSuggestionsSupplier<Option
     return;
   }
 
-  return [{ score: determineScore(dataSummary) }];
+  return [
+    {
+      score: determineScore(dataSummary),
+      cardOptions: {
+        previewModifier: (s) => {
+          s.options!.legend = { show: false };
+        },
+      },
+    },
+  ];
 };

public/app/plugins/panel/histogram/module.tsx

@@ -10,7 +10,7 @@ import {
   DataFrameType,
 } from '@grafana/data';
 import { t } from '@grafana/i18n';
-import { commonOptionsBuilder, getGraphFieldOptions } from '@grafana/ui';
+import { commonOptionsBuilder, getGraphFieldOptions, LegendDisplayMode } from '@grafana/ui';
 import { StackingEditor } from '@grafana/ui/internal';
 
 import { HistogramPanel } from './HistogramPanel';
@@ -160,6 +160,16 @@ export const plugin = new PanelPlugin<Options, FieldConfig>(HistogramPanel)
           score: ds.hasDataFrameType(DataFrameType.Histogram)
             ? VisualizationSuggestionScore.Best
             : VisualizationSuggestionScore.OK,
+          cardOptions: {
+            previewModifier: (s) => {
+              s.options!.legend = {
+                calcs: [],
+                displayMode: LegendDisplayMode.Hidden,
+                placement: 'bottom',
+                showLegend: false,
+              };
+            },
+          },
         },
       ];
     }

public/app/plugins/panel/status-history/module.tsx

@@ -1,6 +1,6 @@
 import { FieldColorModeId, FieldConfigProperty, FieldType, PanelPlugin } from '@grafana/data';
 import { t } from '@grafana/i18n';
-import { AxisPlacement, VisibilityMode } from '@grafana/schema';
+import { AxisPlacement, LegendDisplayMode, VisibilityMode } from '@grafana/schema';
 import { commonOptionsBuilder } from '@grafana/ui';
 
 import { StatusHistoryPanel } from './StatusHistoryPanel';
@@ -144,6 +144,12 @@ export const plugin = new PanelPlugin<Options, FieldConfig>(StatusHistoryPanel)
       {
         cardOptions: {
           previewModifier: (s) => {
+            s.options!.legend = {
+              displayMode: LegendDisplayMode.Hidden,
+              placement: 'bottom',
+              calcs: [],
+              showLegend: false,
+            };
             s.options!.colWidth = 0.7;
           },
         },

public/app/plugins/panel/table/TablePanel.tsx

@@ -89,6 +89,7 @@ export function TablePanel(props: Props) {
       structureRev={data.structureRev}
       transparent={transparent}
       disableSanitizeHtml={disableSanitizeHtml}
+      disableKeyboardEvents={options.disableKeyboardEvents}
     />
   );
 
@@ -107,7 +108,12 @@ export function TablePanel(props: Props) {
     <div className={tableStyles.wrapper}>
       {tableElement}
       <div className={tableStyles.selectWrapper}>
-        <Select options={names} value={names[currentIndex]} onChange={(val) => onChangeTableSelection(val, props)} />
+        <Select
+          tabIndex={options.disableKeyboardEvents ? -1 : 0}
+          options={names}
+          value={names[currentIndex]}
+          onChange={(val) => onChangeTableSelection(val, props)}
+        />
       </div>
     </div>
   );

public/app/plugins/panel/table/panelcfg.cue

@@ -43,6 +43,8 @@ composableKinds: PanelCfg: {
 					frozenColumns?: {
 						left?: number | *0
 					}
+          // If true, disables all keyboard events in the table. this is used when previewing a table (i.e. suggestions)
+          disableKeyboardEvents?: bool
 				} @cuetsy(kind="interface")
 				FieldConfig: {
 					ui.TableFieldOptions

public/app/plugins/panel/table/panelcfg.gen.ts

@@ -15,6 +15,10 @@ export interface Options {
    * Controls the height of the rows
    */
   cellHeight?: ui.TableCellHeight;
+  /**
+   * If true, disables all keyboard events in the table. this is used when previewing a table (i.e. suggestions)
+   */
+  disableKeyboardEvents?: boolean;
   /**
    * Enable pagination on the table
    */

public/app/plugins/panel/table/suggestions.ts

@@ -21,6 +21,8 @@ export const tableSuggestionsSupplier: VisualizationSuggestionsSupplier<Options,
     score: getTableSuggestionScore(dataSummary),
     cardOptions: {
       previewModifier: (s) => {
+        s.options!.showHeader = false;
+        s.options!.disableKeyboardEvents = true;
         if (s.fieldConfig && s.fieldConfig.defaults.custom) {
           s.fieldConfig.defaults.custom.minWidth = 50;
         }

public/app/plugins/panel/timeseries/TimeSeriesPanel.tsx

@@ -145,7 +145,7 @@ export const TimeSeriesPanel = ({
       {(uplotConfig, alignedFrame) => {
         return (
           <>
-            <KeyboardPlugin config={uplotConfig} />
+            {!options.disableKeyboardEvents && <KeyboardPlugin config={uplotConfig} />}
             {cursorSync !== DashboardCursorSync.Off && (
               <EventBusPlugin config={uplotConfig} eventBus={eventBus} frame={alignedFrame} />
             )}

public/app/plugins/panel/timeseries/panelcfg.cue

@@ -31,6 +31,7 @@ composableKinds: PanelCfg: lineage: {
 				timeCompare?: common.TimeCompareOptions
 				orientation?: common.VizOrientation
 				annotations?: common.VizAnnotations
+        disableKeyboardEvents?: bool
 			} @cuetsy(kind="interface")
 
 			FieldConfig: common.GraphFieldConfig & {} @cuetsy(kind="interface")

public/app/plugins/panel/timeseries/panelcfg.gen.ts

@@ -11,6 +11,7 @@
 import * as common from '@grafana/schema';
 
 export interface Options extends common.OptionsWithTimezones, common.OptionsWithAnnotations {
+  disableKeyboardEvents?: boolean;
   legend: common.VizLegendOptions;
   orientation?: common.VizOrientation;
   timeCompare?: common.TimeCompareOptions;

public/app/plugins/panel/timeseries/suggestions.ts

@@ -45,6 +45,7 @@ const withDefaults = (
     },
     cardOptions: {
       previewModifier: (s) => {
+        s.options!.disableKeyboardEvents = true;
         if (s.fieldConfig?.defaults.custom?.drawStyle !== GraphDrawStyle.Bars) {
           s.fieldConfig!.defaults.custom!.lineWidth = Math.max(s.fieldConfig!.defaults.custom!.lineWidth ?? 1, 2);
         }