refactor: Bootstrap to AntD - ListGroup (#13996)

* refactor: Bootstrap to AntD - ListGroup * Improves theme handling on touched files
2021-04-13 19:26:02 -03:00 · 2021-04-13 19:26:02 -03:00 · c8a794368d
parent b77477a9dd
commit c8a794368d
11 changed files with 158 additions and 129 deletions
--- a/superset-frontend/babel.config.js
+++ b/superset-frontend/babel.config.js
@ -63,6 +63,7 @@ module.exports = {
            targets: { node: 'current' },
          },
        ],
+        ['@emotion/babel-preset-css-prop'],
      ],
      plugins: ['babel-plugin-dynamic-import-node'],
    },
--- a/superset-frontend/cypress-base/cypress/integration/explore/annotations.test.ts
+++ b/superset-frontend/cypress-base/cypress/integration/explore/annotations.test.ts
@ -29,7 +29,7 @@ describe('Annotations', () => {

    const layerLabel = 'Goal line';

-    cy.get('[data-test=annotation_layers] button').click();
+    cy.get('[data-test=annotation_layers]').click();

    cy.get('[data-test="popover-content"]').within(() => {
      cy.get('[data-test=annotation-layer-name-header]')
--- a/superset-frontend/spec/javascripts/dashboard/components/Header_spec.jsx
+++ b/superset-frontend/spec/javascripts/dashboard/components/Header_spec.jsx
@ -17,7 +17,7 @@
 * under the License.
 */
 import React from 'react';
-import { shallow } from 'enzyme';
+import { styledMount as mount } from 'spec/helpers/theming';
 import Header from 'src/dashboard/components/Header';
 import EditableTitle from 'src/components/EditableTitle';
 import FaveStar from 'src/components/FaveStar';
@ -83,7 +83,7 @@ describe('Header', () => {
  };

  function setup(overrideProps) {
-    const wrapper = shallow(<Header {...props} {...overrideProps} />);
+    const wrapper = mount(<Header {...props} {...overrideProps} />);
    return wrapper;
  }

--- a/superset-frontend/spec/javascripts/sqllab/ShareSqlLabQuery_spec.jsx
+++ b/superset-frontend/spec/javascripts/sqllab/ShareSqlLabQuery_spec.jsx
@ -121,14 +121,12 @@ describe('ShareSqlLabQuery', () => {
          remoteId: undefined,
        },
      };
-      await act(async () => {
-        render(<ShareSqlLabQuery {...updatedProps} />, {
-          wrapper: standardProvider,
-        });
+
+      render(<ShareSqlLabQuery {...updatedProps} />, {
+        wrapper: standardProvider,
      });
-      const button = screen.getByRole('button', { name: /copy link/i });
-      const style = window.getComputedStyle(button);
-      expect(style.color).toBe('rgb(102, 102, 102)');
+      const button = await screen.findByRole('button', { name: /copy link/i });
+      expect(button).toBeDisabled();
    });
  });
 });
--- a/superset-frontend/src/SqlLab/components/ShareSqlLabQuery.tsx
+++ b/superset-frontend/src/SqlLab/components/ShareSqlLabQuery.tsx
@ -17,9 +17,7 @@
 * under the License.
 */
 import React from 'react';
-import { Tooltip } from 'src/common/components/Tooltip';
-import { t, styled, supersetTheme } from '@superset-ui/core';
-import cx from 'classnames';
+import { t, useTheme } from '@superset-ui/core';

 import Button from 'src/components/Button';
 import withToasts from 'src/messageToasts/enhancers/withToasts';
@ -41,24 +39,12 @@ interface ShareSqlLabQueryPropTypes {
  addDangerToast: (msg: string) => void;
 }

-const Styles = styled.div`
-  .btn-disabled {
-    &,
-    &:hover {
-      cursor: default;
-      background-color: ${supersetTheme.colors.grayscale.light2};
-      color: ${supersetTheme.colors.grayscale.base};
-    }
-  }
-  svg {
-    vertical-align: -${supersetTheme.gridUnit * 1.25}px;
-  }
-`;
-
 function ShareSqlLabQuery({
  queryEditor,
  addDangerToast,
 }: ShareSqlLabQueryPropTypes) {
+  const theme = useTheme();
+
  const getCopyUrlForKvStore = (callback: Function) => {
    const { dbId, title, schema, autorun, sql } = queryEditor;
    const sharedQuery = { dbId, title, schema, autorun, sql };
@ -94,58 +80,41 @@ function ShareSqlLabQuery({
    return getCopyUrlForSavedQuery(callback);
  };

-  const buildButton = () => {
-    const canShare =
-      queryEditor.remoteId ||
-      isFeatureEnabled(FeatureFlag.SHARE_QUERIES_VIA_KV_STORE);
+  const buildButton = (canShare: boolean) => {
+    const tooltip = canShare
+      ? t('Copy query link to your clipboard')
+      : t('Save the query to enable this feature');
    return (
-      <Styles>
-        <Button buttonSize="small" className={cx(!canShare && 'btn-disabled')}>
-          <Icon
-            name="link"
-            color={
-              canShare
-                ? supersetTheme.colors.primary.base
-                : supersetTheme.colors.grayscale.base
-            }
-            width={20}
-            height={20}
-          />{' '}
-          {t('Copy link')}
-        </Button>
-      </Styles>
+      <Button buttonSize="small" tooltip={tooltip} disabled={!canShare}>
+        <Icon
+          name="link"
+          color={
+            canShare ? theme.colors.primary.base : theme.colors.grayscale.base
+          }
+          width={20}
+          height={20}
+        />{' '}
+        {t('Copy link')}
+      </Button>
    );
  };

  const canShare =
-    queryEditor.remoteId ||
+    !!queryEditor.remoteId ||
    isFeatureEnabled(FeatureFlag.SHARE_QUERIES_VIA_KV_STORE);

  return (
-    <Tooltip
-      id="copy_link"
-      placement="top"
-      overlayStyle={{
-        fontSize: supersetTheme.typography.sizes.s,
-        lineHeight: '1.6',
-        maxWidth: '125px',
-      }}
-      title={
-        canShare
-          ? t('Copy query link to your clipboard')
-          : t('Save the query to enable this feature')
-      }
-    >
+    <>
      {canShare ? (
        <CopyToClipboard
          getText={getCopyUrl}
          wrapped={false}
-          copyNode={buildButton()}
+          copyNode={buildButton(canShare)}
        />
      ) : (
-        buildButton()
+        buildButton(canShare)
      )}
-    </Tooltip>
+    </>
  );
 }

--- a/superset-frontend/src/common/components/index.tsx
+++ b/superset-frontend/src/common/components/index.tsx
@ -57,6 +57,7 @@ export { RadioChangeEvent } from 'antd/lib/radio';
 export { TreeProps } from 'antd/lib/tree';
 export { default as Alert, AlertProps } from 'antd/lib/alert';
 export { default as Select, SelectProps } from 'antd/lib/select';
+export { default as List, ListItemProps } from 'antd/lib/list';

 export { default as Collapse } from './Collapse';
 export { default as Badge } from 'src/components/Badge';
--- a/superset-frontend/src/explore/components/controls/AnnotationLayerControl/index.jsx
+++ b/superset-frontend/src/explore/components/controls/AnnotationLayerControl/index.jsx
@ -18,7 +18,7 @@
 */
 import React from 'react';
 import PropTypes from 'prop-types';
-import { ListGroup, ListGroupItem } from 'react-bootstrap';
+import { List } from 'src/common/components';
 import { connect } from 'react-redux';
 import { t, withTheme } from '@superset-ui/core';
 import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls';
@ -26,6 +26,7 @@ import Popover from 'src/components/Popover';
 import AsyncEsmComponent from 'src/components/AsyncEsmComponent';
 import { getChartKey } from 'src/explore/exploreUtils';
 import { runAnnotationQuery } from 'src/chart/chartAction';
+import CustomListItem from 'src/explore/components/controls/CustomListItem';

 const AnnotationLayer = AsyncEsmComponent(
  () => import('./AnnotationLayer'),
@ -164,6 +165,12 @@ class AnnotationLayerControl extends React.PureComponent {
        trigger="click"
        placement="right"
        title={t('Edit annotation layer')}
+        css={theme => ({
+          '&:hover': {
+            cursor: 'pointer',
+            backgroundColor: theme.colors.grayscale.light4,
+          },
+        })}
        content={this.renderPopover(
          i,
          anno,
@ -172,17 +179,17 @@ class AnnotationLayerControl extends React.PureComponent {
        visible={this.state.popoverVisible[i]}
        onVisibleChange={visible => this.handleVisibleChange(visible, i)}
      >
-        <ListGroupItem>
+        <CustomListItem selectable>
          <span>{anno.name}</span>
          <span style={{ float: 'right' }}>{this.renderInfo(anno)}</span>
-        </ListGroupItem>
+        </CustomListItem>
      </Popover>
    ));

    const addLayerPopoverKey = 'add';
    return (
      <div>
-        <ListGroup>
+        <List bordered css={theme => ({ borderRadius: theme.gridUnit })}>
          {annotations}
          <Popover
            trigger="click"
@ -195,15 +202,15 @@ class AnnotationLayerControl extends React.PureComponent {
              this.handleVisibleChange(visible, addLayerPopoverKey)
            }
          >
-            <ListGroupItem>
+            <CustomListItem selectable>
              <i
                data-test="add-annotation-layer-button"
                className="fa fa-plus"
              />{' '}
              &nbsp; {t('Add annotation layer')}
-            </ListGroupItem>
+            </CustomListItem>
          </Popover>
-        </ListGroup>
+        </List>
      </div>
    );
  }
--- a/superset-frontend/src/explore/components/controls/CollectionControl/CollectionControl.less
+++ b/superset-frontend/src/explore/components/controls/CollectionControl/CollectionControl.less
@ -1,21 +0,0 @@
-/**
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- *   http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-.CollectionControl .list-group-item i.fa {
-  padding-top: 5px;
-}
--- a/superset-frontend/src/explore/components/controls/CollectionControl/CollectionControl.test.tsx
+++ b/superset-frontend/src/explore/components/controls/CollectionControl/CollectionControl.test.tsx
@ -74,7 +74,7 @@ const createProps = () => ({
  description: null,
  hovered: false,
  itemGenerator: jest.fn(),
-  keyAccessor: jest.fn(),
+  keyAccessor: jest.fn(() => 'hrYAZ5iBH'),
  label: 'Time series columns',
  name: 'column_collection',
  onChange: jest.fn(),
@ -105,7 +105,7 @@ test('Should have add button', () => {
  render(<CollectionControl {...props} />);

  expect(props.onChange).toBeCalledTimes(0);
-  userEvent.click(screen.getByRole('button', { name: 'add-item' }));
+  userEvent.click(screen.getByRole('button', { name: 'plus-large' }));
  expect(props.onChange).toBeCalledWith([{ key: 'hrYAZ5iBH' }, undefined]);
 });

@ -121,7 +121,7 @@ test('Should have remove button', () => {
 test('Should have SortableDragger icon', () => {
  const props = createProps();
  render(<CollectionControl {...props} />);
-  expect(screen.getByRole('img')).toBeVisible();
+  expect(screen.getByLabelText('drag')).toBeVisible();
 });

 test('Should call Control component', () => {
--- a/superset-frontend/src/explore/components/controls/CollectionControl/index.jsx
+++ b/superset-frontend/src/explore/components/controls/CollectionControl/index.jsx
@ -18,19 +18,24 @@
 */
 import React from 'react';
 import PropTypes from 'prop-types';
-import { ListGroup, ListGroupItem } from 'react-bootstrap';
+import { List } from 'src/common/components';
 import shortid from 'shortid';
+import { t, withTheme } from '@superset-ui/core';
 import {
  SortableContainer,
  SortableHandle,
  SortableElement,
  arrayMove,
 } from 'react-sortable-hoc';
-
+import Icon from 'src/components/Icon';
+import {
+  HeaderContainer,
+  AddIconButton,
+} from 'src/explore/components/controls/OptionControls';
 import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls';
 import ControlHeader from 'src/explore/components/ControlHeader';
+import CustomListItem from 'src/explore/components/controls/CustomListItem';
 import controlMap from '..';
-import './CollectionControl.less';

 const propTypes = {
  name: PropTypes.string.isRequired,
@ -51,23 +56,24 @@ const defaultProps = {
  label: null,
  description: null,
  onChange: () => {},
-  placeholder: 'Empty collection',
+  placeholder: t('Empty collection'),
  itemGenerator: () => ({ key: shortid.generate() }),
  keyAccessor: o => o.key,
  value: [],
-  addTooltip: 'Add an item',
+  addTooltip: t('Add an item'),
 };
-const SortableListGroupItem = SortableElement(ListGroupItem);
-const SortableListGroup = SortableContainer(ListGroup);
+const SortableListItem = SortableElement(CustomListItem);
+const SortableList = SortableContainer(List);
 const SortableDragger = SortableHandle(() => (
  <i
    role="img"
+    aria-label="drag"
    className="fa fa-bars text-primary"
    style={{ cursor: 'ns-resize' }}
  />
 ));

-export default class CollectionControl extends React.Component {
+class CollectionControl extends React.Component {
  constructor(props) {
    super(props);
    this.onAdd = this.onAdd.bind(this);
@ -96,59 +102,69 @@ export default class CollectionControl extends React.Component {
    }
    const Control = controlMap[this.props.controlName];
    return (
-      <SortableListGroup
+      <SortableList
        useDragHandle
        lockAxis="y"
        onSortEnd={this.onSortEnd.bind(this)}
+        bordered
+        css={theme => ({
+          borderRadius: theme.gridUnit,
+        })}
      >
        {this.props.value.map((o, i) => {
          // label relevant only for header, not here
          const { label, ...commonProps } = this.props;
          return (
-            <SortableListGroupItem
+            <SortableListItem
              className="clearfix"
+              css={{ justifyContent: 'flex-start' }}
              key={this.props.keyAccessor(o)}
              index={i}
            >
-              <div className="pull-left m-r-5">
-                <SortableDragger />
-              </div>
-              <div className="pull-left">
+              <SortableDragger />
+              <div
+                css={theme => ({
+                  flex: 1,
+                  marginLeft: theme.gridUnit * 2,
+                  marginRight: theme.gridUnit * 2,
+                })}
+              >
                <Control
                  {...commonProps}
                  {...o}
                  onChange={this.onChange.bind(this, i)}
                />
              </div>
-              <div className="pull-right">
-                <InfoTooltipWithTrigger
-                  icon="times"
-                  label="remove-item"
-                  tooltip="remove item"
-                  bsStyle="primary"
-                  onClick={this.removeItem.bind(this, i)}
-                />
-              </div>
-            </SortableListGroupItem>
+              <InfoTooltipWithTrigger
+                icon="times"
+                label="remove-item"
+                tooltip={t('Remove item')}
+                bsStyle="primary"
+                onClick={this.removeItem.bind(this, i)}
+              />
+            </SortableListItem>
          );
        })}
-      </SortableListGroup>
+      </SortableList>
    );
  }

  render() {
+    const { theme } = this.props;
    return (
      <div data-test="CollectionControl" className="CollectionControl">
-        <ControlHeader {...this.props} />
+        <HeaderContainer>
+          <ControlHeader {...this.props} />
+          <AddIconButton onClick={this.onAdd}>
+            <Icon
+              name="plus-large"
+              width={theme.gridUnit * 3}
+              height={theme.gridUnit * 3}
+              color={theme.colors.grayscale.light5}
+            />
+          </AddIconButton>
+        </HeaderContainer>
        {this.renderList()}
-        <InfoTooltipWithTrigger
-          icon="plus-circle"
-          label="add-item"
-          tooltip={this.props.addTooltip}
-          bsStyle="primary"
-          className="fa-lg"
-          onClick={this.onAdd}
-        />
      </div>
    );
  }
@ -156,3 +172,5 @@ export default class CollectionControl extends React.Component {

 CollectionControl.propTypes = propTypes;
 CollectionControl.defaultProps = defaultProps;
+
+export default withTheme(CollectionControl);
--- a/superset-frontend/src/explore/components/controls/CustomListItem/index.tsx
+++ b/superset-frontend/src/explore/components/controls/CustomListItem/index.tsx
@ -0,0 +1,56 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React from 'react';
+import { useTheme } from '@superset-ui/core';
+import { List, ListItemProps } from 'src/common/components';
+
+export interface CustomListItemProps extends ListItemProps {
+  selectable: boolean;
+}
+
+export default function CustomListItem(props: CustomListItemProps) {
+  const { selectable, children, ...rest } = props;
+  const theme = useTheme();
+  const css = {
+    '&.ant-list-item': {
+      padding: `${theme.gridUnit + 2}px ${theme.gridUnit * 3}px`,
+      ':first-of-type': {
+        borderTopLeftRadius: theme.gridUnit,
+        borderTopRightRadius: theme.gridUnit,
+      },
+      ':last-of-type': {
+        borderBottomLeftRadius: theme.gridUnit,
+        borderBottomRightRadius: theme.gridUnit,
+      },
+    },
+  };
+
+  if (selectable) {
+    css['&:hover'] = {
+      cursor: 'pointer',
+      backgroundColor: theme.colors.grayscale.light4,
+    };
+  }
+
+  return (
+    <List.Item {...rest} css={css}>
+      {children}
+    </List.Item>
+  );
+}