Apply code review comments

apache · Apr 11, 2022 · c528621 · c528621
1 parent 73788dd
commit c528621
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 42 deletions.
diff --git a/superset-frontend/src/components/Chart/Chart.jsx b/superset-frontend/src/components/Chart/Chart.jsx
@@ -298,7 +298,7 @@ class Chart extends React.PureComponent {
         <EmptyStateBig
           title={t('Your chart is ready to go!')}
           description={t(
-            'Click on "Create chart" button in the control panel on the left to preview a visualisation',
+            'Click on "Create chart" button in the control panel on the left to preview a visualization',
           )}
           image="chart.svg"
         />

diff --git a/superset-frontend/src/explore/components/ControlPanelsContainer.tsx b/superset-frontend/src/explore/components/ControlPanelsContainer.tsx
@@ -56,12 +56,12 @@ import { getSectionsToRender } from 'src/explore/controlUtils';
 import { ExploreActions } from 'src/explore/actions/exploreActions';
 import { ExplorePageState } from 'src/explore/reducers/getInitialState';
 import { ChartState } from 'src/explore/types';
+import { Tooltip } from 'src/components/Tooltip';
 
 import ControlRow from './ControlRow';
 import Control from './Control';
 import { ControlPanelAlert } from './ControlPanelAlert';
 import { RunQueryButton } from './RunQueryButton';
-import { Tooltip } from '../../components/Tooltip';
 
 export type ControlPanelsContainerProps = {
   exploreState: ExplorePageState['explore'];
@@ -93,10 +93,13 @@ const ActionButtonsContainer = styled.div`
     flex-direction: column;
     align-items: center;
     padding: ${theme.gridUnit * 4}px;
-    background: linear-gradient(transparent, white 25%);
+    background: linear-gradient(
+      transparent,
+      ${theme.colors.grayscale.light5} ${theme.opacity.mediumLight}
+    );
 
     & > button {
-      min-width: ${theme.gridUnit * 39}px;
+      min-width: 156px;
     }
   `};
 `;

diff --git a/superset-frontend/src/explore/components/RunQueryButton/RunQueryButton.test.tsx b/superset-frontend/src/explore/components/RunQueryButton/RunQueryButton.test.tsx
@@ -33,46 +33,44 @@ const createProps = (overrides: Record<string, any> = {}) => ({
   ...overrides,
 });
 
-describe('RunQueryButton', () => {
-  it('renders update chart button', () => {
-    const props = createProps();
-    render(<RunQueryButton {...props} />);
-    expect(screen.getByText('Update chart')).toBeVisible();
-    userEvent.click(screen.getByRole('button'));
-    expect(props.onQuery).toHaveBeenCalled();
-  });
+test('renders update chart button', () => {
+  const props = createProps();
+  render(<RunQueryButton {...props} />);
+  expect(screen.getByText('Update chart')).toBeVisible();
+  userEvent.click(screen.getByRole('button'));
+  expect(props.onQuery).toHaveBeenCalled();
+});
 
-  it('renders create chart button', () => {
-    const props = createProps({ isNewChart: true });
-    render(<RunQueryButton {...props} />);
-    expect(screen.getByText('Create chart')).toBeVisible();
-    userEvent.click(screen.getByRole('button'));
-    expect(props.onQuery).toHaveBeenCalled();
-  });
+test('renders create chart button', () => {
+  const props = createProps({ isNewChart: true });
+  render(<RunQueryButton {...props} />);
+  expect(screen.getByText('Create chart')).toBeVisible();
+  userEvent.click(screen.getByRole('button'));
+  expect(props.onQuery).toHaveBeenCalled();
+});
 
-  it('renders disabled button', () => {
-    const props = createProps({ errorMessage: 'error' });
-    render(<RunQueryButton {...props} />);
-    expect(screen.getByText('Update chart')).toBeVisible();
-    expect(screen.getByRole('button')).toBeDisabled();
-    userEvent.click(screen.getByRole('button'));
-    expect(props.onQuery).not.toHaveBeenCalled();
-  });
+test('renders disabled button', () => {
+  const props = createProps({ errorMessage: 'error' });
+  render(<RunQueryButton {...props} />);
+  expect(screen.getByText('Update chart')).toBeVisible();
+  expect(screen.getByRole('button')).toBeDisabled();
+  userEvent.click(screen.getByRole('button'));
+  expect(props.onQuery).not.toHaveBeenCalled();
+});
 
-  it('renders query running button', () => {
-    const props = createProps({ loading: true });
-    render(<RunQueryButton {...props} />);
-    expect(screen.getByText('Stop')).toBeVisible();
-    userEvent.click(screen.getByRole('button'));
-    expect(props.onStop).toHaveBeenCalled();
-  });
+test('renders query running button', () => {
+  const props = createProps({ loading: true });
+  render(<RunQueryButton {...props} />);
+  expect(screen.getByText('Stop')).toBeVisible();
+  userEvent.click(screen.getByRole('button'));
+  expect(props.onStop).toHaveBeenCalled();
+});
 
-  it('renders query running button disabled', () => {
-    const props = createProps({ loading: true, canStopQuery: false });
-    render(<RunQueryButton {...props} />);
-    expect(screen.getByText('Stop')).toBeVisible();
-    expect(screen.getByRole('button')).toBeDisabled();
-    userEvent.click(screen.getByRole('button'));
-    expect(props.onStop).not.toHaveBeenCalled();
-  });
+test('renders query running button disabled', () => {
+  const props = createProps({ loading: true, canStopQuery: false });
+  render(<RunQueryButton {...props} />);
+  expect(screen.getByText('Stop')).toBeVisible();
+  expect(screen.getByRole('button')).toBeDisabled();
+  userEvent.click(screen.getByRole('button'));
+  expect(props.onStop).not.toHaveBeenCalled();
 });