From bedd662c0727fbfd02578f84852f5afdfeb4ba19 Mon Sep 17 00:00:00 2001
From: Dominik Prokop <dominik.prokop@grafana.com>
Date: Mon, 18 Jan 2021 12:53:22 +0100
Subject: [PATCH] Panel options UI: Allow collapsible categories (#30301)

---
 .../src/panel/registryFactories.ts            |   1 -
 packages/grafana-data/src/types/options.ts    |   2 +-
 .../src/selectors/components.ts               |   2 +-
 .../DefaultFieldConfigEditor.test.tsx         | 174 ++++++++++++++++++
 .../PanelEditor/DefaultFieldConfigEditor.tsx  |  39 ++--
 .../PanelEditor/DynamicConfigValueEditor.tsx  |   5 +-
 .../PanelEditor/PanelOptionsEditor.tsx        |   9 +-
 7 files changed, 214 insertions(+), 18 deletions(-)

diff --git a/packages/grafana-data/src/panel/registryFactories.ts b/packages/grafana-data/src/panel/registryFactories.ts
index bf25b5c5602..694d6010c47 100644
--- a/packages/grafana-data/src/panel/registryFactories.ts
+++ b/packages/grafana-data/src/panel/registryFactories.ts
@@ -24,7 +24,6 @@ export function createFieldConfigRegistry<TFieldConfigOptions>(
 
     for (const customProp of builder.getRegistry().list()) {
       customProp.isCustom = true;
-      customProp.category = [`${pluginName} options`].concat(customProp.category || []);
       // need to do something to make the custom items not conflict with standard ones
       // problem is id (registry index) is used as property path
       // so sort of need a property path on the FieldPropertyEditorItem
diff --git a/packages/grafana-data/src/types/options.ts b/packages/grafana-data/src/types/options.ts
index d44879afefa..13ec9a43ca0 100644
--- a/packages/grafana-data/src/types/options.ts
+++ b/packages/grafana-data/src/types/options.ts
@@ -39,7 +39,7 @@ export interface OptionEditorConfig<TOptions, TSettings = any, TValue = any> {
   /**
    * Array of strings representing category of the option. First element in the array will make option render as collapsible section.
    */
-  category?: string[];
+  category?: Array<string | undefined>;
 
   /**
    * Set this value if undefined
diff --git a/packages/grafana-e2e-selectors/src/selectors/components.ts b/packages/grafana-e2e-selectors/src/selectors/components.ts
index 33d8afab5c4..7b99a4756de 100644
--- a/packages/grafana-e2e-selectors/src/selectors/components.ts
+++ b/packages/grafana-e2e-selectors/src/selectors/components.ts
@@ -127,7 +127,7 @@ export const Components = {
     backArrow: 'Go Back button',
   },
   OptionsGroup: {
-    toggle: (title: string) => `Options group ${title}`,
+    toggle: (title?: string) => (title ? `Options group ${title}` : 'Options group'),
   },
   PluginVisualization: {
     item: (title: string) => `Plugin visualization item ${title}`,
diff --git a/public/app/features/dashboard/components/PanelEditor/DefaultFieldConfigEditor.test.tsx b/public/app/features/dashboard/components/PanelEditor/DefaultFieldConfigEditor.test.tsx
index 36ccdd181c2..b3f322a3ee4 100644
--- a/public/app/features/dashboard/components/PanelEditor/DefaultFieldConfigEditor.test.tsx
+++ b/public/app/features/dashboard/components/PanelEditor/DefaultFieldConfigEditor.test.tsx
@@ -86,4 +86,178 @@ describe('DefaultFieldConfigEditor', () => {
     const editors = queryAllByLabelText(selectors.components.PanelEditor.FieldOptions.propertyEditor('Custom'));
     expect(editors).toHaveLength(2);
   });
+
+  describe('categories', () => {
+    it('should render uncategorized options under panel category', () => {
+      const plugin = new PanelPlugin(() => null).useFieldConfig({
+        standardOptions: {},
+        useCustomConfig: b => {
+          b.addBooleanSwitch({
+            name: 'a',
+            path: 'a',
+          } as FieldConfigEditorConfig<FakeFieldOptions>)
+            .addBooleanSwitch({
+              name: 'c',
+              path: 'c',
+            } as FieldConfigEditorConfig<FakeFieldOptions>)
+            .addTextInput({
+              name: 'b',
+              path: 'b',
+            } as FieldConfigEditorConfig<FakeFieldOptions>);
+        },
+      });
+      plugin.meta.name = 'Test plugin';
+
+      const { queryAllByLabelText } = render(
+        <DefaultFieldConfigEditor data={[]} onChange={jest.fn()} plugin={plugin} config={fieldConfigMock} />
+      );
+
+      expect(
+        queryAllByLabelText(selectors.components.OptionsGroup.toggle(`${plugin.meta.name} options/0`))
+      ).toHaveLength(1);
+
+      expect(queryAllByLabelText(selectors.components.OptionsGroup.toggle(), { exact: false })).toHaveLength(1);
+    });
+
+    it('should render categorized options under custom category', () => {
+      const CATEGORY_NAME = 'Cat1';
+      const plugin = new PanelPlugin(() => null).useFieldConfig({
+        standardOptions: {},
+        useCustomConfig: b => {
+          b.addTextInput({
+            name: 'b',
+            path: 'b',
+          } as FieldConfigEditorConfig<FakeFieldOptions>)
+            .addBooleanSwitch({
+              name: 'a',
+              path: 'a',
+              category: [CATEGORY_NAME],
+            } as FieldConfigEditorConfig<FakeFieldOptions>)
+            .addBooleanSwitch({
+              name: 'c',
+              path: 'c',
+              category: [CATEGORY_NAME],
+            } as FieldConfigEditorConfig<FakeFieldOptions>);
+        },
+      });
+      plugin.meta.name = 'Test plugin';
+
+      const { queryAllByLabelText } = render(
+        <DefaultFieldConfigEditor data={[]} onChange={jest.fn()} plugin={plugin} config={fieldConfigMock} />
+      );
+
+      expect(
+        queryAllByLabelText(selectors.components.OptionsGroup.toggle(`${plugin.meta.name} options/0`))
+      ).toHaveLength(1);
+
+      expect(queryAllByLabelText(selectors.components.OptionsGroup.toggle(`${CATEGORY_NAME}/1`))).toHaveLength(1);
+
+      expect(queryAllByLabelText(selectors.components.OptionsGroup.toggle(), { exact: false })).toHaveLength(2);
+    });
+
+    it('should allow subcategories in panel category', () => {
+      const SUBCATEGORY_NAME = 'Sub1';
+      const plugin = new PanelPlugin(() => null).useFieldConfig({
+        standardOptions: {},
+        useCustomConfig: b => {
+          b.addTextInput({
+            name: 'b',
+            path: 'b',
+            category: [undefined, SUBCATEGORY_NAME],
+          } as FieldConfigEditorConfig<FakeFieldOptions>)
+            .addBooleanSwitch({
+              name: 'a',
+              path: 'a',
+            } as FieldConfigEditorConfig<FakeFieldOptions>)
+            .addBooleanSwitch({
+              name: 'c',
+              path: 'c',
+            } as FieldConfigEditorConfig<FakeFieldOptions>);
+        },
+      });
+      plugin.meta.name = 'Test plugin';
+
+      const { queryAllByLabelText, queryAllByText } = render(
+        <DefaultFieldConfigEditor data={[]} onChange={jest.fn()} plugin={plugin} config={fieldConfigMock} />
+      );
+
+      expect(
+        queryAllByLabelText(selectors.components.OptionsGroup.toggle(`${plugin.meta.name} options/0`))
+      ).toHaveLength(1);
+
+      expect(queryAllByText(SUBCATEGORY_NAME, { exact: false })).toHaveLength(1);
+    });
+
+    it('should allow subcategories in custom category', () => {
+      const CATEGORY_NAME = 'Cat1';
+      const SUBCATEGORY_NAME = 'Sub1';
+      const plugin = new PanelPlugin(() => null).useFieldConfig({
+        standardOptions: {},
+        useCustomConfig: b => {
+          b.addBooleanSwitch({
+            name: 'a',
+            path: 'a',
+          } as FieldConfigEditorConfig<FakeFieldOptions>)
+            .addBooleanSwitch({
+              name: 'c',
+              path: 'c',
+            } as FieldConfigEditorConfig<FakeFieldOptions>)
+            .addTextInput({
+              name: 'b',
+              path: 'b',
+              category: [CATEGORY_NAME, SUBCATEGORY_NAME],
+            } as FieldConfigEditorConfig<FakeFieldOptions>);
+        },
+      });
+      plugin.meta.name = 'Test plugin';
+
+      const { queryAllByLabelText, queryAllByText } = render(
+        <DefaultFieldConfigEditor data={[]} onChange={jest.fn()} plugin={plugin} config={fieldConfigMock} />
+      );
+
+      expect(
+        queryAllByLabelText(selectors.components.OptionsGroup.toggle(`${plugin.meta.name} options/0`))
+      ).toHaveLength(1);
+      expect(queryAllByLabelText(selectors.components.OptionsGroup.toggle(`${CATEGORY_NAME}/1`))).toHaveLength(1);
+
+      expect(queryAllByText(SUBCATEGORY_NAME, { exact: false })).toHaveLength(1);
+    });
+
+    it('should not render categories with hidden fields only', () => {
+      const CATEGORY_NAME = 'Cat1';
+      const SUBCATEGORY_NAME = 'Sub1';
+      const plugin = new PanelPlugin(() => null).useFieldConfig({
+        standardOptions: {},
+        useCustomConfig: b => {
+          b.addBooleanSwitch({
+            name: 'a',
+            path: 'a',
+          } as FieldConfigEditorConfig<FakeFieldOptions>)
+            .addBooleanSwitch({
+              name: 'c',
+              path: 'c',
+            } as FieldConfigEditorConfig<FakeFieldOptions>)
+            .addTextInput({
+              name: 'b',
+              path: 'b',
+              hideFromDefaults: true,
+              category: [CATEGORY_NAME, SUBCATEGORY_NAME],
+            } as FieldConfigEditorConfig<FakeFieldOptions>);
+        },
+      });
+      plugin.meta.name = 'Test plugin';
+
+      const { queryAllByLabelText, queryAllByText } = render(
+        <DefaultFieldConfigEditor data={[]} onChange={jest.fn()} plugin={plugin} config={fieldConfigMock} />
+      );
+
+      expect(
+        queryAllByLabelText(selectors.components.OptionsGroup.toggle(`${plugin.meta.name} options/0`))
+      ).toHaveLength(1);
+
+      expect(queryAllByLabelText(selectors.components.OptionsGroup.toggle(`${CATEGORY_NAME}/1`))).toHaveLength(0);
+
+      expect(queryAllByText(SUBCATEGORY_NAME, { exact: false })).toHaveLength(0);
+    });
+  });
 });
diff --git a/public/app/features/dashboard/components/PanelEditor/DefaultFieldConfigEditor.tsx b/public/app/features/dashboard/components/PanelEditor/DefaultFieldConfigEditor.tsx
index 946f7f04b33..8775198bb46 100644
--- a/public/app/features/dashboard/components/PanelEditor/DefaultFieldConfigEditor.tsx
+++ b/public/app/features/dashboard/components/PanelEditor/DefaultFieldConfigEditor.tsx
@@ -34,7 +34,7 @@ export const DefaultFieldConfigEditor: React.FC<Props> = ({ data, onChange, conf
         : get(defaults, item.path);
 
       let label: ReactNode | undefined = (
-        <Label description={item.description} category={item.category?.slice(1)}>
+        <Label description={item.description} category={item.category?.slice(1) as string[]}>
           {item.name}
         </Label>
       );
@@ -67,27 +67,39 @@ export const DefaultFieldConfigEditor: React.FC<Props> = ({ data, onChange, conf
     [config]
   );
 
-  const groupedConfigs = groupBy(plugin.fieldConfigRegistry.list(), i => i.category && i.category[0]);
+  const GENERAL_OPTIONS_CATEGORY = `${plugin.meta.name} options`;
+
+  const groupedConfigs = groupBy(plugin.fieldConfigRegistry.list(), i => {
+    if (!i.category) {
+      return GENERAL_OPTIONS_CATEGORY;
+    }
+    return i.category[0] ? i.category[0] : GENERAL_OPTIONS_CATEGORY;
+  });
 
   return (
     <div aria-label={selectors.components.FieldConfigEditor.content}>
-      {Object.keys(groupedConfigs).map((k, i) => {
-        const groupItemsCounter = countGroupItems(groupedConfigs[k], config);
+      {Object.keys(groupedConfigs).map((groupName, i) => {
+        const group = groupedConfigs[groupName];
+        const groupItemsCounter = countGroupItems(group, config);
+
+        if (!shouldRenderGroup(group)) {
+          return undefined;
+        }
 
         return (
           <OptionsGroup
             renderTitle={isExpanded => {
               return (
                 <>
-                  {k} {!isExpanded && groupItemsCounter && <Counter value={groupItemsCounter} />}
+                  {groupName} {!isExpanded && groupItemsCounter && <Counter value={groupItemsCounter} />}
                 </>
               );
             }}
-            id={`${k}/${i}`}
-            key={`${k}/${i}`}
+            id={`${groupName}/${i}`}
+            key={`${groupName}/${i}`}
           >
-            {groupedConfigs[k].map(c => {
-              return renderEditor(c, groupedConfigs[k].length);
+            {group.map(c => {
+              return renderEditor(c, group.length);
             })}
           </OptionsGroup>
         );
@@ -96,7 +108,7 @@ export const DefaultFieldConfigEditor: React.FC<Props> = ({ data, onChange, conf
   );
 };
 
-const countGroupItems = (group: FieldConfigPropertyItem[], config: FieldConfigSource) => {
+function countGroupItems(group: FieldConfigPropertyItem[], config: FieldConfigSource) {
   let counter = 0;
 
   for (const item of group) {
@@ -111,4 +123,9 @@ const countGroupItems = (group: FieldConfigPropertyItem[], config: FieldConfigSo
   }
 
   return counter === 0 ? undefined : counter;
-};
+}
+
+function shouldRenderGroup(group: FieldConfigPropertyItem[]) {
+  const hiddenPropertiesCount = group.filter(i => i.hideFromDefaults).length;
+  return group.length - hiddenPropertiesCount > 0;
+}
diff --git a/public/app/features/dashboard/components/PanelEditor/DynamicConfigValueEditor.tsx b/public/app/features/dashboard/components/PanelEditor/DynamicConfigValueEditor.tsx
index cad57c6c8a7..b41404ec60f 100644
--- a/public/app/features/dashboard/components/PanelEditor/DynamicConfigValueEditor.tsx
+++ b/public/app/features/dashboard/components/PanelEditor/DynamicConfigValueEditor.tsx
@@ -35,7 +35,10 @@ export const DynamicConfigValueEditor: React.FC<DynamicConfigValueEditorProps> =
   // eslint-disable-next-line react/display-name
   const renderLabel = (includeDescription = true, includeCounter = false) => (isExpanded = false) => (
     <HorizontalGroup justify="space-between">
-      <Label category={item.category?.splice(1)} description={includeDescription ? item.description : undefined}>
+      <Label
+        category={item.category?.filter(c => c !== undefined) as string[]}
+        description={includeDescription ? item.description : undefined}
+      >
         {item.name}
         {!isExpanded && includeCounter && item.getItemsCount && <Counter value={item.getItemsCount(property.value)} />}
       </Label>
diff --git a/public/app/features/dashboard/components/PanelEditor/PanelOptionsEditor.tsx b/public/app/features/dashboard/components/PanelEditor/PanelOptionsEditor.tsx
index b758fc7a03c..2bcbe6eef02 100644
--- a/public/app/features/dashboard/components/PanelEditor/PanelOptionsEditor.tsx
+++ b/public/app/features/dashboard/components/PanelEditor/PanelOptionsEditor.tsx
@@ -22,7 +22,7 @@ interface PanelOptionsEditorProps<TOptions> {
   options: TOptions;
   onChange: (options: TOptions) => void;
 }
-
+const DISPLAY_OPTIONS_CATEGORY = 'Display';
 export const PanelOptionsEditor: React.FC<PanelOptionsEditorProps<any>> = ({
   plugin,
   options,
@@ -33,7 +33,10 @@ export const PanelOptionsEditor: React.FC<PanelOptionsEditorProps<any>> = ({
 }) => {
   const optionEditors = useMemo<Record<string, PanelOptionsEditorItem[]>>(() => {
     return groupBy(plugin.optionEditors.list(), i => {
-      return i.category ? i.category[0] : 'Display';
+      if (!i.category) {
+        return DISPLAY_OPTIONS_CATEGORY;
+      }
+      return i.category[0] ? i.category[0] : DISPLAY_OPTIONS_CATEGORY;
     });
   }, [plugin]);
 
@@ -62,7 +65,7 @@ export const PanelOptionsEditor: React.FC<PanelOptionsEditorProps<any>> = ({
             }
 
             const label = (
-              <Label description={e.description} category={e.category?.slice(1)}>
+              <Label description={e.description} category={e.category?.slice(1) as string[]}>
                 {e.name}
               </Label>
             );