style: Pass at propagating (and enhancing) Button component throughout Superset (#10649)

* getting rid of weird focus/active outline ring

* Buttons... buttons _everywhere_

* linting

* Nixing views/CRUD/dataset/Button component

* fixing 2 typing errors

* fixing more TS errors

* prefer src path for include

* one more real button, one less CSS class

* one more "button" to "Button"

* Published Status is now a proper clickable Label

* nixing the CRUD button again

* touching up stories, with SupersetButton story

* SIP-34 button colors

* adding polished package to mix colors

* updating button colors to match Superset theme

* abstracting away from bootstrap-specific props (might pivot libraries soon!)

* more abstraction from bsStyle/bsSize props

* exchanging styles for a prop

* linting

* restoring feature flag to stock

* using src alias

* last <button> replacement

* this classname would never be applied

* more linting action

* fixing unsupported bsSize 'medium', and cta typing error

* more cta action

* unnecessary styles

* errant bsSize prop

* cleanup

* tweaks to make new New button work

* Linting

* fixing a couple tests

* fixing theme based test failure

* margin tweak for NEW button

* another fixed test

* another fixed test

* fixing two more tests

* fixing last broken tests.

* always be linting

* Adding tertiary/dashed buttons

* cleaning up QueryAndSave buttons

* fixing "link" button styles

* fixing/updating link button styles

* cta buttons on Modal component

* linting.

* exporting button story knobs, making ALL knobs safe for export.

* capitalizing a file... no big whoop

* Basic button tests

* renaming button - temporarily

* renaming file to fix capitalization issue

* passing theme through to a difficult popover.

* fixin' a newly busted unit test

* lint fixin'

* oops, shouldn't have changed this prop!

* adding a dive() to themedShallow, and fixing a cypress/jest test

* addressing lint stuff

* touching up stories, with SupersetButton story

* SIP-34 button colors

* updating button colors to match Superset theme

* abstracting away from bootstrap-specific props (might pivot libraries soon!)

* linting

* restoring feature flag to stock

* cleanup

* Linting

* renaming button - temporarily

* renaming file to fix capitalization issue

* oops, shouldn't have changed this prop!

* adding a dive() to themedShallow, and fixing a cypress/jest test

* addressing lint stuff

* nixing new modal button

* Fixing another popover/button issue that should break cypress

* lint ✨

* passing classNames through to new button (should fix some tests)

* cleaning unused classes, making cypress tests use data attrs

* fixin' the test

* fixing another class-based test with data-test attr

* no longer passing theme as prop to buttons in popovers... themeprovider is better

* outline/border tweaks!

superset-frontend/src/components/Button/Button.stories.jsx

@@ -25,13 +25,16 @@ export default {
   title: 'Button',
   component: Button,
   decorators: [withKnobs],
+  excludeStories: /.*Knob$/,
 };
 
-const bsStyleKnob = {
+export const buttonStyleKnob = {
   label: 'Types',
   options: {
     Primary: 'primary',
     Secondary: 'secondary',
+    Tertiary: 'tertiary',
+    Dashed: 'dashed',
     Danger: 'danger',
     Warning: 'warning',
     Success: 'success',
@@ -42,17 +45,18 @@ const bsStyleKnob = {
   defaultValue: null,
   // groupId: 'ButtonType',
 };
-const bsSizeKnob = {
+
+export const buttonSizeKnob = {
   label: 'Sizes',
   options: {
     XS: 'xsmall',
     S: 'small',
-    M: 'medium',
+    Default: null,
     L: 'large',
-    None: null,
   },
   defaultValue: null,
 };
+
 // TODO remove the use of these in the codebase where they're not necessary
 // const classKnob = {
 //   label: 'Known Classes',
@@ -62,7 +66,6 @@ const bsSizeKnob = {
 //     Reset: 'reset',
 //     Fetch: 'fetch',
 //     Query: 'query',
-//     saveBtn: 'save-btn',
 //     MR3: 'm-r-3',
 //     cancelQuery: 'cancelQuery',
 //     toggleSave: 'toggleSave',
@@ -101,43 +104,43 @@ const hrefKnob = {
 
 export const ButtonGallery = () => (
   <>
-    {Object.values(bsSizeKnob.options)
-      .filter(a => a)
-      .map(size => (
-        <div>
-          <h4>{size}</h4>
-          {Object.values(bsStyleKnob.options)
-            .filter(o => o)
-            .map(style => (
-              <Button
-                disabled={boolean('Disabled', false)}
-                bsStyle={style}
-                bsSize={size}
-                onClick={action('clicked')}
-                style={{ marginRight: 5 }}
-              >
-                {style}
-              </Button>
-            ))}
-        </div>
-      ))}
+    {Object.entries(buttonSizeKnob.options).map(([name, size]) => (
+      <div key={size}>
+        <h4>{name}</h4>
+        {Object.values(buttonStyleKnob.options)
+          .filter(o => o)
+          .map(style => (
+            <Button
+              disabled={boolean('Disabled', false)}
+              cta={boolean('CTA', false)}
+              buttonStyle={style}
+              buttonSize={size}
+              onClick={action('clicked')}
+              key={`${style}_${size}`}
+            >
+              {style}
+            </Button>
+          ))}
+      </div>
+    ))}
   </>
 );
 
 export const InteractiveButton = () => (
   <Button
     disabled={boolean('Disabled', false)}
-    bsStyle={select(
-      bsStyleKnob.label,
-      bsStyleKnob.options,
-      bsStyleKnob.defaultValue,
-      bsStyleKnob.groupId,
+    cta={boolean('CTA', false)}
+    buttonStyle={select(
+      buttonStyleKnob.label,
+      buttonStyleKnob.options,
+      buttonStyleKnob.defaultValue,
+      buttonStyleKnob.groupId,
     )}
-    bsSize={select(
-      bsSizeKnob.label,
-      bsSizeKnob.options,
-      bsSizeKnob.defaultValue,
-      bsSizeKnob.groupId,
+    size={select(
+      buttonSizeKnob.label,
+      buttonSizeKnob.options,
+      buttonSizeKnob.defaultValue,
+      buttonSizeKnob.groupId,
     )}
     onClick={action('clicked')}
     type={select(

superset-frontend/src/components/Button/Button.test.tsx

@@ -0,0 +1,68 @@
+/**
+ * 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 { ReactWrapper } from 'enzyme';
+import { styledMount as mount } from 'spec/helpers/theming';
+import Button from '.';
+import {
+  ButtonGallery,
+  buttonSizeKnob,
+  buttonStyleKnob,
+} from './Button.stories';
+
+describe('Button', () => {
+  let wrapper: ReactWrapper;
+
+  // test the basic component
+  it('renders the base component', () => {
+    expect(React.isValidElement(<Button />)).toBe(true);
+  });
+
+  it('works with an onClick handler', () => {
+    const mockAction = jest.fn();
+    wrapper = mount(<Button onClick={mockAction} />);
+    wrapper.find('Button').first().simulate('click');
+    expect(mockAction).toHaveBeenCalled();
+  });
+
+  it('does not handle onClicks when disabled', () => {
+    const mockAction = jest.fn();
+    wrapper = mount(<Button onClick={mockAction} disabled />);
+    wrapper.find('Button').first().simulate('click');
+    expect(mockAction).toHaveBeenCalledTimes(0);
+  });
+
+  // test stories from the storybook!
+  it('All the sorybook gallery variants mount', () => {
+    wrapper = mount(<ButtonGallery />);
+
+    const permutationCount =
+      Object.values(buttonStyleKnob.options).filter(o => o).length *
+      Object.values(buttonSizeKnob.options).length;
+
+    expect(wrapper.find(Button).length).toEqual(permutationCount);
+  });
+
+  // test things NOT in the storybook!
+  it('renders custom button styles without melting', () => {
+    wrapper = mount(<Button buttonStyle="foobar" />);
+    expect(wrapper.find('Button.btn-foobar')).toHaveLength(1);
+  });
+});

superset-frontend/src/components/Button/index.tsx

@@ -18,6 +18,8 @@
  */
 import React from 'react';
 import { kebabCase } from 'lodash';
+import { mix } from 'polished';
+import cx from 'classnames';
 import {
   Button as BootstrapButton,
   Tooltip,
@@ -40,52 +42,201 @@ export interface ButtonProps {
   placement?: string;
   onClick?: OnClickHandler;
   disabled?: boolean;
-  bsStyle?: string;
+  buttonStyle?: string;
   btnStyles?: string;
-  bsSize?: BootstrapButton.ButtonProps['bsSize'];
+  buttonSize?: BootstrapButton.ButtonProps['bsSize'];
   style?: BootstrapButton.ButtonProps['style'];
   children?: React.ReactNode;
   dropdownItems?: DropdownItemProps[];
+  href?: string; // React-Bootstrap creates a link when this is passed in.
+  target?: string; // React-Bootstrap creates a link when this is passed in.
+  type?: string; // React-Bootstrap supports this when rendering an HTML button element
+  cta?: boolean;
 }
 
 const BUTTON_WRAPPER_STYLE = { display: 'inline-block', cursor: 'not-allowed' };
 
 const SupersetButton = styled(BootstrapButton)`
-  &.supersetButton {
-    border-radius: ${({ theme }) => theme.borderRadius}px;
-    border: none;
-    color: ${({ theme }) => theme.colors.secondary.light5};
-    font-size: ${({ theme }) => theme.typography.sizes.s}px;
-    font-weight: ${({ theme }) => theme.typography.weights.bold};
+  &:focus,
+  &:active,
+  &:focus:active {
+    outline: none;
+    box-shadow: none;
+  }
+  transition: all ${({ theme }) => theme.transitionTiming}s;
+  border-radius: ${({ theme }) => theme.borderRadius}px;
+  border: none;
+  font-size: ${({ theme }) => theme.typography.sizes.s}px;
+  font-weight: ${({ theme }) => theme.typography.weights.bold};
+  margin-left: ${({ theme }) => theme.gridUnit * 4}px;
+  &:first-of-type {
+    margin-left: 0;
+  }
+
+  i {
+    padding: 0 ${({ theme }) => theme.gridUnit * 2}px 0 0;
+  }
+
+  /* SIP 34 colors! */
+  &.btn {
+    border: 1px solid transparent; /* this just makes sure the height is the same as tertiary/dashed buttons */
+    &:hover,
+    &:active {
+      border: 1px solid transparent;
+    }
+    &-default,
+    &-secondary {
+      background-color: ${({ theme }) => theme.colors.primary.light4};
+      color: ${({ theme }) => theme.colors.primary.dark1};
+      &:hover {
+        background-color: ${({ theme }) =>
+          mix(0.1, theme.colors.grayscale.light5, theme.colors.primary.light4)};
+        color: ${({ theme }) => theme.colors.primary.dark1};
+      }
+      &:active {
+        background-color: ${({ theme }) =>
+          mix(0.25, theme.colors.primary.base, theme.colors.primary.light4)};
+        color: ${({ theme }) => theme.colors.primary.dark1};
+      }
+    }
+    &-tertiary,
+    &-dashed {
+      border-width: 1px;
+      border-style: solid;
+      background-color: ${({ theme }) => theme.colors.grayscale.light5};
+      color: ${({ theme }) => theme.colors.primary.dark1};
+      border-color: ${({ theme }) => theme.colors.primary.dark1};
+      &:hover {
+        background-color: ${({ theme }) => theme.colors.grayscale.light5};
+        color: ${({ theme }) => theme.colors.primary.dark1};
+        border-color: ${({ theme }) => theme.colors.primary.light1};
+      }
+      &:active {
+        background-color: ${({ theme }) => theme.colors.grayscale.light5};
+        color: ${({ theme }) => theme.colors.primary.dark1};
+        border-color: ${({ theme }) => theme.colors.primary.dark1};
+      }
+      &[disabled],
+      &[disabled]:hover {
+        background-color: ${({ theme }) => theme.colors.grayscale.light5};
+        color: ${({ theme }) => theme.colors.grayscale.base};
+        border-color: ${({ theme }) => theme.colors.grayscale.light2};
+      }
+    }
+    &-dashed {
+      border-style: dashed;
+      &:hover,
+      &:active {
+        border-style: dashed;
+      }
+    }
+    &-link {
+      background: none;
+      text-decoration: none;
+      color: ${({ theme }) => theme.colors.primary.dark1};
+      &:hover {
+        background: none;
+        color: ${({ theme }) => theme.colors.primary.base};
+      }
+      &:active {
+        background: none;
+        color: ${({ theme }) => theme.colors.primary.dark1};
+      }
+      &[disabled],
+      &[disabled]:hover {
+        background: none;
+        color: ${({ theme }) => theme.colors.grayscale.base};
+      }
+    }
+    &-primary {
+      background-color: ${({ theme }) => theme.colors.primary.dark1};
+      color: ${({ theme }) => theme.colors.grayscale.light5};
+      &:hover {
+        background-color: ${({ theme }) =>
+          mix(0.1, theme.colors.grayscale.light5, theme.colors.primary.dark1)};
+        color: ${({ theme }) => theme.colors.grayscale.light5};
+      }
+      &:active {
+        background-color: ${({ theme }) =>
+          mix(0.2, theme.colors.grayscale.dark2, theme.colors.primary.dark1)};
+        color: ${({ theme }) => theme.colors.grayscale.light5};
+      }
+    }
+    &-danger {
+      background-color: ${({ theme }) => theme.colors.error.base};
+      color: ${({ theme }) => theme.colors.grayscale.light5};
+      &:hover {
+        background-color: ${({ theme }) =>
+          mix(0.1, theme.colors.grayscale.light5, theme.colors.error.base)};
+        color: ${({ theme }) => theme.colors.grayscale.light5};
+      }
+      &:active {
+        background-color: ${({ theme }) =>
+          mix(0.2, theme.colors.grayscale.dark2, theme.colors.error.base)};
+        color: ${({ theme }) => theme.colors.grayscale.light5};
+      }
+    }
+    &-success {
+      background-color: ${({ theme }) => theme.colors.success.base};
+      color: ${({ theme }) => theme.colors.grayscale.light5};
+      &:hover {
+        background-color: ${({ theme }) =>
+          mix(0.1, theme.colors.grayscale.light5, theme.colors.success.base)};
+        color: ${({ theme }) => theme.colors.grayscale.light5};
+      }
+      &:active {
+        background-color: ${({ theme }) =>
+          mix(0.2, theme.colors.grayscale.dark2, theme.colors.success.base)};
+        color: ${({ theme }) => theme.colors.grayscale.light5};
+      }
+    }
+    &-warning {
+      background-color: ${({ theme }) => theme.colors.warning.base};
+      color: ${({ theme }) => theme.colors.grayscale.light5};
+      &:hover {
+        background-color: ${({ theme }) =>
+          mix(0.1, theme.colors.grayscale.light5, theme.colors.warning.base)};
+        color: ${({ theme }) => theme.colors.grayscale.light5};
+      }
+      &:active {
+        background-color: ${({ theme }) =>
+          mix(0.2, theme.colors.grayscale.dark2, theme.colors.warning.base)};
+        color: ${({ theme }) => theme.colors.grayscale.light5};
+      }
+    }
+    &-info {
+      background-color: ${({ theme }) => theme.colors.info.dark1};
+      color: ${({ theme }) => theme.colors.grayscale.light5};
+      &:hover {
+        background-color: ${({ theme }) =>
+          mix(0.1, theme.colors.grayscale.light5, theme.colors.info.dark1)};
+        color: ${({ theme }) => theme.colors.grayscale.light5};
+      }
+      &:active {
+        background-color: ${({ theme }) =>
+          mix(0.2, theme.colors.grayscale.dark2, theme.colors.info.dark1)};
+        color: ${({ theme }) => theme.colors.grayscale.light5};
+      }
+    }
+    &[disabled],
+    &[disabled]:hover {
+      background-color: ${({ theme }) => theme.colors.grayscale.light2};
+      color: ${({ theme }) => theme.colors.grayscale.light1};
+    }
+  }
+
+  /* big Call to Action buttons */
+  &.cta {
     min-width: ${({ theme }) => theme.gridUnit * 36}px;
     min-height: ${({ theme }) => theme.gridUnit * 8}px;
     text-transform: uppercase;
-    margin-left: ${({ theme }) => theme.gridUnit * 4}px;
-    &:first-of-type {
-      margin-left: 0;
-    }
-
-    i {
-      padding: 0 ${({ theme }) => theme.gridUnit * 2}px 0 0;
-    }
-
-    &.primary {
-      background-color: ${({ theme }) => theme.colors.primary.base};
-    }
-    &.secondary {
-      color: ${({ theme }) => theme.colors.primary.base};
-      background-color: ${({ theme }) => theme.colors.primary.light4};
-    }
-    &.danger {
-      background-color: ${({ theme }) => theme.colors.error.base};
-    }
   }
 `;
 
 export default function Button(props: ButtonProps) {
   const buttonProps = {
     ...props,
-    bsSize: props.bsSize || 'sm',
+    bsSize: props.buttonSize,
     placement: props.placement || 'top',
   };
   const tooltip = props.tooltip;
@@ -100,17 +251,40 @@ export default function Button(props: ButtonProps) {
     buttonProps.style = { pointerEvents: 'none' };
   }
 
-  let button = (
-    <SupersetButton {...buttonProps}>{props.children}</SupersetButton>
-  );
+  const officialBootstrapStyles = [
+    'success',
+    'warning',
+    'danger',
+    'info',
+    'default',
+    'primary',
+  ];
 
-  const whittledProps = { ...buttonProps };
-  delete whittledProps.dropdownItems;
+  const transformedProps = {
+    ...buttonProps,
+    bsStyle: officialBootstrapStyles.includes(props.buttonStyle || '')
+      ? props.buttonStyle
+      : 'default',
+    className: cx(props.className, {
+      cta: !!buttonProps.cta,
+      [`btn-${props.buttonStyle}`]: !officialBootstrapStyles.includes(
+        props.buttonStyle || '',
+      ),
+    }),
+  };
+  delete transformedProps.dropdownItems;
+  delete transformedProps.buttonSize;
+  delete transformedProps.buttonStyle;
+  delete transformedProps.cta;
+
+  let button = (
+    <SupersetButton {...transformedProps}>{props.children}</SupersetButton>
+  );
 
   if (dropdownItems) {
     button = (
       <div style={BUTTON_WRAPPER_STYLE}>
-        <SupersetButton {...whittledProps} data-toggle="dropdown">
+        <SupersetButton {...transformedProps} data-toggle="dropdown">
           {props.children}
         </SupersetButton>
         <ul className="dropdown-menu">