Grafana/ui: Refactor button and add default type = button (#20042)

packages/grafana-ui/src/components/Button/Button.story.tsx

@@ -5,7 +5,6 @@ import withPropsCombinations from 'react-storybook-addon-props-combinations';
 import { action } from '@storybook/addon-actions';
 import { ThemeableCombinationsRowRenderer } from '../../utils/storybook/CombinationsRowRenderer';
 import { select, boolean } from '@storybook/addon-knobs';
-import { CommonButtonProps } from './types';
 
 const ButtonStories = storiesOf('UI/Button', module);
 
@@ -22,7 +21,7 @@ const combinationOptions = {
   CombinationRenderer: ThemeableCombinationsRowRenderer,
 };
 
-const renderButtonStory = (buttonComponent: React.ComponentType<CommonButtonProps>) => {
+const renderButtonStory = (buttonComponent: typeof Button | typeof LinkButton) => {
   const isDisabled = boolean('Disable button', false);
   return withPropsCombinations(
     buttonComponent,

packages/grafana-ui/src/components/Button/Button.test.tsx

+import React from 'react';
+import { Button, LinkButton } from './Button';
+import { mount } from 'enzyme';
+
+describe('Button', () => {
+  it('renders correct html', () => {
+    const wrapper = mount(<Button icon={'fa fa-plus'}>Click me</Button>);
+    expect(wrapper.html()).toMatchSnapshot();
+  });
+});
+
+describe('LinkButton', () => {
+  it('renders correct html', () => {
+    const wrapper = mount(<LinkButton icon={'fa fa-plus'}>Click me</LinkButton>);
+    expect(wrapper.html()).toMatchSnapshot();
+  });
+
+  it('allows a disable state on link button', () => {
+    const wrapper = mount(
+      <LinkButton disabled icon={'fa fa-plus'}>
+        Click me
+      </LinkButton>
+    );
+    expect(wrapper.find('a[disabled]').length).toBe(1);
+  });
+});

packages/grafana-ui/src/components/Button/Button.tsx

-import React, { useContext } from 'react';
-import { AbstractButton } from './AbstractButton';
+import React, { AnchorHTMLAttributes, ButtonHTMLAttributes, useContext } from 'react';
 import { ThemeContext } from '../../themes';
-import { ButtonProps, LinkButtonProps } from './types';
+import { getButtonStyles } from './styles';
+import { ButtonContent } from './ButtonContent';
+import cx from 'classnames';
+import { ButtonSize, ButtonStyles, ButtonVariant } from './types';
 
+type CommonProps = {
+  size?: ButtonSize;
+  variant?: ButtonVariant;
+  /**
+   * icon prop is a temporary solution. It accepts legacy icon class names for the icon to be rendered.
+   * TODO: migrate to a component when we are going to migrate icons to @grafana/ui
+   */
+  icon?: string;
+  className?: string;
+  styles?: ButtonStyles;
+};
+
+type ButtonProps = CommonProps & ButtonHTMLAttributes<HTMLButtonElement>;
 export const Button: React.FunctionComponent<ButtonProps> = props => {
   const theme = useContext(ThemeContext);
-  return <AbstractButton {...props} renderAs="button" theme={theme} />;
+  const { size, variant, icon, children, className, styles: stylesProp, ...buttonProps } = props;
+
+  // Default this to 'button', otherwise html defaults to 'submit' which then submits any form it is in.
+  buttonProps.type = buttonProps.type || 'button';
+  const styles =
+    stylesProp || getButtonStyles({ theme, size: size || 'md', variant: variant || 'primary', withIcon: !!icon });
+
+  return (
+    <button className={cx(styles.button, className)} {...buttonProps}>
+      <ButtonContent iconClassName={styles.icon} className={styles.iconWrap} icon={icon}>
+        {children}
+      </ButtonContent>
+    </button>
+  );
 };
 Button.displayName = 'Button';
 
+type LinkButtonProps = CommonProps &
+  AnchorHTMLAttributes<HTMLAnchorElement> & {
+    // We allow disabled here even though it is not standard for a link. We use it as a selector to style it as
+    // disabled.
+    disabled?: boolean;
+  };
 export const LinkButton: React.FunctionComponent<LinkButtonProps> = props => {
   const theme = useContext(ThemeContext);
-  return <AbstractButton {...props} renderAs="a" theme={theme} />;
+  const { size, variant, icon, children, className, styles: stylesProp, ...anchorProps } = props;
+  const styles =
+    stylesProp || getButtonStyles({ theme, size: size || 'md', variant: variant || 'primary', withIcon: !!icon });
+
+  return (
+    <a className={cx(styles.button, className)} {...anchorProps}>
+      <ButtonContent iconClassName={styles.icon} className={styles.iconWrap} icon={icon}>
+        {children}
+      </ButtonContent>
+    </a>
+  );
 };
 LinkButton.displayName = 'LinkButton';

packages/grafana-ui/src/components/Button/ButtonContent.tsx

+import React from 'react';
+import cx from 'classnames';
+
+type Props = {
+  icon?: string;
+  className: string;
+  iconClassName: string;
+  children: React.ReactNode;
+};
+export function ButtonContent(props: Props) {
+  const { icon, className, iconClassName, children } = props;
+  return icon ? (
+    <span className={className}>
+      <i className={cx([icon, iconClassName])} />
+      <span>{children}</span>
+    </span>
+  ) : (
+    <>{children}</>
+  );
+}

packages/grafana-ui/src/components/Button/__snapshots__/Button.test.tsx.snap

+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`Button renders correct html 1`] = `"<button class=\\"css-pdywgc-button\\" type=\\"button\\"><span class=\\"css-1dxly9g-button-icon-wrap\\"><i class=\\"fa fa-plus css-iu6xgj-button-icon\\"></i><span>Click me</span></span></button>"`;
+
+exports[`LinkButton renders correct html 1`] = `"<a class=\\"css-pdywgc-button\\"><span class=\\"css-1dxly9g-button-icon-wrap\\"><i class=\\"fa fa-plus css-iu6xgj-button-icon\\"></i><span>Click me</span></span></a>"`;

packages/grafana-ui/src/components/Button/AbstractButton.tsx → packages/grafana-ui/src/components/Button/styles.ts

-import React, { ComponentType, ReactNode } from 'react';
 import tinycolor from 'tinycolor2';
-import { css, cx } from 'emotion';
+import { css } from 'emotion';
 import { selectThemeVariant, stylesFactory } from '../../themes';
-import { AbstractButtonProps, ButtonSize, ButtonStyles, ButtonVariant, CommonButtonProps, StyleDeps } from './types';
-import { GrafanaTheme } from '../../types';
+import { StyleDeps } from './types';
 
 const buttonVariantStyles = (
   from: string,
@@ -26,7 +24,7 @@ const buttonVariantStyles = (
   }
 `;
 
-const getButtonStyles = stylesFactory(({ theme, size, variant, withIcon }: StyleDeps) => {
+export const getButtonStyles = stylesFactory(({ theme, size, variant, withIcon }: StyleDeps) => {
   const borderRadius = theme.border.radius.sm;
   let padding,
     background,
@@ -140,63 +138,3 @@ const getButtonStyles = stylesFactory(({ theme, size, variant, withIcon }: Style
     `,
   };
 });
-
-export const renderButton = (
-  theme: GrafanaTheme,
-  buttonStyles: ButtonStyles,
-  renderAs: ComponentType<CommonButtonProps> | string,
-  children: ReactNode,
-  size: ButtonSize,
-  variant: ButtonVariant,
-  icon?: string,
-  className?: string,
-  otherProps?: Partial<AbstractButtonProps>
-) => {
-  const nonHtmlProps = {
-    theme,
-    size,
-    variant,
-  };
-  const finalClassName = cx(buttonStyles.button, className);
-  const finalChildren = icon ? (
-    <span className={buttonStyles.iconWrap}>
-      <i className={cx([icon, buttonStyles.icon])} />
-      <span>{children}</span>
-    </span>
-  ) : (
-    children
-  );
-
-  const finalProps =
-    typeof renderAs === 'string'
-      ? {
-          ...otherProps,
-          className: finalClassName,
-          children: finalChildren,
-        }
-      : {
-          ...otherProps,
-          ...nonHtmlProps,
-          className: finalClassName,
-          children: finalChildren,
-        };
-
-  return React.createElement(renderAs, finalProps);
-};
-
-export const AbstractButton: React.FunctionComponent<AbstractButtonProps> = ({
-  renderAs,
-  theme,
-  size = 'md',
-  variant = 'primary',
-  className,
-  icon,
-  children,
-  ...otherProps
-}) => {
-  const buttonStyles = getButtonStyles({ theme, size, variant, withIcon: !!icon });
-
-  return renderButton(theme, buttonStyles, renderAs, children, size, variant, icon, className, otherProps);
-};
-
-AbstractButton.displayName = 'AbstractButton';

packages/grafana-ui/src/components/Button/types.ts

-import { AnchorHTMLAttributes, ButtonHTMLAttributes, ComponentType } from 'react';
-import { GrafanaTheme, Themeable } from '../../types';
+import { GrafanaTheme } from '../../types';
 
 export type ButtonVariant = 'primary' | 'secondary' | 'danger' | 'inverse' | 'transparent' | 'destructive';
 
@@ -17,23 +16,3 @@ export interface ButtonStyles {
   iconWrap: string;
   icon: string;
 }
-
-export interface CommonButtonProps {
-  size?: ButtonSize;
-  variant?: ButtonVariant;
-  /**
-   * icon prop is a temporary solution. It accepts legacy icon class names for the icon to be rendered.
-   * TODO: migrate to a component when we are going to migrate icons to @grafana/ui
-   */
-  icon?: string;
-  className?: string;
-}
-
-export interface LinkButtonProps extends CommonButtonProps, AnchorHTMLAttributes<HTMLAnchorElement> {
-  disabled?: boolean;
-}
-export interface ButtonProps extends CommonButtonProps, ButtonHTMLAttributes<HTMLButtonElement> {}
-
-export interface AbstractButtonProps extends CommonButtonProps, Themeable {
-  renderAs: ComponentType<CommonButtonProps> | string;
-}

packages/grafana-ui/src/components/Forms/Button.story.tsx

 import React from 'react';
-import { Button } from './Button';
+import { Button, ButtonVariant } from './Button';
 import { withCenteredStory, withHorizontallyCenteredStory } from '../../utils/storybook/withCenteredStory';
 import { select, text } from '@storybook/addon-knobs';
-import { ButtonSize, ButtonVariant } from '../Button/types';
+import { ButtonSize } from '../Button/types';
 import mdx from './Button.mdx';
 
 export default {
@@ -26,7 +26,7 @@ export const simple = () => {
   const buttonText = text('text', 'Button');
 
   return (
-    <Button variant={variant as ButtonVariant} size={size as ButtonSize} renderAs="button">
+    <Button variant={variant as ButtonVariant} size={size as ButtonSize}>
       {buttonText}
     </Button>
   );

packages/grafana-ui/src/components/Forms/Button.tsx

-import { FC } from 'react';
+import React, { AnchorHTMLAttributes, ButtonHTMLAttributes, useContext } from 'react';
 import { css, cx } from 'emotion';
 import tinycolor from 'tinycolor2';
-import { selectThemeVariant, stylesFactory, useTheme } from '../../themes';
-import { renderButton } from '../Button/AbstractButton';
+import { selectThemeVariant, stylesFactory, ThemeContext } from '../../themes';
+import { Button as DefaultButton, LinkButton as DefaultLinkButton } from '../Button/Button';
 import { getFocusStyle } from './commonStyles';
-import { AbstractButtonProps, ButtonSize, ButtonVariant, StyleDeps } from '../Button/types';
+import { ButtonSize, StyleDeps } from '../Button/types';
 import { GrafanaTheme } from '../../types';
 
 const buttonVariantStyles = (from: string, to: string, textColor: string) => css`
@@ -96,7 +96,9 @@ const getPropertiesForVariant = (theme: GrafanaTheme, variant: ButtonVariant) =>
   }
 };
 
-export const getButtonStyles = stylesFactory(({ theme, size, variant, withIcon }: StyleDeps) => {
+// Need to do this because of mismatch between variants in standard buttons and here
+type StyleProps = Omit<StyleDeps, 'variant'> & { variant: ButtonVariant };
+export const getButtonStyles = stylesFactory(({ theme, size, variant, withIcon }: StyleProps) => {
   const { padding, fontSize, iconDistance, height } = getPropertiesForSize(theme, size);
   const { background, borderColor } = getPropertiesForVariant(theme, variant);
 
@@ -142,17 +144,38 @@ export const getButtonStyles = stylesFactory(({ theme, size, variant, withIcon }
   };
 });
 
-export const Button: FC<Omit<AbstractButtonProps, 'theme'>> = ({
-  renderAs,
-  size = 'md',
-  variant = 'primary',
-  className,
-  icon,
-  children,
-  ...otherProps
-}) => {
-  const theme = useTheme();
-  const buttonStyles = getButtonStyles({ theme, size, variant, withIcon: !!icon });
-
-  return renderButton(theme, buttonStyles, renderAs, children, size, variant, icon, className, otherProps);
+// These are different from the standard Button where there are 5 variants.
+export type ButtonVariant = 'primary' | 'secondary' | 'destructive';
+
+// These also needs to be different because the ButtonVariant is different
+type CommonProps = {
+  size?: ButtonSize;
+  variant?: ButtonVariant;
+  icon?: string;
+  className?: string;
+};
+
+type ButtonProps = CommonProps & ButtonHTMLAttributes<HTMLButtonElement>;
+
+export const Button = (props: ButtonProps) => {
+  const theme = useContext(ThemeContext);
+  const styles = getButtonStyles({
+    theme,
+    size: props.size || 'md',
+    variant: props.variant || 'primary',
+    withIcon: !!props.icon,
+  });
+  return <DefaultButton {...props} styles={styles} />;
+};
+
+type ButtonLinkProps = CommonProps & AnchorHTMLAttributes<HTMLAnchorElement>;
+export const LinkButton = (props: ButtonLinkProps) => {
+  const theme = useContext(ThemeContext);
+  const styles = getButtonStyles({
+    theme,
+    size: props.size || 'md',
+    variant: props.variant || 'primary',
+    withIcon: !!props.icon,
+  });
+  return <DefaultLinkButton {...props} styles={styles} />;
 };