fix(explore): hide a control wrapped with StashFormDataContainer corr…

…ectly (#28555)
apache · May 16, 2024 · 956511f · 956511f
1 parent 5ae6458
commit 956511f
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 41 deletions.
diff --git a/superset-frontend/src/explore/components/ControlRow.test.tsx b/superset-frontend/src/explore/components/ControlRow.test.tsx
@@ -19,49 +19,73 @@
 import React from 'react';
 import { render, screen } from 'spec/helpers/testing-library';
 import ControlSetRow from 'src/explore/components/ControlRow';
+import StashFormDataContainer from './StashFormDataContainer';
 
 const MockControl = (props: {
   children: React.ReactElement;
   type?: string;
   isVisible?: boolean;
 }) => <div>{props.children}</div>;
-describe('ControlSetRow', () => {
-  it('renders a single row with one element', () => {
-    render(<ControlSetRow controls={[<p>My Control 1</p>]} />);
-    expect(screen.getAllByText('My Control 1').length).toBe(1);
-  });
-  it('renders a single row with two elements', () => {
-    render(
-      <ControlSetRow controls={[<p>My Control 1</p>, <p>My Control 2</p>]} />,
-    );
-    expect(screen.getAllByText(/My Control/)).toHaveLength(2);
-  });
+test('renders a single row with one element', () => {
+  render(<ControlSetRow controls={[<p>My Control 1</p>]} />);
+  expect(screen.getAllByText('My Control 1').length).toBe(1);
+});
+test('renders a single row with two elements', () => {
+  render(
+    <ControlSetRow controls={[<p>My Control 1</p>, <p>My Control 2</p>]} />,
+  );
+  expect(screen.getAllByText(/My Control/)).toHaveLength(2);
+  expect(screen.getAllByText(/My Control/)[0]).toBeVisible();
+  expect(screen.getAllByText(/My Control/)[1]).toBeVisible();
+});
 
-  it('renders a single row with one elements if is HiddenControl', () => {
-    render(
-      <ControlSetRow
-        controls={[
-          <p>My Control 1</p>,
-          <MockControl type="HiddenControl">
-            <p>My Control 2</p>
-          </MockControl>,
-        ]}
-      />,
-    );
-    expect(screen.getAllByText(/My Control/)).toHaveLength(2);
-  });
+test('renders a single row with one elements if is HiddenControl', () => {
+  render(
+    <ControlSetRow
+      controls={[
+        <p>My Control 1</p>,
+        <MockControl type="HiddenControl">
+          <p>My Control 2</p>
+        </MockControl>,
+      ]}
+    />,
+  );
+  expect(screen.getAllByText(/My Control/)).toHaveLength(2);
+  expect(screen.getAllByText(/My Control/)[0]).toBeVisible();
+  expect(screen.getAllByText(/My Control/)[1]).not.toBeVisible();
+});
+
+test('renders a single row with one elements if is invisible', () => {
+  render(
+    <ControlSetRow
+      controls={[
+        <p>My Control 1</p>,
+        <MockControl isVisible={false}>
+          <p>My Control 2</p>
+        </MockControl>,
+      ]}
+    />,
+  );
+  expect(screen.getAllByText(/My Control/)).toHaveLength(2);
+  expect(screen.getAllByText(/My Control/)[0]).toBeVisible();
+  expect(screen.getAllByText(/My Control/)[1]).not.toBeVisible();
+});
 
-  it('renders a single row with one elements if is invisible', () => {
-    render(
-      <ControlSetRow
-        controls={[
-          <p>My Control 1</p>,
+test('renders a single row with one element wrapping with StashContainer if is invisible', () => {
+  render(
+    <ControlSetRow
+      controls={[
+        <p>My Control 1</p>,
+        <StashFormDataContainer shouldStash fieldNames={['field1']}>
           <MockControl isVisible={false}>
             <p>My Control 2</p>
-          </MockControl>,
-        ]}
-      />,
-    );
-    expect(screen.getAllByText(/My Control/)).toHaveLength(2);
-  });
+          </MockControl>
+        </StashFormDataContainer>,
+      ]}
+    />,
+    { useRedux: true },
+  );
+  expect(screen.getAllByText(/My Control/)).toHaveLength(2);
+  expect(screen.getAllByText(/My Control/)[0]).toBeVisible();
+  expect(screen.getAllByText(/My Control/)[1]).not.toBeVisible();
 });
diff --git a/superset-frontend/src/explore/components/ControlRow.tsx b/superset-frontend/src/explore/components/ControlRow.tsx
@@ -23,12 +23,13 @@ const NUM_COLUMNS = 12;
 type Control = React.ReactElement | null;
 
 export default function ControlRow({ controls }: { controls: Control[] }) {
-  const isHiddenControl = useCallback(
-    (control: Control) =>
-      control?.props.type === 'HiddenControl' ||
-      control?.props.isVisible === false,
-    [],
-  );
+  const isHiddenControl = useCallback((control: Control) => {
+    const props =
+      control && 'shouldStash' in control.props
+        ? control.props.children.props
+        : control?.props;
+    return props?.type === 'HiddenControl' || props?.isVisible === false;
+  }, []);
   // Invisible control should not be counted
   // in the columns number
   const countableControls = controls.filter(
@@ -41,6 +42,7 @@ export default function ControlRow({ controls }: { controls: Control[] }) {
     <div className="row">
       {controls.map((control, i) => (
         <div
+          data-test="control-item"
           className={`col-lg-${colSize} col-xs-12`}
           style={{
             display: isHiddenControl(control) ? 'none' : 'block',