From b0515f46cc4d3aabcccb055c41007a9533a85efb Mon Sep 17 00:00:00 2001 From: Marcus Andersson Date: Sun, 12 Jan 2020 19:55:10 +0100 Subject: [PATCH] DeleteButton: Button with icon only was not centered correctly. (#21432) * Refactoring to get the correct spacing between the first icon and button border. * Should not be smaller then 8px to the left. * Removed unused dependency. * Updated snapshot for LinkButton. --- .../src/components/Button/Button.tsx | 9 +- .../src/components/Button/ButtonContent.tsx | 11 +++ .../Button/__snapshots__/Button.test.tsx.snap | 4 +- .../src/components/Button/styles.ts | 97 +++++++++++-------- .../grafana-ui/src/components/Button/types.ts | 3 +- 5 files changed, 76 insertions(+), 48 deletions(-) diff --git a/packages/grafana-ui/src/components/Button/Button.tsx b/packages/grafana-ui/src/components/Button/Button.tsx index 664d1955aa2..cad5db2b3d6 100644 --- a/packages/grafana-ui/src/components/Button/Button.tsx +++ b/packages/grafana-ui/src/components/Button/Button.tsx @@ -2,7 +2,6 @@ import React, { AnchorHTMLAttributes, ButtonHTMLAttributes, useContext } from 'r import { ThemeContext } from '../../themes'; import { getButtonStyles } from './styles'; import { ButtonContent } from './ButtonContent'; -import cx from 'classnames'; import { ButtonSize, ButtonStyles, ButtonVariant } from './types'; type CommonProps = { @@ -31,11 +30,11 @@ export const Button = React.forwardRef((props, r theme, size: size || 'md', variant: variant || 'primary', + textAndIcon: !!(children && icon), }); - const buttonClassName = cx(styles.button, icon && styles.buttonWithIcon, icon && !children && styles.iconButton); return ( - ); @@ -59,11 +58,11 @@ export const LinkButton = React.forwardRef(( theme, size: size || 'md', variant: variant || 'primary', + textAndIcon: !!(children && icon), }); - const buttonClassName = cx(styles.button, icon && styles.buttonWithIcon, icon && !children && styles.iconButton); return ( - + {children} ); diff --git a/packages/grafana-ui/src/components/Button/ButtonContent.tsx b/packages/grafana-ui/src/components/Button/ButtonContent.tsx index 28b77dcbfaa..8e818fd6814 100644 --- a/packages/grafana-ui/src/components/Button/ButtonContent.tsx +++ b/packages/grafana-ui/src/components/Button/ButtonContent.tsx @@ -2,6 +2,7 @@ import React from 'react'; import { css } from 'emotion'; import { stylesFactory, useTheme } from '../../themes'; import { GrafanaTheme } from '@grafana/data'; +import { ButtonSize } from './types'; const getStyles = stylesFactory((theme: GrafanaTheme) => ({ content: css` @@ -23,12 +24,22 @@ type Props = { icon?: string; className?: string; children: React.ReactNode; + size?: ButtonSize; }; + export function ButtonContent(props: Props) { const { icon, children } = props; const theme = useTheme(); const styles = getStyles(theme); + if (!children) { + return ( + + + + ); + } + const iconElement = icon && ( diff --git a/packages/grafana-ui/src/components/Button/__snapshots__/Button.test.tsx.snap b/packages/grafana-ui/src/components/Button/__snapshots__/Button.test.tsx.snap index 3abfa65f8b6..fdd2afd9875 100644 --- a/packages/grafana-ui/src/components/Button/__snapshots__/Button.test.tsx.snap +++ b/packages/grafana-ui/src/components/Button/__snapshots__/Button.test.tsx.snap @@ -1,5 +1,5 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`Button renders correct html 1`] = `""`; +exports[`Button renders correct html 1`] = `""`; -exports[`LinkButton renders correct html 1`] = `"Click me"`; +exports[`LinkButton renders correct html 1`] = `"Click me"`; diff --git a/packages/grafana-ui/src/components/Button/styles.ts b/packages/grafana-ui/src/components/Button/styles.ts index cbef4e28eea..a94fa82012f 100644 --- a/packages/grafana-ui/src/components/Button/styles.ts +++ b/packages/grafana-ui/src/components/Button/styles.ts @@ -1,7 +1,8 @@ import tinycolor from 'tinycolor2'; import { css } from 'emotion'; import { selectThemeVariant, stylesFactory } from '../../themes'; -import { StyleDeps } from './types'; +import { StyleDeps, ButtonSize } from './types'; +import { GrafanaTheme } from '@grafana/data'; const buttonVariantStyles = ( from: string, @@ -24,39 +25,11 @@ const buttonVariantStyles = ( } `; -export const getButtonStyles = stylesFactory(({ theme, size, variant }: StyleDeps) => { +export const getButtonStyles = stylesFactory(({ theme, size, variant, textAndIcon }: StyleDeps) => { const borderRadius = theme.border.radius.sm; - let padding, - background, - fontSize, - height, - fontWeight = theme.typography.weight.semibold; + const { padding, fontSize, height, fontWeight } = calculateMeasures(theme, size, !!textAndIcon); - switch (size) { - case 'sm': - padding = `${theme.spacing.xs} ${theme.spacing.sm}`; - fontSize = theme.typography.size.sm; - height = theme.height.sm; - break; - - case 'md': - padding = `${theme.spacing.sm} ${theme.spacing.md}`; - fontSize = theme.typography.size.md; - height = theme.height.md; - break; - - case 'lg': - padding = `${theme.spacing.md} ${theme.spacing.lg}`; - fontSize = theme.typography.size.lg; - fontWeight = theme.typography.weight.regular; - height = theme.height.lg; - break; - - default: - padding = `${theme.spacing.sm} ${theme.spacing.md}`; - fontSize = theme.typography.size.base; - height = theme.height.md; - } + let background; switch (variant) { case 'primary': @@ -120,13 +93,6 @@ export const getButtonStyles = stylesFactory(({ theme, size, variant }: StyleDep box-shadow: none; } `, - buttonWithIcon: css` - padding-left: ${theme.spacing.sm}; - `, - // used for buttons with icon onlys - iconButton: css` - padding-right: 0; - `, iconWrap: css` label: button-icon-wrap; & + * { @@ -135,3 +101,56 @@ export const getButtonStyles = stylesFactory(({ theme, size, variant }: StyleDep `, }; }); + +type ButtonMeasures = { + padding: string; + fontSize: string; + height: string; + fontWeight: number; +}; + +const calculateMeasures = (theme: GrafanaTheme, size: ButtonSize, textAndIcon: boolean): ButtonMeasures => { + switch (size) { + case 'sm': { + return { + padding: `${theme.spacing.xs} ${theme.spacing.sm}`, + fontSize: theme.typography.size.sm, + height: theme.height.sm, + fontWeight: theme.typography.weight.semibold, + }; + } + + case 'md': { + const leftPadding = textAndIcon ? theme.spacing.sm : theme.spacing.md; + + return { + padding: `${theme.spacing.sm} ${theme.spacing.md} ${theme.spacing.sm} ${leftPadding}`, + fontSize: theme.typography.size.md, + height: theme.height.md, + fontWeight: theme.typography.weight.semibold, + }; + } + + case 'lg': { + const leftPadding = textAndIcon ? theme.spacing.md : theme.spacing.lg; + + return { + padding: `${theme.spacing.md} ${theme.spacing.lg} ${theme.spacing.md} ${leftPadding}`, + fontSize: theme.typography.size.lg, + height: theme.height.lg, + fontWeight: theme.typography.weight.regular, + }; + } + + default: { + const leftPadding = textAndIcon ? theme.spacing.sm : theme.spacing.md; + + return { + padding: `${theme.spacing.sm} ${theme.spacing.md} ${theme.spacing.sm} ${leftPadding}`, + fontSize: theme.typography.size.base, + height: theme.height.md, + fontWeight: theme.typography.weight.regular, + }; + } + } +}; diff --git a/packages/grafana-ui/src/components/Button/types.ts b/packages/grafana-ui/src/components/Button/types.ts index eb67b3c0420..a6f6f6ba865 100644 --- a/packages/grafana-ui/src/components/Button/types.ts +++ b/packages/grafana-ui/src/components/Button/types.ts @@ -8,12 +8,11 @@ export interface StyleDeps { theme: GrafanaTheme; size: ButtonSize; variant: ButtonVariant; + textAndIcon?: boolean; } export interface ButtonStyles { button: string; - buttonWithIcon: string; - iconButton: string; iconWrap: string; icon?: string; }