Skip to content

Commit

Permalink
fix(Button): use height and width to fix sizes in Icon Only Button (#…
Browse files Browse the repository at this point in the history
…2078)

* feat: move to height width in icon-only button

* fix: remove unused tokens

* tests: update snapshot

* Create few-camels-argue.md

* feat: add temporary ts-ignore

* fix: ButtonGroup icon widths

* fix: snapshots
  • Loading branch information
saurabhdaware authored Mar 20, 2024
1 parent c7f359b commit 9dcc291
Show file tree
Hide file tree
Showing 15 changed files with 110 additions and 171 deletions.
5 changes: 5 additions & 0 deletions .changeset/few-camels-argue.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@razorpay/blade": patch
---

fix(Button): use height and width to fix sizes in Icon Only Button
65 changes: 15 additions & 50 deletions packages/blade/src/components/Button/BaseButton/BaseButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,27 +4,26 @@ import React from 'react';
import styled from 'styled-components';
import type { GestureResponderEvent } from 'react-native';
import StyledBaseButton from './StyledBaseButton';
import type { ButtonTypography, ButtonMinHeight } from './buttonTokens';
import type { ButtonTypography } from './buttonTokens';
import {
textColor,
backgroundColor,
buttonIconOnlyPadding,
buttonIconOnlySizeToIconSizeMap,
typography as buttonTypography,
minHeight as buttonMinHeight,
buttonSizeToIconSizeMap,
buttonSizeToSpinnerSizeMap,
buttonIconPadding,
buttonPadding,
buttonIconOnlyHeightWidth,
} from './buttonTokens';
import type { BaseButtonStyleProps, IconColor } from './types';
import AnimatedButtonContent from './AnimatedButtonContent';
import type { DotNotationToken } from '~utils/lodashButBetter/get';
import getIn from '~utils/lodashButBetter/get';
import type { BaseLinkProps } from '~components/Link/BaseLink';
import type { Theme } from '~components/BladeProvider';
import type { IconComponent, IconProps, IconSize } from '~components/Icons';
import type { DurationString, EasingString } from '~tokens/global';
import type { BorderRadiusValues, BorderWidthValues, SpacingValues } from '~tokens/theme/theme';
import type { IconComponent } from '~components/Icons';
import type { Platform } from '~utils';
import { isReactNative } from '~utils';
import type { StyledPropsBlade } from '~components/Box/styledProps';
Expand All @@ -33,15 +32,9 @@ import { getStyledProps } from '~components/Box/styledProps';
import { BaseText } from '~components/Typography/BaseText';
import { useTheme } from '~components/BladeProvider';
import { announce } from '~components/LiveAnnouncer';
import type { BaseSpinnerProps } from '~components/Spinner/BaseSpinner';
import { BaseSpinner } from '~components/Spinner/BaseSpinner';
import BaseBox from '~components/Box/BaseBox';
import type {
BladeElementRef,
DotNotationSpacingStringToken,
StringChildrenType,
TestID,
} from '~utils/types';
import type { BladeElementRef, StringChildrenType, TestID } from '~utils/types';
import type { BaseTextProps } from '~components/Typography/BaseText/types';
import { assignWithoutSideEffects } from '~utils/assignWithoutSideEffects';
import { usePrevious } from '~utils/usePrevious';
Expand Down Expand Up @@ -107,11 +100,6 @@ type BaseButtonColorTokenModifiers = {
color: BaseButtonProps['color'];
};

/**
* All possible icon colors, derived from `IconProps` minus `currentColor` because possible values should only be from tokens
*/
type IconColor = Exclude<IconProps['color'], 'currentColor'>;

const getRenderElement = (href?: string): 'a' | 'button' | undefined => {
if (isReactNative()) {
return undefined; // as property doesn't work with react native
Expand Down Expand Up @@ -178,33 +166,6 @@ const getTextColorToken = ({
return tokens.base[variant][_state];
};

type BaseButtonStyleProps = {
iconSize: IconSize;
spinnerSize: BaseSpinnerProps['size'];
fontSize: keyof Theme['typography']['fonts']['size'];
lineHeight: keyof Theme['typography']['lineHeights'];
minHeight: `${ButtonMinHeight}px`;
iconPadding?: DotNotationSpacingStringToken;
iconColor: IconColor;
textColor: BaseTextProps['color'];
buttonPaddingTop: SpacingValues;
buttonPaddingBottom: SpacingValues;
buttonPaddingLeft: SpacingValues;
buttonPaddingRight: SpacingValues;
text?: string;
defaultBackgroundColor: string;
defaultBorderColor: string;
hoverBackgroundColor: string;
hoverBorderColor: string;
focusBackgroundColor: string;
focusBorderColor: string;
focusRingColor: string;
motionDuration: DurationString;
motionEasing: EasingString;
borderWidth: BorderWidthValues;
borderRadius: BorderRadiusValues;
};

const getProps = ({
buttonTypographyTokens,
children,
Expand Down Expand Up @@ -238,6 +199,8 @@ const getProps = ({
fontSize: buttonTypographyTokens.fonts.size[size],
lineHeight: buttonTypographyTokens.lineHeights[size],
minHeight: makeSize(buttonMinHeight[size]),
height: isIconOnly ? buttonIconOnlyHeightWidth[size] : undefined,
width: isIconOnly ? buttonIconOnlyHeightWidth[size] : undefined,
iconPadding: hasIcon && children?.trim() ? `spacing.${buttonIconPadding[size]}` : undefined,
iconColor: getTextColorToken({
property: 'icon',
Expand All @@ -251,17 +214,15 @@ const getProps = ({
color,
state: 'default',
}) as BaseTextProps['color'],
buttonPaddingTop: isIconOnly
? makeSpace(theme.spacing[buttonIconOnlyPadding[size].top])
: makeSpace(theme.spacing[buttonPadding[size].top]),
buttonPaddingTop: isIconOnly ? makeSpace(0) : makeSpace(theme.spacing[buttonPadding[size].top]),
buttonPaddingBottom: isIconOnly
? makeSpace(theme.spacing[buttonIconOnlyPadding[size].bottom])
? makeSpace(0)
: makeSpace(theme.spacing[buttonPadding[size].bottom]),
buttonPaddingLeft: isIconOnly
? makeSpace(theme.spacing[buttonIconOnlyPadding[size].left])
? makeSpace(0)
: makeSpace(theme.spacing[buttonPadding[size].left]),
buttonPaddingRight: isIconOnly
? makeSpace(theme.spacing[buttonIconOnlyPadding[size].right])
? makeSpace(0)
: makeSpace(theme.spacing[buttonPadding[size].right]),
text: size === 'xsmall' ? children?.trim().toUpperCase() : children?.trim(),
defaultBackgroundColor: getIn(
Expand Down Expand Up @@ -391,6 +352,8 @@ const _BaseButton: React.ForwardRefRenderFunction<BladeElementRef, BaseButtonPro
defaultBorderColor,
defaultBackgroundColor,
minHeight,
height,
width,
buttonPaddingTop,
buttonPaddingBottom,
buttonPaddingLeft,
Expand Down Expand Up @@ -512,6 +475,8 @@ const _BaseButton: React.ForwardRefRenderFunction<BladeElementRef, BaseButtonPro
borderRadius={borderRadius}
motionDuration={motionDuration}
motionEasing={motionEasing}
height={height}
width={width}
isPressed={isPressed}
onMouseDown={handlePointerPressedIn}
onMouseUp={handlePointerPressedOut}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,7 @@ exports[`<BaseButton /> should render button with icon with default iconPosition
"alignItems": "center",
"display": "flex",
"justifyContent": "center",
"paddingRight": 4,
"paddingRight": 8,
},
]
}
Expand Down Expand Up @@ -747,7 +747,7 @@ exports[`<BaseButton /> should render button with icon with left iconPosition 1`
"alignItems": "center",
"display": "flex",
"justifyContent": "center",
"paddingRight": 4,
"paddingRight": 8,
},
]
}
Expand Down Expand Up @@ -1029,7 +1029,7 @@ exports[`<BaseButton /> should render button with icon with right iconPosition 1
"alignItems": "center",
"display": "flex",
"justifyContent": "center",
"paddingLeft": 4,
"paddingLeft": 8,
},
]
}
Expand Down Expand Up @@ -1127,8 +1127,8 @@ exports[`<BaseButton /> should render button with icon without text 1`] = `
borderRadius="4px"
borderWidth="0px"
buttonPaddingBottom="0px"
buttonPaddingLeft="8px"
buttonPaddingRight="8px"
buttonPaddingLeft="0px"
buttonPaddingRight="0px"
buttonPaddingTop="0px"
collapsable={false}
data-blade-component="button"
Expand All @@ -1138,6 +1138,7 @@ exports[`<BaseButton /> should render button with icon without text 1`] = `
focusBorderColor="hsla(227, 100%, 59%, 1)"
focusRingColor="hsla(227, 100%, 59%, 0.18)"
focusable={true}
height="36px"
hoverBackgroundColor="hsla(227, 71%, 51%, 1)"
hoverBorderColor="hsla(227, 100%, 59%, 1)"
isFullWidth={false}
Expand Down Expand Up @@ -1175,17 +1176,18 @@ exports[`<BaseButton /> should render button with icon without text 1`] = `
"cursor": "pointer",
"display": "flex",
"flexDirection": "row",
"height": 36,
"justifyContent": "center",
"minHeight": 36,
"overflow": "hidden",
"paddingBottom": 0,
"paddingLeft": 8,
"paddingRight": 8,
"paddingLeft": 0,
"paddingRight": 0,
"paddingTop": 0,
"textDecorationColor": "black",
"textDecorationLine": "none",
"textDecorationStyle": "solid",
"width": "auto",
"width": 36,
},
{
"backgroundColor": "hsla(227, 100%, 59%, 1)",
Expand All @@ -1194,6 +1196,7 @@ exports[`<BaseButton /> should render button with icon without text 1`] = `
]
}
type="button"
width="36px"
>
<View
style={
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`<BaseButton /> should render button with icon with left iconPosition 1`] = `"<div id="root"><button type="button" data-blade-component="button" role="button" class="StyledBaseButtonweb__StyledBaseButton-sc-26bt38-0 ctKBsz"><div data-blade-component="base-box" class="BaseBox-bmPWx AnimatedButtonContentweb__AnimatedButtonContent-sc-1fkx0t6-0 jPBClF"><div data-blade-component="base-box" class="BaseBox-bmPWx BaseButton__ButtonContent-zf1huq-0 LeezU esItiH"><div data-blade-component="base-box" class="BaseBox-bmPWx vtnBD"><svg aria-hidden="true" data-blade-component="icon" height="16px" viewBox="0 0 24 24" width="16px" fill="none" class="Svgweb__StyledSvg-vcmjs8-0"><path d="M3 3C1.34315 3 0 4.34315 0 6V18C0 19.6569 1.34314 21 3 21H21C22.6569 21 24 19.6569 24 18V6C24 4.34315 22.6569 3 21 3H3ZM22 9V6C22 5.44771 21.5523 5 21 5H3C2.44772 5 2 5.44772 2 6V9H22ZM2 11H22V18C22 18.5523 21.5523 19 21 19H3C2.44772 19 2 18.5523 2 18V11Z" clip-rule="evenodd" fill="hsla(0, 0%, 100%, 1)" fill-rule="evenodd" data-blade-component="svg-path"></path></svg></div><div class="StyledBaseText-dVBfTO eGcQGV" data-blade-component="base-text">Pay Now</div></div></div></button></div>"`;
exports[`<BaseButton /> should render button with icon with left iconPosition 1`] = `"<div id="root"><button type="button" data-blade-component="button" role="button" class="StyledBaseButtonweb__StyledBaseButton-sc-26bt38-0 ctKBsz"><div data-blade-component="base-box" class="BaseBox-bmPWx AnimatedButtonContentweb__AnimatedButtonContent-sc-1fkx0t6-0 jPBClF"><div data-blade-component="base-box" class="BaseBox-bmPWx BaseButton__ButtonContent-zf1huq-0 LeezU esItiH"><div data-blade-component="base-box" class="BaseBox-bmPWx gQJHef"><svg aria-hidden="true" data-blade-component="icon" height="16px" viewBox="0 0 24 24" width="16px" fill="none" class="Svgweb__StyledSvg-vcmjs8-0"><path d="M3 3C1.34315 3 0 4.34315 0 6V18C0 19.6569 1.34314 21 3 21H21C22.6569 21 24 19.6569 24 18V6C24 4.34315 22.6569 3 21 3H3ZM22 9V6C22 5.44771 21.5523 5 21 5H3C2.44772 5 2 5.44772 2 6V9H22ZM2 11H22V18C22 18.5523 21.5523 19 21 19H3C2.44772 19 2 18.5523 2 18V11Z" clip-rule="evenodd" fill="hsla(0, 0%, 100%, 1)" fill-rule="evenodd" data-blade-component="svg-path"></path></svg></div><div class="StyledBaseText-dVBfTO eGcQGV" data-blade-component="base-text">Pay Now</div></div></div></button></div>"`;

exports[`<BaseButton /> should render button with icon with left iconPosition 2`] = `
.c0.c0.c0.c0.c0 {
Expand Down Expand Up @@ -101,7 +101,7 @@ exports[`<BaseButton /> should render button with icon with left iconPosition 2`
-webkit-justify-content: center;
-ms-flex-pack: center;
justify-content: center;
padding-right: 4px;
padding-right: 8px;
}
.c1.c1.c1.c1.c1 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -739,7 +739,7 @@ exports[`<BaseButton /> should render button with icon with default iconPosition
-webkit-justify-content: center;
-ms-flex-pack: center;
justify-content: center;
padding-right: 4px;
padding-right: 8px;
}
.c1.c1.c1.c1.c1 {
Expand Down Expand Up @@ -922,7 +922,7 @@ exports[`<BaseButton /> should render button with icon with left iconPosition 1`
-webkit-justify-content: center;
-ms-flex-pack: center;
justify-content: center;
padding-right: 4px;
padding-right: 8px;
}
.c1.c1.c1.c1.c1 {
Expand Down Expand Up @@ -1105,7 +1105,7 @@ exports[`<BaseButton /> should render button with icon with right iconPosition 1
-webkit-justify-content: center;
-ms-flex-pack: center;
justify-content: center;
padding-left: 4px;
padding-left: 8px;
}
.c1.c1.c1.c1.c1 {
Expand Down Expand Up @@ -1192,7 +1192,8 @@ exports[`<BaseButton /> should render button with icon with right iconPosition 1
exports[`<BaseButton /> should render button with icon without text 1`] = `
.c0.c0.c0.c0.c0 {
min-height: 36px;
width: auto;
height: 36px;
width: 36px;
cursor: pointer;
background-color: hsla(227,100%,59%,1);
border-color: hsla(227,100%,59%,1);
Expand All @@ -1201,8 +1202,8 @@ exports[`<BaseButton /> should render button with icon without text 1`] = `
border-style: solid;
padding-top: 0px;
padding-bottom: 0px;
padding-left: 8px;
padding-right: 8px;
padding-left: 0px;
padding-right: 0px;
-webkit-box-pack: center;
-webkit-justify-content: center;
-ms-flex-pack: center;
Expand Down
44 changes: 11 additions & 33 deletions packages/blade/src/components/Button/BaseButton/buttonTokens.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type { SpinnerProps } from '~components/Spinner';
import type { Size } from '~tokens/global';
import { size } from '~tokens/global';
import type { FeedbackColors } from '~tokens/theme/theme';
import { makeSize } from '~utils';

export type ButtonMinHeight = Size[28] | Size[32] | Size[36] | Size[48];

Expand Down Expand Up @@ -188,35 +189,12 @@ const buttonPadding: Record<
},
};

const buttonIconOnlyPadding: Record<
NonNullable<BaseButtonProps['size']>,
Record<'top' | 'bottom' | 'left' | 'right', keyof Theme['spacing']>
> = {
xsmall: {
top: 0,
bottom: 0,
left: 3, // should be `6px` as per design but we're making it `8px` since `6px` is not available as a spacing token
right: 3, // should be `6px` as per design but we're making it `8px` since `6px` is not available as a spacing token
},
small: {
top: 0,
bottom: 0,
left: 3,
right: 3,
},
medium: {
top: 0,
bottom: 0,
left: 3,
right: 3,
},
large: {
top: 0,
bottom: 0,
left: 2,
right: 2,
},
};
const buttonIconOnlyHeightWidth = {
xsmall: makeSize(size['28']),
small: makeSize(size['32']),
medium: makeSize(size['36']),
large: makeSize(size['48']),
} as const;

const buttonSizeToIconSizeMap: Record<NonNullable<BaseButtonProps['size']>, IconSize> = {
xsmall: 'small',
Expand Down Expand Up @@ -244,9 +222,9 @@ const buttonSizeToSpinnerSizeMap: Record<

const buttonIconPadding: Record<NonNullable<BaseButtonProps['size']>, keyof Theme['spacing']> = {
xsmall: 1,
small: 1,
medium: 2,
large: 2,
small: 2,
medium: 3,
large: 3,
};

export {
Expand All @@ -259,5 +237,5 @@ export {
buttonSizeToSpinnerSizeMap,
buttonIconPadding,
buttonPadding,
buttonIconOnlyPadding,
buttonIconOnlyHeightWidth,
};
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,15 @@ const getBaseButtonStyles = ({
isFullWidth,
borderWidth,
borderRadius,
height,
width = 'auto',
}: Omit<
StyledBaseButtonProps,
'children' | 'onClick' | 'accessibilityProps' | 'accessibilityLabel'
>): CSSObject => ({
minHeight,
width: isFullWidth ? '100%' : 'auto',
height,
width: isFullWidth ? '100%' : width,
cursor: disabled ? 'not-allowed' : 'pointer',
backgroundColor: defaultBackgroundColor,
borderColor: defaultBorderColor,
Expand Down
Loading

0 comments on commit 9dcc291

Please sign in to comment.