You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2021/06/25 18:16:05 UTC

[superset] branch master updated: chore: Rewrites dashboard IconButton component (#14174)

This is an automated email from the ASF dual-hosted git repository.

michaelsmolina 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 bdb9b0a  chore: Rewrites dashboard IconButton component (#14174)
bdb9b0a is described below

commit bdb9b0a9be749a2a9222cfedcb259c1557b6b71c
Author: Michael S. Molina <70...@users.noreply.github.com>
AuthorDate: Fri Jun 25 15:15:20 2021 -0300

    chore: Rewrites dashboard IconButton component (#14174)
---
 .../components/gridComponents/Divider_spec.jsx     |  2 +-
 .../DashboardBuilder/DashboardBuilder.tsx          |  3 +-
 .../dashboard/components/DashboardBuilder/utils.ts |  3 +-
 .../dashboard/components/DeleteComponentButton.jsx |  6 ++-
 .../src/dashboard/components/IconButton.jsx        | 63 ----------------------
 .../buttons.less => components/IconButton.tsx}     | 52 +++++++++++-------
 .../dashboard/components/PropertiesModal/index.jsx |  1 -
 .../dashboard/components/gridComponents/Column.jsx | 30 +++++------
 .../dashboard/components/gridComponents/Row.jsx    | 26 ++++-----
 .../src/dashboard/stylesheets/index.less           |  1 -
 10 files changed, 67 insertions(+), 120 deletions(-)

diff --git a/superset-frontend/spec/javascripts/dashboard/components/gridComponents/Divider_spec.jsx b/superset-frontend/spec/javascripts/dashboard/components/gridComponents/Divider_spec.jsx
index 3ab7952..6331f68 100644
--- a/superset-frontend/spec/javascripts/dashboard/components/gridComponents/Divider_spec.jsx
+++ b/superset-frontend/spec/javascripts/dashboard/components/gridComponents/Divider_spec.jsx
@@ -17,7 +17,7 @@
  * under the License.
  */
 import React from 'react';
-import { mount } from 'enzyme';
+import { styledMount as mount } from 'spec/helpers/theming';
 import sinon from 'sinon';
 import { DndProvider } from 'react-dnd';
 import { HTML5Backend } from 'react-dnd-html5-backend';
diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx
index e122238..d8a0a34 100644
--- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx
+++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx
@@ -23,6 +23,7 @@ import { JsonObject, styled } from '@superset-ui/core';
 import ErrorBoundary from 'src/components/ErrorBoundary';
 import BuilderComponentPane from 'src/dashboard/components/BuilderComponentPane';
 import DashboardHeader from 'src/dashboard/containers/DashboardHeader';
+import Icons from 'src/components/Icons';
 import IconButton from 'src/dashboard/components/IconButton';
 import DragDroppable from 'src/dashboard/components/dnd/DragDroppable';
 import DashboardComponent from 'src/dashboard/containers/DashboardComponent';
@@ -251,7 +252,7 @@ const DashboardBuilder: FC<DashboardBuilderProps> = () => {
                   shouldFocus={shouldFocusTabs}
                   menuItems={[
                     <IconButton
-                      className="fa fa-level-down"
+                      icon={<Icons.FallOutlined iconSize="xl" />}
                       label="Collapse tab content"
                       onClick={handleDeleteTopLevelTabs}
                     />,
diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/utils.ts b/superset-frontend/src/dashboard/components/DashboardBuilder/utils.ts
index 999adf6..f2aa381 100644
--- a/superset-frontend/src/dashboard/components/DashboardBuilder/utils.ts
+++ b/superset-frontend/src/dashboard/components/DashboardBuilder/utils.ts
@@ -37,8 +37,7 @@ export const shouldFocusTabs = (
 ) =>
   // don't focus the tabs when we click on a tab
   event.target.className === 'ant-tabs-nav-wrap' ||
-  (/icon-button/.test(event.target.className) &&
-    container.contains(event.target));
+  container.contains(event.target);
 
 export const getRootLevelTabIndex = (
   dashboardLayout: DashboardLayout,
diff --git a/superset-frontend/src/dashboard/components/DeleteComponentButton.jsx b/superset-frontend/src/dashboard/components/DeleteComponentButton.jsx
index 4e3e10d..d3936f3 100644
--- a/superset-frontend/src/dashboard/components/DeleteComponentButton.jsx
+++ b/superset-frontend/src/dashboard/components/DeleteComponentButton.jsx
@@ -18,7 +18,7 @@
  */
 import React from 'react';
 import PropTypes from 'prop-types';
-
+import Icons from 'src/components/Icons';
 import IconButton from './IconButton';
 
 const propTypes = {
@@ -30,7 +30,9 @@ const defaultProps = {};
 export default class DeleteComponentButton extends React.PureComponent {
   render() {
     const { onDelete } = this.props;
-    return <IconButton onClick={onDelete} className="fa fa-trash" />;
+    return (
+      <IconButton onClick={onDelete} icon={<Icons.Trash iconSize="xl" />} />
+    );
   }
 }
 
diff --git a/superset-frontend/src/dashboard/components/IconButton.jsx b/superset-frontend/src/dashboard/components/IconButton.jsx
deleted file mode 100644
index 1db166d..0000000
--- a/superset-frontend/src/dashboard/components/IconButton.jsx
+++ /dev/null
@@ -1,63 +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.
- */
-import React from 'react';
-import PropTypes from 'prop-types';
-
-const propTypes = {
-  onClick: PropTypes.func.isRequired,
-  className: PropTypes.string,
-  label: PropTypes.string,
-};
-
-const defaultProps = {
-  className: null,
-  label: null,
-};
-
-export default class IconButton extends React.PureComponent {
-  constructor(props) {
-    super(props);
-    this.handleClick = this.handleClick.bind(this);
-  }
-
-  handleClick(event) {
-    event.preventDefault();
-    const { onClick } = this.props;
-    onClick(event);
-  }
-
-  render() {
-    const { className, label } = this.props;
-    return (
-      <div
-        className="icon-button"
-        onClick={this.handleClick}
-        tabIndex="0"
-        role="button"
-        data-test="icon-button"
-      >
-        <span data-test="icon-button-span" className={className} />
-        {label && <span className="icon-button-label">{label}</span>}
-      </div>
-    );
-  }
-}
-
-IconButton.propTypes = propTypes;
-IconButton.defaultProps = defaultProps;
diff --git a/superset-frontend/src/dashboard/stylesheets/buttons.less b/superset-frontend/src/dashboard/components/IconButton.tsx
similarity index 53%
rename from superset-frontend/src/dashboard/stylesheets/buttons.less
rename to superset-frontend/src/dashboard/components/IconButton.tsx
index 9cec645..1a15456 100644
--- a/superset-frontend/src/dashboard/stylesheets/buttons.less
+++ b/superset-frontend/src/dashboard/components/IconButton.tsx
@@ -16,28 +16,40 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-@import '../../../stylesheets/less/variables.less';
+import React, { MouseEventHandler } from 'react';
+import { styled } from '@superset-ui/core';
 
-.icon-button {
-  color: @gray;
-  font-size: @font-size-l;
+interface IconButtonProps {
+  icon: JSX.Element;
+  label?: string;
+  onClick: MouseEventHandler<HTMLDivElement>;
+}
+
+const StyledDiv = styled.div`
   display: flex;
-  flex-direction: row;
   align-items: center;
-  justify-content: center;
-  outline: none;
-
-  &:hover,
-  &:active,
-  &:focus {
-    color: @almost-black;
-    outline: none;
-    text-decoration: none;
+  color: ${({ theme }) => theme.colors.grayscale.base};
+  &:hover {
+    color: ${({ theme }) => theme.colors.primary.base};
   }
-}
+`;
 
-.icon-button-label {
-  color: @gray-dark;
-  padding-left: 8px;
-  font-size: @font-size-m;
-}
+const StyledSpan = styled.span`
+  margin-left: ${({ theme }) => theme.gridUnit * 2}px;
+`;
+
+const IconButton = ({ icon, label, onClick }: IconButtonProps) => (
+  <StyledDiv
+    tabIndex={0}
+    role="button"
+    onClick={e => {
+      e.preventDefault();
+      onClick(e);
+    }}
+  >
+    {icon}
+    {label && <StyledSpan>{label}</StyledSpan>}
+  </StyledDiv>
+);
+
+export default IconButton;
diff --git a/superset-frontend/src/dashboard/components/PropertiesModal/index.jsx b/superset-frontend/src/dashboard/components/PropertiesModal/index.jsx
index cdc1b95..c149479 100644
--- a/superset-frontend/src/dashboard/components/PropertiesModal/index.jsx
+++ b/superset-frontend/src/dashboard/components/PropertiesModal/index.jsx
@@ -38,7 +38,6 @@ import { JsonEditor } from 'src/components/AsyncAceEditor';
 import ColorSchemeControlWrapper from 'src/dashboard/components/ColorSchemeControlWrapper';
 import { getClientErrorObject } from 'src/utils/getClientErrorObject';
 import withToasts from 'src/messageToasts/enhancers/withToasts';
-import 'src/dashboard/stylesheets/buttons.less';
 import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags';
 
 const StyledJsonEditor = styled(JsonEditor)`
diff --git a/superset-frontend/src/dashboard/components/gridComponents/Column.jsx b/superset-frontend/src/dashboard/components/gridComponents/Column.jsx
index 78d272b..d636349 100644
--- a/superset-frontend/src/dashboard/components/gridComponents/Column.jsx
+++ b/superset-frontend/src/dashboard/components/gridComponents/Column.jsx
@@ -19,21 +19,19 @@
 import React from 'react';
 import PropTypes from 'prop-types';
 import cx from 'classnames';
-
-import DashboardComponent from '../../containers/DashboardComponent';
-import DeleteComponentButton from '../DeleteComponentButton';
-import DragDroppable from '../dnd/DragDroppable';
-import DragHandle from '../dnd/DragHandle';
-import HoverMenu from '../menu/HoverMenu';
-import IconButton from '../IconButton';
-import ResizableContainer from '../resizable/ResizableContainer';
-import BackgroundStyleDropdown from '../menu/BackgroundStyleDropdown';
-import WithPopoverMenu from '../menu/WithPopoverMenu';
-
-import backgroundStyleOptions from '../../util/backgroundStyleOptions';
-import { componentShape } from '../../util/propShapes';
-
-import { BACKGROUND_TRANSPARENT } from '../../util/constants';
+import Icons from 'src/components/Icons';
+import DashboardComponent from 'src/dashboard/containers/DashboardComponent';
+import DeleteComponentButton from 'src/dashboard/components/DeleteComponentButton';
+import DragDroppable from 'src/dashboard/components/dnd/DragDroppable';
+import DragHandle from 'src/dashboard/components/dnd/DragHandle';
+import HoverMenu from 'src/dashboard/components/menu/HoverMenu';
+import IconButton from 'src/dashboard/components/IconButton';
+import ResizableContainer from 'src/dashboard/components/resizable/ResizableContainer';
+import BackgroundStyleDropdown from 'src/dashboard/components/menu/BackgroundStyleDropdown';
+import WithPopoverMenu from 'src/dashboard/components/menu/WithPopoverMenu';
+import backgroundStyleOptions from 'src/dashboard/util/backgroundStyleOptions';
+import { componentShape } from 'src/dashboard/util/propShapes';
+import { BACKGROUND_TRANSPARENT } from 'src/dashboard/util/constants';
 
 const propTypes = {
   id: PropTypes.string.isRequired,
@@ -169,7 +167,7 @@ class Column extends React.PureComponent {
                   />
                   <IconButton
                     onClick={this.handleChangeFocus}
-                    className="fa fa-cog"
+                    icon={<Icons.Cog iconSize="xl" />}
                   />
                 </HoverMenu>
               )}
diff --git a/superset-frontend/src/dashboard/components/gridComponents/Row.jsx b/superset-frontend/src/dashboard/components/gridComponents/Row.jsx
index 3614d9a..a6b3598 100644
--- a/superset-frontend/src/dashboard/components/gridComponents/Row.jsx
+++ b/superset-frontend/src/dashboard/components/gridComponents/Row.jsx
@@ -20,18 +20,18 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import cx from 'classnames';
 
-import DragDroppable from '../dnd/DragDroppable';
-import DragHandle from '../dnd/DragHandle';
-import DashboardComponent from '../../containers/DashboardComponent';
-import DeleteComponentButton from '../DeleteComponentButton';
-import HoverMenu from '../menu/HoverMenu';
-import IconButton from '../IconButton';
-import BackgroundStyleDropdown from '../menu/BackgroundStyleDropdown';
-import WithPopoverMenu from '../menu/WithPopoverMenu';
-
-import { componentShape } from '../../util/propShapes';
-import backgroundStyleOptions from '../../util/backgroundStyleOptions';
-import { BACKGROUND_TRANSPARENT } from '../../util/constants';
+import DragDroppable from 'src/dashboard/components/dnd/DragDroppable';
+import DragHandle from 'src/dashboard/components/dnd/DragHandle';
+import DashboardComponent from 'src/dashboard/containers/DashboardComponent';
+import DeleteComponentButton from 'src/dashboard/components/DeleteComponentButton';
+import HoverMenu from 'src/dashboard/components/menu/HoverMenu';
+import Icons from 'src/components/Icons';
+import IconButton from 'src/dashboard/components/IconButton';
+import BackgroundStyleDropdown from 'src/dashboard/components/menu/BackgroundStyleDropdown';
+import WithPopoverMenu from 'src/dashboard/components/menu/WithPopoverMenu';
+import { componentShape } from 'src/dashboard/util/propShapes';
+import backgroundStyleOptions from 'src/dashboard/util/backgroundStyleOptions';
+import { BACKGROUND_TRANSPARENT } from 'src/dashboard/util/constants';
 
 const propTypes = {
   id: PropTypes.string.isRequired,
@@ -149,7 +149,7 @@ class Row extends React.PureComponent {
                 <DeleteComponentButton onDelete={this.handleDeleteComponent} />
                 <IconButton
                   onClick={this.handleChangeFocus}
-                  className="fa fa-cog"
+                  icon={<Icons.Cog iconSize="xl" />}
                 />
               </HoverMenu>
             )}
diff --git a/superset-frontend/src/dashboard/stylesheets/index.less b/superset-frontend/src/dashboard/stylesheets/index.less
index 2f882eb..6dee17e 100644
--- a/superset-frontend/src/dashboard/stylesheets/index.less
+++ b/superset-frontend/src/dashboard/stylesheets/index.less
@@ -20,7 +20,6 @@
 
 @import './builder.less';
 @import './builder-sidepane.less';
-@import './buttons.less';
 @import './dashboard.less';
 @import './dnd.less';
 @import './filter-scope-selector.less';