This is an automated email from the ASF dual-hosted git repository.
enzomartellucci pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 9555798d37 fix(data-zoom): Data-zoom not rendered properly in
Matrixify (#37134)
9555798d37 is described below
commit 9555798d37ddd1af3a61d3bdb3cfb13fee3333e5
Author: Alexandru Soare <[email protected]>
AuthorDate: Fri Jan 16 15:55:21 2026 +0200
fix(data-zoom): Data-zoom not rendered properly in Matrixify (#37134)
---
.../src/chart/components/StatefulChart.test.tsx | 248 +++++++++++++++++++++
.../src/chart/components/StatefulChart.tsx | 26 ++-
2 files changed, 273 insertions(+), 1 deletion(-)
diff --git
a/superset-frontend/packages/superset-ui-core/src/chart/components/StatefulChart.test.tsx
b/superset-frontend/packages/superset-ui-core/src/chart/components/StatefulChart.test.tsx
index b02d3bb04a..3d73326b45 100644
---
a/superset-frontend/packages/superset-ui-core/src/chart/components/StatefulChart.test.tsx
+++
b/superset-frontend/packages/superset-ui-core/src/chart/components/StatefulChart.test.tsx
@@ -472,6 +472,254 @@ test('should handle chartId changes', async () => {
});
});
+test('should NOT refetch data when string-based renderTrigger control
(zoomable) changes', async () => {
+ // Control panel with zoomable as a string reference (like ['zoomable'] in
control panels)
+ const controlPanelConfig = {
+ controlPanelSections: [
+ {
+ controlSetRows: [
+ ['zoomable'], // String reference to shared control
+ [
+ {
+ name: 'metrics',
+ config: {
+ renderTrigger: false,
+ },
+ },
+ ],
+ ],
+ },
+ ],
+ };
+
+ (getChartControlPanelRegistry as any).mockReturnValue({
+ get: jest.fn().mockReturnValue(controlPanelConfig),
+ });
+
+ const formDataWithZoom = {
+ ...mockFormData,
+ zoomable: false,
+ };
+
+ const { rerender, getByTestId } = render(
+ <StatefulChart formData={formDataWithZoom} chartType="test_chart" />,
+ );
+
+ await waitFor(() => {
+ expect(mockChartClient.client.post).toHaveBeenCalledTimes(1);
+ });
+
+ // Toggle zoomable (string-based shared control with renderTrigger: true)
+ const updatedFormData = {
+ ...formDataWithZoom,
+ zoomable: true,
+ };
+
+ rerender(<StatefulChart formData={updatedFormData} chartType="test_chart"
/>);
+
+ await waitFor(() => {
+ // Should NOT refetch data - zoomable is a renderTrigger control
+ expect(mockChartClient.client.post).toHaveBeenCalledTimes(1);
+ // But should re-render with new formData
+ expect(getByTestId('super-chart')).toHaveTextContent(
+ JSON.stringify(updatedFormData),
+ );
+ });
+});
+
+test('should NOT refetch data when other string-based renderTrigger controls
change', async () => {
+ // Test other controls in RENDER_TRIGGER_SHARED_CONTROLS set
+ const controlPanelConfig = {
+ controlPanelSections: [
+ {
+ controlSetRows: [
+ ['color_scheme'], // String reference
+ ['y_axis_format'], // String reference
+ ['currency_format'], // String reference
+ ['time_shift_color'], // String reference
+ ],
+ },
+ ],
+ };
+
+ (getChartControlPanelRegistry as any).mockReturnValue({
+ get: jest.fn().mockReturnValue(controlPanelConfig),
+ });
+
+ const { rerender, getByTestId } = render(
+ <StatefulChart formData={mockFormData} chartType="test_chart" />,
+ );
+
+ await waitFor(() => {
+ expect(mockChartClient.client.post).toHaveBeenCalledTimes(1);
+ });
+
+ // Change multiple string-based renderTrigger controls
+ const updatedFormData = {
+ ...mockFormData,
+ color_scheme: 'new_scheme',
+ y_axis_format: '.2f',
+ };
+
+ rerender(<StatefulChart formData={updatedFormData} chartType="test_chart"
/>);
+
+ await waitFor(() => {
+ // Should NOT refetch data
+ expect(mockChartClient.client.post).toHaveBeenCalledTimes(1);
+ // But should re-render
+ expect(getByTestId('super-chart')).toHaveTextContent(
+ JSON.stringify(updatedFormData),
+ );
+ });
+});
+
+test('should refetch when string control is NOT in
RENDER_TRIGGER_SHARED_CONTROLS', async () => {
+ // Control panel with a string control that is NOT in the renderTrigger set
+ const controlPanelConfig = {
+ controlPanelSections: [
+ {
+ controlSetRows: [
+ ['some_unknown_control'], // Not in RENDER_TRIGGER_SHARED_CONTROLS
+ ],
+ },
+ ],
+ };
+
+ (getChartControlPanelRegistry as any).mockReturnValue({
+ get: jest.fn().mockReturnValue(controlPanelConfig),
+ });
+
+ const { rerender } = render(
+ <StatefulChart formData={mockFormData} chartType="test_chart" />,
+ );
+
+ await waitFor(() => {
+ expect(mockChartClient.client.post).toHaveBeenCalledTimes(1);
+ });
+
+ // Change the unknown control
+ const updatedFormData = {
+ ...mockFormData,
+ some_unknown_control: 'new_value',
+ };
+
+ rerender(<StatefulChart formData={updatedFormData} chartType="test_chart"
/>);
+
+ await waitFor(() => {
+ // Should refetch because the control is not recognized as renderTrigger
+ expect(mockChartClient.client.post).toHaveBeenCalledTimes(2);
+ });
+});
+
+test('should handle mixed string and object controls correctly', async () => {
+ // Control panel with both string references and object definitions
+ const controlPanelConfig = {
+ controlPanelSections: [
+ {
+ controlSetRows: [
+ ['zoomable'], // String reference (in RENDER_TRIGGER_SHARED_CONTROLS)
+ [
+ {
+ name: 'minorTicks',
+ config: {
+ renderTrigger: true,
+ },
+ },
+ ], // Object definition
+ ],
+ },
+ ],
+ };
+
+ (getChartControlPanelRegistry as any).mockReturnValue({
+ get: jest.fn().mockReturnValue(controlPanelConfig),
+ });
+
+ const formDataWithControls = {
+ ...mockFormData,
+ zoomable: false,
+ minorTicks: false,
+ };
+
+ const { rerender, getByTestId } = render(
+ <StatefulChart formData={formDataWithControls} chartType="test_chart" />,
+ );
+
+ await waitFor(() => {
+ expect(mockChartClient.client.post).toHaveBeenCalledTimes(1);
+ });
+
+ // Change both string-based and object-based renderTrigger controls
+ const updatedFormData = {
+ ...formDataWithControls,
+ zoomable: true,
+ minorTicks: true,
+ };
+
+ rerender(<StatefulChart formData={updatedFormData} chartType="test_chart"
/>);
+
+ await waitFor(() => {
+ // Should NOT refetch - both are renderTrigger controls
+ expect(mockChartClient.client.post).toHaveBeenCalledTimes(1);
+ // But should re-render
+ expect(getByTestId('super-chart')).toHaveTextContent(
+ JSON.stringify(updatedFormData),
+ );
+ });
+});
+
+test('should refetch when mixing renderTrigger string control with
non-renderTrigger change', async () => {
+ const controlPanelConfig = {
+ controlPanelSections: [
+ {
+ controlSetRows: [
+ ['zoomable'], // String reference (renderTrigger)
+ [
+ {
+ name: 'metrics',
+ config: {
+ renderTrigger: false,
+ },
+ },
+ ],
+ ],
+ },
+ ],
+ };
+
+ (getChartControlPanelRegistry as any).mockReturnValue({
+ get: jest.fn().mockReturnValue(controlPanelConfig),
+ });
+
+ const formDataWithZoom = {
+ ...mockFormData,
+ zoomable: false,
+ metrics: ['metric1'],
+ };
+
+ const { rerender } = render(
+ <StatefulChart formData={formDataWithZoom} chartType="test_chart" />,
+ );
+
+ await waitFor(() => {
+ expect(mockChartClient.client.post).toHaveBeenCalledTimes(1);
+ });
+
+ // Change both zoomable (renderTrigger) and metrics (non-renderTrigger)
+ const updatedFormData = {
+ ...formDataWithZoom,
+ zoomable: true,
+ metrics: ['metric2'],
+ };
+
+ rerender(<StatefulChart formData={updatedFormData} chartType="test_chart"
/>);
+
+ await waitFor(() => {
+ // Should refetch because metrics changed (non-renderTrigger)
+ expect(mockChartClient.client.post).toHaveBeenCalledTimes(2);
+ });
+});
+
test('should display error message when HTTP request fails with Response
object', async () => {
const errorBody = JSON.stringify({ message: 'Error: division by zero' });
const mockResponse = new Response(errorBody, {
diff --git
a/superset-frontend/packages/superset-ui-core/src/chart/components/StatefulChart.tsx
b/superset-frontend/packages/superset-ui-core/src/chart/components/StatefulChart.tsx
index 83ddb688b9..d9ee87e9ea 100644
---
a/superset-frontend/packages/superset-ui-core/src/chart/components/StatefulChart.tsx
+++
b/superset-frontend/packages/superset-ui-core/src/chart/components/StatefulChart.tsx
@@ -37,6 +37,24 @@ import SuperChart from './SuperChart';
// Using more specific states that align with chart loading process
type LoadingState = 'uninitialized' | 'loading' | 'loaded' | 'error';
+/**
+ * Known shared controls that have renderTrigger: true.
+ * These are controls defined in sharedControls that only affect rendering,
+ * not data fetching. When these controls change, we should re-render
+ * without refetching data.
+ *
+ * This list is needed because string-based control references (e.g.,
['zoomable'])
+ * cannot be introspected for their renderTrigger property without importing
+ * sharedControls, which would create a circular dependency.
+ */
+const RENDER_TRIGGER_SHARED_CONTROLS = new Set([
+ 'zoomable',
+ 'color_scheme',
+ 'time_shift_color',
+ 'y_axis_format',
+ 'currency_format',
+]);
+
/**
* Helper function to determine if data should be refetched based on formData
changes
* @param prevFormData Previous formData
@@ -73,7 +91,13 @@ function shouldRefetchData(
if (section.controlSetRows) {
section.controlSetRows.forEach((row: any) => {
row.forEach((control: any) => {
- if (control && typeof control === 'object') {
+ // Handle string references to shared controls with renderTrigger
+ if (
+ typeof control === 'string' &&
+ RENDER_TRIGGER_SHARED_CONTROLS.has(control)
+ ) {
+ renderTriggerControls.add(control);
+ } else if (control && typeof control === 'object') {
const controlName = control.name || control.config?.name;
if (controlName && control.config?.renderTrigger === true) {
renderTriggerControls.add(controlName);
