Table: Clean up filter popover layout and improve filter selection UX (#114052)

* Table: Clean up filter popover layout

* Table: Improve filter selection ux text

* remove unused ref

* update snapshot test

* whoops, we don't want to change the global input suffix padding

packages/grafana-ui/src/components/FilterInput/FilterInput.tsx

@@ -6,35 +6,40 @@ import { Trans } from '@grafana/i18n';
 import { useCombinedRefs } from '../../utils/useCombinedRefs';
 import { Button } from '../Button/Button';
 import { Icon } from '../Icon/Icon';
-import { Input } from '../Input/Input';
+import { Input, Props as InputProps } from '../Input/Input';
 
 export interface Props extends Omit<HTMLProps<HTMLInputElement>, 'onChange'> {
   value: string | undefined;
   width?: number;
   onChange: (value: string) => void;
   escapeRegex?: boolean;
+  suffix?: InputProps['suffix'];
 }
 
 export const FilterInput = forwardRef<HTMLInputElement, Props>(
-  ({ value, width, onChange, escapeRegex = true, ...restProps }, ref) => {
+  ({ value, width, onChange, escapeRegex = true, suffix: _suffix, ...restProps }, ref) => {
     const innerRef = useRef<HTMLInputElement | null>(null);
     const combinedRef = useCombinedRefs<HTMLInputElement>(ref, innerRef);
 
-    const suffix =
-      value !== '' ? (
-        <Button
-          icon="times"
-          fill="text"
-          size="sm"
-          onClick={(e) => {
-            innerRef.current?.focus();
-            onChange('');
-            e.stopPropagation();
-          }}
-        >
-          <Trans i18nKey="grafana-ui.filter-input.clear">Clear</Trans>
-        </Button>
-      ) : null;
+    const suffix = (
+      <>
+        {value !== '' && (
+          <Button
+            icon="times"
+            fill="text"
+            size="sm"
+            onClick={(e) => {
+              innerRef.current?.focus();
+              onChange('');
+              e.stopPropagation();
+            }}
+          >
+            <Trans i18nKey="grafana-ui.filter-input.clear">Clear</Trans>
+          </Button>
+        )}
+        {_suffix}
+      </>
+    );
 
     return (
       <Input

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

@@ -5,7 +5,7 @@ import { FixedSizeList as List, ListChildComponentProps } from 'react-window';
 
 import { GrafanaTheme2, formattedValueToString, getValueFormat, SelectableValue } from '@grafana/data';
 import { selectors } from '@grafana/e2e-selectors';
-import { Trans } from '@grafana/i18n';
+import { t, Trans } from '@grafana/i18n';
 
 import { useStyles2, useTheme2 } from '../../../../themes/ThemeContext';
 import { Checkbox } from '../../../Forms/Checkbox';
@@ -24,7 +24,7 @@ interface Props {
 }
 
 const ITEM_HEIGHT = 28;
-const MIN_HEIGHT = ITEM_HEIGHT * 5;
+const MIN_HEIGHT = ITEM_HEIGHT * 4.5; // split an item in the middle to imply there are more items to scroll
 
 export const REGEX_OPERATOR = operatorSelectableValues['Contains'];
 const XPR_OPERATOR = operatorSelectableValues['Expression'];
@@ -109,21 +109,31 @@ export const FilterList = ({ options, values, caseSensitive, onChange, searchFil
     () => selectedItems.length > 0 && items.length > selectedItems.length,
     [items, selectedItems]
   );
-  const selectCheckLabel = useMemo(
-    () => (selectedItems.length ? `${selectedItems.length} selected` : `Select all`),
-    [selectedItems]
-  );
+  const selectCheckLabel = useMemo(() => {
+    if (!values.length) {
+      return t('grafana-ui.table.filter.select-all', 'Select all');
+    }
+    if (values.length !== selectedItems.length) {
+      return t('grafana-ui.table.filter.selected-some-hidden', '{{ numSelected }} selected ({{ numHidden }} hidden)', {
+        numSelected: values.length,
+        numHidden: values.length - selectedItems.length,
+      });
+    }
+    return t('grafana-ui.table.filter.selected', '{{ numSelected }} selected', {
+      numSelected: values.length,
+    });
+  }, [selectedItems.length, values.length]);
   const selectCheckDescription = useMemo(
     () =>
       items.length !== selectedItems.length
-        ? 'Add all displayed values to the filter'
-        : 'Remove all displayed values from the filter',
+        ? t('grafana-ui.table.filter.add-all', 'Add all displayed values to the filter')
+        : t('grafana-ui.table.filter.remove-all', 'Remove all displayed values from the filter'),
     [items, selectedItems]
   );
 
   const styles = useStyles2(getStyles);
   const theme = useTheme2();
-  const gutter = theme.spacing.gridSize;
+  const gutter = theme.spacing.gridSize / 2;
   const height = useMemo(() => Math.min(items.length * ITEM_HEIGHT, MIN_HEIGHT) + gutter, [gutter, items.length]);
 
   const onCheckedChanged = useCallback(
@@ -207,9 +217,8 @@ function ItemRenderer({ index, style, data: { onCheckedChanged, items, values, c
 const getStyles = (theme: GrafanaTheme2) => ({
   filterList: css({
     label: 'filterList',
-    backgroundColor: theme.components.input.background,
-    border: `1px solid ${theme.colors.border.medium}`,
-    borderRadius: theme.shape.radius.default,
+    marginBottom: theme.spacing(0.5),
+    borderBottom: `1px solid ${theme.colors.border.weak}`,
   }),
   filterListRow: css({
     label: 'filterListRow',
@@ -223,12 +232,6 @@ const getStyles = (theme: GrafanaTheme2) => ({
       backgroundColor: theme.colors.action.hover,
     },
   }),
-  selectDivider: css({
-    label: 'selectDivider',
-    width: '100%',
-    borderTop: `1px solid ${theme.colors.border.medium}`,
-    padding: theme.spacing(0.5, 2),
-  }),
   noValuesLabel: css({
     paddingTop: theme.spacing(1),
   }),

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

@@ -118,27 +118,28 @@ export const FilterPopup = ({
         ref={containerRef}
       >
         <Stack direction="column">
-          <Stack alignItems="center">
-            {field && <Label className={styles.label}>{getDisplayName(field)}</Label>}
-            <ButtonSelect
-              variant="canvas"
-              options={OPERATORS}
-              onChange={setOperator}
-              value={operator}
-              tooltip={operator.description}
-              root={containerRef.current ?? undefined}
-            />
-          </Stack>
-
-          <div className={styles.listDivider} />
+          <Stack alignItems="center">{field && <Label className={styles.label}>{getDisplayName(field)}</Label>}</Stack>
 
           <Stack gap={1}>
-            <FilterInput
-              placeholder={filterInputPlaceholder}
-              title={filterInputPlaceholder}
-              onChange={setSearchFilter}
-              value={searchFilter}
-            />
+            <div className={styles.inputContainer}>
+              <FilterInput
+                placeholder={filterInputPlaceholder}
+                title={filterInputPlaceholder}
+                onChange={setSearchFilter}
+                value={searchFilter}
+                suffix={
+                  <ButtonSelect
+                    className={styles.buttonSelectOverrides}
+                    options={OPERATORS}
+                    onChange={setOperator}
+                    value={operator}
+                    tooltip={operator.description}
+                    narrow
+                    root={containerRef.current ?? undefined}
+                  />
+                }
+              />
+            </div>
             <Button
               tooltip={t('grafana-ui.table.filter-popup-aria-label-match-case', 'Match case')}
               variant="secondary"
@@ -159,21 +160,17 @@ export const FilterPopup = ({
             operator={operator}
           />
 
-          <Stack gap={3}>
-            <Stack>
-              <Button size="sm" onClick={onFilter}>
-                <Trans i18nKey="grafana-ui.table.filter-popup-apply">Ok</Trans>
-              </Button>
-              <Button size="sm" variant="secondary" onClick={onCancel}>
-                <Trans i18nKey="grafana-ui.table.filter-popup-cancel">Cancel</Trans>
-              </Button>
-            </Stack>
+          <Stack justifyContent="end" direction="row-reverse">
+            <Button size="sm" onClick={onFilter}>
+              <Trans i18nKey="grafana-ui.table.filter-popup-apply">Ok</Trans>
+            </Button>
+            <Button size="sm" variant="secondary" onClick={onCancel}>
+              <Trans i18nKey="grafana-ui.table.filter-popup-cancel">Cancel</Trans>
+            </Button>
             {clearFilterVisible && (
-              <Stack>
-                <Button fill="text" size="sm" onClick={onClearFilter}>
-                  <Trans i18nKey="grafana-ui.table.filter-popup-clear">Clear filter</Trans>
-                </Button>
-              </Stack>
+              <Button fill="text" size="sm" onClick={onClearFilter}>
+                <Trans i18nKey="grafana-ui.table.filter-popup-clear">Clear filter</Trans>
+              </Button>
             )}
           </Stack>
         </Stack>
@@ -186,7 +183,7 @@ const getStyles = (theme: GrafanaTheme2) => ({
   filterContainer: css({
     label: 'filterContainer',
     width: '100%',
-    minWidth: '250px',
+    minWidth: '320px',
     height: '100%',
     backgroundColor: theme.colors.background.primary,
     border: `1px solid ${theme.colors.border.weak}`,
@@ -194,14 +191,19 @@ const getStyles = (theme: GrafanaTheme2) => ({
     boxShadow: theme.shadows.z3,
     borderRadius: theme.shape.radius.default,
   }),
-  listDivider: css({
-    label: 'listDivider',
-    width: '100%',
-    borderTop: `1px solid ${theme.colors.border.medium}`,
-  }),
   label: css({
     marginBottom: 0,
   }),
+  inputContainer: css({
+    width: 300,
+  }),
+  buttonSelectOverrides: css({
+    fontSize: 12,
+    '&:hover, &:focus, &:active': {
+      color: theme.colors.text.primary,
+      background: 'transparent',
+    },
+  }),
 });
 
 const stopPropagation = (event: React.MouseEvent) => {

public/locales/en-US/grafana.json

@@ -9002,6 +9002,13 @@
       "copy": "Copy to Clipboard",
       "csv-counts": "Rows:{{rows}}, Columns:{{columns}}",
       "csv-placeholder": "Enter CSV here...",
+      "filter": {
+        "add-all": "Add all displayed values to the filter",
+        "remove-all": "Remove all displayed values from the filter",
+        "select-all": "Select all",
+        "selected": "{{ numSelected }} selected",
+        "selected-some-hidden": "{{ numSelected }} selected ({{ numHidden }} hidden)"
+      },
       "filter-placeholder": "Filter values",
       "filter-popup-apply": "Ok",
       "filter-popup-aria-label-match-case": "Match case",