[Explore Vis] Separate histogram from bar (opensearch-project#10823)

---------

Signed-off-by: Qxisylolo <[email protected]>
Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>

Author: Qxisylolo

changelogs/fragments/10823.yml

@@ -0,0 +1,2 @@
+fix:
+- Separate histogram from bar ([#10823](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/10823))

src/plugins/explore/public/components/visualizations/bar/bar_exclusive_vis_options.test.tsx

@@ -16,6 +16,7 @@ jest.mock('../utils/use_debounced_value', () => ({
 
 describe('BarExclusiveVisOptions', () => {
   const defaultProps = {
+    type: 'bar',
     barSizeMode: 'manual' as 'manual' | 'auto',
     barWidth: 0.7,
     barPadding: 0.1,

src/plugins/explore/public/components/visualizations/bar/bar_exclusive_vis_options.tsx

@@ -19,6 +19,7 @@ import { DebouncedFieldNumber } from '../style_panel/utils';
 import { defaultBarChartStyles } from './bar_vis_config';
 
 interface BarExclusiveVisOptionsProps {
+  type: 'bar' | 'histogram';
   barSizeMode: 'auto' | 'manual';
   barWidth: number;
   barPadding: number;
@@ -37,6 +38,7 @@ interface BarExclusiveVisOptionsProps {
 }
 
 export const BarExclusiveVisOptions = ({
+  type,
   barSizeMode,
   barWidth,
   barPadding,
@@ -68,14 +70,17 @@ export const BarExclusiveVisOptions = ({
     },
   ];
 
+  const barAccordionMessage =
+    type === 'bar'
+      ? i18n.translate('explore.vis.barChart.exclusiveSettings', {
+          defaultMessage: 'Bar',
+        })
+      : i18n.translate('explore.vis.histogramChart.exclusiveSettings', {
+          defaultMessage: 'Histogram',
+        });
+
   return (
-    <StyleAccordion
-      id="barSection"
-      accordionLabel={i18n.translate('explore.vis.barChart.exclusiveSettings', {
-        defaultMessage: 'Bar',
-      })}
-      initialIsOpen={true}
-    >
+    <StyleAccordion id="barSection" accordionLabel={barAccordionMessage} initialIsOpen={true}>
       {!shouldDisableUseThresholdColor && (
         <EuiFormRow>
           <EuiSwitch

src/plugins/explore/public/components/visualizations/bar/bar_vis_config.ts

@@ -234,9 +234,6 @@ export const createBarConfig = (): VisualizationType<'bar'> => ({
         [AxisRole.X]: { type: VisFieldType.Numerical, index: 0 },
         [AxisRole.Y]: { type: VisFieldType.Numerical, index: 1 },
       },
-      {
-        [AxisRole.X]: { type: VisFieldType.Numerical, index: 0 },
-      },
     ],
   },
 });

src/plugins/explore/public/components/visualizations/bar/bar_vis_options.test.tsx

@@ -536,19 +536,11 @@ describe('BarVisStyleControls', () => {
     });
   });
 
-  test('render bucket panel with num bucket type', async () => {
+  test('render bucket panel with category bucket', async () => {
     const propsWithNumBucket = {
       ...defaultProps,
       axisColumnMappings: {
         ...mockAxisColumnMappings,
-        x: {
-          id: 1,
-          name: 'Numerical X',
-          schema: VisFieldType.Numerical,
-          column: 'numX',
-          validValuesCount: 100,
-          uniqueValuesCount: 50,
-        },
       },
     };
 
@@ -559,21 +551,9 @@ describe('BarVisStyleControls', () => {
     );
 
     expect(screen.getByTestId('mockBucketOptionsPanel')).toBeInTheDocument();
-    expect(screen.getByTestId('bucketType')).toHaveTextContent('num');
-    expect(screen.getByTestId('mockUpdateBucketSize')).toBeInTheDocument();
-    expect(screen.getByTestId('mockUpdateBucketCount')).toBeInTheDocument();
+    expect(screen.getByTestId('bucketType')).toHaveTextContent('cate');
     expect(screen.getByTestId('mockUpdateAggregation')).toBeInTheDocument();
 
-    await userEvent.click(screen.getByTestId('mockUpdateBucketSize'));
-    expect(defaultProps.onStyleChange).toHaveBeenCalledWith({
-      bucket: { bucketTimeUnit: 'auto', aggregationType: 'sum', bucketSize: 100 },
-    });
-
-    await userEvent.click(screen.getByTestId('mockUpdateBucketCount'));
-    expect(defaultProps.onStyleChange).toHaveBeenCalledWith({
-      bucket: { bucketTimeUnit: 'auto', aggregationType: 'sum', bucketCount: 20 },
-    });
-
     await userEvent.click(screen.getByTestId('mockUpdateAggregation'));
     expect(defaultProps.onStyleChange).toHaveBeenCalledWith({
       bucket: { bucketTimeUnit: 'auto', aggregationType: 'sum' },

src/plugins/explore/public/components/visualizations/bar/bar_vis_options.tsx

@@ -39,22 +39,14 @@ export const BarVisStyleControls: React.FC<BarVisStyleControlsProps> = ({
   };
 
   const axes = [axisColumnMappings[AxisRole.X], axisColumnMappings[AxisRole.Y]];
-  const hasCategory = axes.some((axis) => axis?.schema === VisFieldType.Categorical);
-  const hasNum = axes.some((axis) => axis?.schema === VisFieldType.Numerical);
   const hasDate = axes.some((axis) => axis?.schema === VisFieldType.Date);
 
-  // 4 bucket types for bar chart:
-  // 1. time-numerical(Regular histogram): requires one axis to be date type, bucket options: time interval + aggregationType
+  // 3 bucket types for bar chart:
+  // 1. time-numerical: requires one axis to be date type, bucket options: time interval + aggregationType
   // 2. categorical-numerical: requires one axis to be categorical type, bucket options: aggregationType only
-  // 3. numerical-numerical: both x and y axes are numerical, bucket options: bucket size + bucket count + aggregationType
-  // 4. single-numerical: only x-axis mapped (numerical), y-axis displays count of records, bucket options: bucket size + bucket count
-  const bucketType = hasDate
-    ? 'time'
-    : hasCategory
-    ? 'cate'
-    : hasNum && axisColumnMappings[AxisRole.Y] !== undefined
-    ? 'num'
-    : 'single';
+  // 3. numerical-numerical: both x and y axes are numerical, treated as categorical, bucket options: aggregationType only
+  // Note: single-numerical has been moved to histogram
+  const bucketType = hasDate ? 'time' : 'cate';
 
   // The mapping object will be an empty object if no fields are selected on the axes selector. No
   // visualization is generated in this case so we shouldn't display style option panels.
@@ -78,6 +70,7 @@ export const BarVisStyleControls: React.FC<BarVisStyleControlsProps> = ({
         <>
           <EuiFlexItem grow={false}>
             <BarExclusiveVisOptions
+              type="bar"
               barSizeMode={styleOptions.barSizeMode}
               barWidth={styleOptions.barWidth}
               barPadding={styleOptions.barPadding}

src/plugins/explore/public/components/visualizations/bar/to_expression.test.ts

@@ -9,8 +9,7 @@ import {
   createTimeBarChart,
   createGroupedTimeBarChart,
   createFacetedTimeBarChart,
-  createNumericalHistogramBarChart,
-  createSingleBarChart,
+  createDoubleNumericalBarChart,
 } from './to_expression';
 import { defaultBarChartStyles, BarChartStyle } from './bar_vis_config';
 import {
@@ -1151,7 +1150,7 @@ describe('bar to_expression', () => {
     });
   });
 
-  describe('createNumericalHistogramBarChart', () => {
+  describe('createDoubleNumericalBarChart', () => {
     const mockNumericalColumn2 = {
       id: 2,
       name: 'sum',
@@ -1166,7 +1165,7 @@ describe('bar to_expression', () => {
       [AxisRole.Y]: mockNumericalColumn2,
     };
     test('creates a numerical histogram bar chart spec', () => {
-      const spec = createNumericalHistogramBarChart(
+      const spec = createDoubleNumericalBarChart(
         mockData,
         [mockNumericalColumn, mockNumericalColumn2],
         defaultBarChartStyles,
@@ -1181,7 +1180,7 @@ describe('bar to_expression', () => {
       expect(mainLayer.mark.type).toBe('bar');
       expect(mainLayer.mark.tooltip).toBe(true);
       expect(mainLayer.encoding.x.field).toBe('count');
-      expect(mainLayer.encoding.x.type).toBe('quantitative');
+      expect(mainLayer.encoding.x.type).toBe('nominal');
       expect(mainLayer.encoding.y.field).toBe('sum');
       expect(mainLayer.encoding.y.aggregate).toBe('sum');
       expect(mainLayer.encoding.y.type).toBe('quantitative');
@@ -1199,73 +1198,18 @@ describe('bar to_expression', () => {
       const stylesWithBucket = {
         ...defaultBarChartStyles,
         bucket: {
-          bucketSize: 50,
           aggregationType: AggregationType.SUM,
         },
       };
 
-      const spec = createNumericalHistogramBarChart(
+      const spec = createDoubleNumericalBarChart(
         mockData,
         [mockNumericalColumn, mockNumericalColumn2],
         stylesWithBucket,
         mockAxisColumnMappings
       );
 
-      expect(spec.layer[0].encoding.x.bin.step).toBe(50);
       expect(spec.layer[0].encoding.y.aggregate).toBe('sum');
     });
   });
-
-  describe('createSingleBarChart', () => {
-    const mockAxisColumnMappings = {
-      [AxisRole.X]: mockNumericalColumn,
-    };
-    test('creates a numerical histogram bar chart spec', () => {
-      const spec = createSingleBarChart(
-        mockData,
-        [mockNumericalColumn],
-        defaultBarChartStyles,
-        mockAxisColumnMappings
-      );
-
-      expect(spec.$schema).toBe(VEGASCHEMA);
-      expect(spec.data.values).toBe(mockData);
-      expect(spec.layer).toHaveLength(1);
-
-      const mainLayer = spec.layer[0];
-      expect(mainLayer.mark.type).toBe('bar');
-      expect(mainLayer.mark.tooltip).toBe(true);
-      expect(mainLayer.encoding.x.field).toBe('count');
-      expect(mainLayer.encoding.x.type).toBe('quantitative');
-      expect(spec.layer[0].encoding.y.aggregate).toBe('count');
-
-      // select time range params
-      expect(spec.params).not.toEqual(
-        expect.arrayContaining([expect.objectContaining({ name: 'applyTimeFilter' })])
-      );
-      expect(mainLayer.params).not.toEqual(
-        expect.arrayContaining([expect.objectContaining({ name: 'timeRangeBrush' })])
-      );
-    });
-
-    test('applies bucket options correctly, single bar aggregation should only be count', () => {
-      const stylesWithBucket = {
-        ...defaultBarChartStyles,
-        bucket: {
-          bucketSize: 50,
-          aggregationType: AggregationType.SUM,
-        },
-      };
-
-      const spec = createSingleBarChart(
-        mockData,
-        [mockNumericalColumn],
-        stylesWithBucket,
-        mockAxisColumnMappings
-      );
-
-      expect(spec.layer[0].encoding.x.bin.step).toBe(50);
-      expect(spec.layer[0].encoding.y.aggregate).toBe('count');
-    });
-  });
 });

src/plugins/explore/public/components/visualizations/bar/to_expression.ts

@@ -10,7 +10,6 @@ import {
   VisColumn,
   VisFieldType,
   TimeUnit,
-  AggregationType,
 } from '../types';
 import { BarChartStyle, defaultBarChartStyles } from './bar_vis_config';
 import { createThresholdLayer } from '../style_panel/threshold/threshold_utils';
@@ -25,7 +24,6 @@ import {
   inferTimeIntervals,
   buildEncoding,
   buildTooltipEncoding,
-  adjustBucketBins,
   buildThresholdColorEncoding,
 } from './bar_chart_utils';
 import { DEFAULT_OPACITY } from '../constants';
@@ -595,7 +593,7 @@ export const createStackedBarSpec = (
   return spec;
 };
 
-export const createNumericalHistogramBarChart = (
+export const createDoubleNumericalBarChart = (
   transformedData: Array<Record<string, any>>,
   numericalColumns: VisColumn[],
   styleOptions: BarChartStyle,
@@ -632,8 +630,7 @@ export const createNumericalHistogramBarChart = (
     encoding: {
       x: {
         field: xAxis?.column,
-        type: getSchemaByAxis(xAxis),
-        bin: adjustBucketBins(styles?.bucket, transformedData, xAxis?.column),
+        type: 'nominal',
         axis: applyAxisStyling({ axis: xAxis, axisStyle: xAxisStyle }),
       },
       y: {
@@ -647,8 +644,7 @@ export const createNumericalHistogramBarChart = (
         tooltip: [
           {
             field: xAxis?.column,
-            type: getSchemaByAxis(xAxis),
-            bin: adjustBucketBins(styles?.bucket, transformedData, xAxis?.column),
+            type: 'nominal',
             title: xAxisStyle?.title?.text || xAxis?.name,
           },
           {
@@ -680,85 +676,3 @@ export const createNumericalHistogramBarChart = (
     layer: layers,
   };
 };
-
-export const createSingleBarChart = (
-  transformedData: Array<Record<string, any>>,
-  numericalColumns: VisColumn[],
-  styleOptions: BarChartStyle,
-  axisColumnMappings?: AxisColumnMappings
-): any => {
-  // Check if we have the required columns
-  if (numericalColumns.length < 1) {
-    throw new Error('Histogram bar chart requires at least one numerical column');
-  }
-
-  const styles = { ...defaultBarChartStyles, ...styleOptions };
-  const { xAxis, xAxisStyle, yAxis, yAxisStyle } = getSwappedAxisRole(styles, axisColumnMappings);
-
-  const layers: any[] = [];
-
-  // Configure bar mark
-  const barMark: any = {
-    type: 'bar',
-    tooltip: styles.tooltipOptions?.mode !== 'hidden',
-  };
-
-  const colorEncodingLayer = buildThresholdColorEncoding(undefined, styleOptions);
-
-  configureBarSizeAndSpacing(barMark, styles);
-
-  // Add border if enabled
-  if (styles.showBarBorder) {
-    barMark.stroke = styles.barBorderColor || '#000000';
-    barMark.strokeWidth = styles.barBorderWidth || 1;
-  }
-
-  const mainLayer = {
-    mark: barMark,
-    encoding: {
-      x: {
-        field: xAxis?.column,
-        type: getSchemaByAxis(xAxis),
-        bin: adjustBucketBins(styles?.bucket, transformedData, xAxis?.column),
-        axis: applyAxisStyling({ axis: xAxis, axisStyle: xAxisStyle }),
-      },
-      y: {
-        aggregate: AggregationType.COUNT,
-        type: 'quantitative',
-        axis: applyAxisStyling({ axis: yAxis, axisStyle: yAxisStyle }),
-      },
-      color: styleOptions?.useThresholdColor ? colorEncodingLayer : [],
-      ...(styles.tooltipOptions?.mode !== 'hidden' && {
-        tooltip: [
-          {
-            field: xAxis?.column,
-            type: getSchemaByAxis(xAxis),
-            bin: adjustBucketBins(styles?.bucket, transformedData, xAxis?.column),
-            title: xAxisStyle?.title?.text || xAxis?.name,
-          },
-          {
-            aggregate: AggregationType.COUNT,
-            title: yAxisStyle?.title?.text || yAxis?.name,
-          },
-        ],
-      }),
-    },
-  };
-
-  layers.push(mainLayer);
-
-  // Add threshold layer if enabled
-  const thresholdLayer = createThresholdLayer(styles?.thresholdOptions, 'y');
-  if (thresholdLayer) {
-    layers.push(...thresholdLayer.layer);
-  }
-
-  return {
-    $schema: VEGASCHEMA,
-    title: styles.titleOptions?.show
-      ? styles.titleOptions?.titleName || `Record counts of ${xAxis?.name}`
-      : undefined,
-    data: { values: transformedData },
-    layer: layers,
-  };
-};

src/plugins/explore/public/components/visualizations/constants.ts

@@ -19,6 +19,8 @@ export const CHART_METADATA: Record<ChartType, ChartMetadata> = {
   gauge: { type: 'gauge', name: 'Gauge', icon: 'visGauge' },
   state_timeline: { type: 'state_timeline', name: 'State timeline', icon: 'visBarHorizontal' },
   bar_gauge: { type: 'bar_gauge', name: 'Bar Gauge', icon: 'visBarHorizontal' },
+  // TODO add histogram icons
+  histogram: { type: 'histogram', name: 'Histogram', icon: 'visBarVertical' },
 };
 
 // Map both OSD_FIELD_TYPES and OPENSEARCH_FIELD_TYPES to VisFieldType