apache / superset

Apache Superset is a Data Visualization and Data Exploration Platform
https://superset.apache.org/
Apache License 2.0
61.98k stars 13.59k forks source link

X-Axis title disappears when editing bar chart #30370

Open BGZ-spec opened 4 days ago

BGZ-spec commented 4 days ago

Bug description

Description

When editing a bar chart in Superset, changing the X-axis title causes the title to disappear. Similarly, adjusting the margins leads to unexpected behavior—they either reset or are not applied correctly.

How to reproduce the bug

Open an existing bar chart in Superset or create a new one.
Enter the edit mode for the chart.
Change the X-axis title to a new value.
Observe that the X-axis title disappears from the chart.
Adjust the margin settings.
Notice that the margins do not update as expected or revert to previous settings.

Screenshots/recordings

Screencast from 2024-09-24 10-57-58.webm

Superset version

master / latest-dev

Python version

3.9

Node version

16

Browser

Firefox

Additional context

No response

Checklist

dosubot[bot] commented 4 days ago

I found a similar closed issue that might be relevant to your problem: [chart viz][Time-series bar chart v2] chart will shrink when input customize X AXIS TITLE MARGIN or Y AXIS TITLE MARGIN. This issue was resolved by a pull request that fixed the customize margin functionality [1].

To fix the issue where the X-axis title disappears and margins reset or do not apply correctly when editing a bar chart in Superset, ensure that the x_axis_title and x_axis_title_margin controls are properly configured and rendered. Here is the relevant code snippet:

import { t } from '@superset-ui/core';
import {
  ControlPanelsContainerProps,
  ControlStateMapping,
  ControlSubSectionHeader,
  formatSelectOptions,
  sections,
} from '@superset-ui/chart-controls';
import { OrientationType } from '../../types';

function createAxisTitleControl(axis: 'x' | 'y') {
  const isXAxis = axis === 'x';
  const isVertical = (controls: ControlStateMapping) =>
    Boolean(controls?.orientation.value === OrientationType.Vertical);
  const isHorizontal = (controls: ControlStateMapping) =>
    Boolean(controls?.orientation.value === OrientationType.Horizontal);

  return [
    [
      {
        name: 'x_axis_title',
        config: {
          type: 'TextControl',
          label: t('Axis Title'),
          renderTrigger: true,
          default: '',
          description: t('Changing this control takes effect instantly'),
          visibility: ({ controls }: ControlPanelsContainerProps) =>
            isXAxis ? isVertical(controls) : isHorizontal(controls),
        },
      },
    ],
    [
      {
        name: 'x_axis_title_margin',
        config: {
          type: 'SelectControl',
          freeForm: true,
          clearable: true,
          label: t('AXIS TITLE MARGIN'),
          renderTrigger: true,
          default: sections.TITLE_MARGIN_OPTIONS[0],
          choices: formatSelectOptions(sections.TITLE_MARGIN_OPTIONS),
          description: t('Changing this control takes effect instantly'),
          visibility: ({ controls }: ControlPanelsContainerProps) =>
            isXAxis ? isVertical(controls) : isHorizontal(controls),
        },
      },
    ],
  ];
}

const config = {
  controlPanelSections: [
    {
      label: t('Chart Title'),
      tabOverride: 'customize',
      expanded: true,
      controlSetRows: [
        [<ControlSubSectionHeader>{t('X Axis')}</ControlSubSectionHeader>],
        ...createAxisTitleControl('x'),
      ],
    },
  ],
};

export default config;

Ensure that the x_axis_title and x_axis_title_margin controls are correctly set up in your chart configuration. The renderTrigger: true property ensures that changes to these controls will trigger a re-render of the chart, which should apply the updated title and margins correctly. Additionally, the visibility property ensures that these controls are only visible when the chart orientation is appropriate, preventing any misconfiguration [2][3][4].

To continue talking to Dosu, mention @dosu.

BGZ-spec commented 3 days ago

@dosu I have the original version of the code and haven't changed anything. I downloaded a clean, fresh version of Superset.

controlSetRows: [
        [<ControlSubSectionHeader>{t('X Axis')}</ControlSubSectionHeader>],
        ...createAxisTitleControl('x'),
        [<ControlSubSectionHeader>{t('Y Axis')}</ControlSubSectionHeader>],
        ...createAxisTitleControl('y'),
      ],

When I comment out ...createAxisTitleControl('y'), changing the X-axis title works correctly.

dosubot[bot] commented 3 days ago

The issue arises because the visibility conditions for the X-axis and Y-axis titles are interdependent. Specifically, the visibility of the X-axis title controls depends on the chart orientation being vertical, and the visibility of the Y-axis title controls depends on the chart orientation being horizontal. When both createAxisTitleControl('x') and createAxisTitleControl('y') are used, they might conflict due to these interdependencies.

Here is the relevant code snippet showing the visibility conditions:

function createAxisTitleControl(axis: 'x' | 'y'): ControlSetRow[] {
  const isXAxis = axis === 'x';
  const isVertical = (controls: ControlStateMapping) =>
    Boolean(controls?.orientation.value === OrientationType.Vertical);
  const isHorizontal = (controls: ControlStateMapping) =>
    Boolean(controls?.orientation.value === OrientationType.Horizontal);
  return [
    [
      {
        name: 'x_axis_title',
        config: {
          type: 'TextControl',
          label: t('Axis Title'),
          renderTrigger: true,
          default: '',
          description: t('Changing this control takes effect instantly'),
          visibility: ({ controls }: ControlPanelsContainerProps) =>
            isXAxis ? isVertical(controls) : isHorizontal(controls),
        },
      },
    ],
    [
      {
        name: 'x_axis_title_margin',
        config: {
          type: 'SelectControl',
          freeForm: true,
          clearable: true,
          label: t('AXIS TITLE MARGIN'),
          renderTrigger: true,
          default: sections.TITLE_MARGIN_OPTIONS[0],
          choices: formatSelectOptions(sections.TITLE_MARGIN_OPTIONS),
          description: t('Changing this control takes effect instantly'),
          visibility: ({ controls }: ControlPanelsContainerProps) =>
            isXAxis ? isVertical(controls) : isHorizontal(controls),
        },
      },
    ],
    [
      {
        name: 'y_axis_title',
        config: {
          type: 'TextControl',
          label: t('Axis Title'),
          renderTrigger: true,
          default: '',
          description: t('Changing this control takes effect instantly'),
          visibility: ({ controls }: ControlPanelsContainerProps) =>
            isXAxis ? isHorizontal(controls) : isVertical(controls),
        },
      },
    ],
    [
      {
        name: 'y_axis_title_margin',
        config: {
          type: 'SelectControl',
          freeForm: true,
          clearable: true,
          label: t('AXIS TITLE MARGIN'),
          renderTrigger: true,
          default: sections.TITLE_MARGIN_OPTIONS[0],
          choices: formatSelectOptions(sections.TITLE_MARGIN_OPTIONS),
          description: t('Changing this control takes effect instantly'),
          visibility: ({ controls }: ControlPanelsContainerProps) =>
            isXAxis ? isHorizontal(controls) : isVertical(controls),
        },
      },
    ],
  ];
}

When you comment out ...createAxisTitleControl('y'), the conflicting visibility conditions are removed, allowing the X-axis title to work correctly [1].

To continue talking to Dosu, mention @dosu.