diff --git a/packages/shared/src/components/buttons/Button.spec.tsx b/packages/shared/src/components/buttons/Button.spec.tsx index 616f6ebb96d..e14bb217203 100644 --- a/packages/shared/src/components/buttons/Button.spec.tsx +++ b/packages/shared/src/components/buttons/Button.spec.tsx @@ -8,7 +8,7 @@ import { ButtonVariant, ButtonIconPosition, } from './Button'; -import { UpvoteIcon } from '../icons'; +import { ArrowIcon, UpvoteIcon } from '../icons'; const renderComponent = ( props: Partial> = {}, @@ -78,7 +78,7 @@ describe('Button', () => { icon: , children: 'Upvote', }); - expect(await screen.findByTestId('icon')).toBeInTheDocument(); + expect(await screen.findByTestId('icon')).toHaveClass('btn-icon-left'); expect(await screen.findByRole('button')).not.toHaveClass('iconOnly'); }); @@ -94,6 +94,7 @@ describe('Button', () => { expect(button).toBeInTheDocument(); expect(rightIcon).toBeInTheDocument(); + expect(rightIcon).toHaveClass('btn-icon-right'); // check if icon appears AFTER the label expect(button.innerHTML.indexOf('Upvote')).toBeLessThan( @@ -141,6 +142,18 @@ describe('Button', () => { expect(loadingLabel).toHaveClass('invisible'); }); + it('does not wrap mixed button content in the label span', () => { + render( + , + ); + + expect(screen.getByTestId('child-icon')).toBeInTheDocument(); + expect(screen.getByRole('button').querySelector('.btn-label')).toBeNull(); + }); + it('should set aria-pressed when pressed is true', async () => { renderComponent({ children: 'Button', pressed: true }); expect(await screen.findByRole('button')).toHaveAttribute( diff --git a/packages/shared/src/components/buttons/Button.tsx b/packages/shared/src/components/buttons/Button.tsx index c6971c143c5..4c23fce469c 100644 --- a/packages/shared/src/components/buttons/Button.tsx +++ b/packages/shared/src/components/buttons/Button.tsx @@ -16,7 +16,6 @@ import { VariantColorToClassName, VariantToClassName, } from './common'; -import { isNullOrUndefined } from '../../lib/func'; import classed from '../../lib/classed'; export type IconType = React.ReactElement; @@ -83,7 +82,13 @@ function ButtonComponent( }: ButtonProps, ref?: Ref>, ): ReactElement { - const hasChildren = !isNullOrUndefined(children); + const childNodes = React.Children.toArray(children); + const hasChildren = childNodes.length > 0; + const shouldWrapLabel = + hasChildren && + childNodes.every( + (child) => typeof child === 'string' || typeof child === 'number', + ); const iconOnly = icon && !hasChildren; const getIconWithSize = useGetIconWithSize( size, @@ -91,12 +96,20 @@ function ButtonComponent( iconPosition, ); const isAnchor = Tag === 'a'; + const anchorClickProps = + isAnchor && onClick + ? combinedClicks( + onClick as React.MouseEventHandler, + ) + : {}; + const isOptionOrQuiz = + variant === ButtonVariant.Option || variant === ButtonVariant.Quiz; const [isHovering, setIsHovering] = useState(false); return ( ( `btn focus-outline inline-flex cursor-pointer select-none flex-row items-center border no-underline shadow-none transition duration-200 ease-in-out typo-callout`, - ![ButtonVariant.Option, ButtonVariant.Quiz].includes(variant) && - 'justify-center font-bold', + !isOptionOrQuiz && 'justify-center font-bold', { iconOnly }, iconOnly ? IconOnlySizeToClassName[size] : SizeToClassName[size], iconPosition === ButtonIconPosition.Top && `flex-col !px-2`, - !color && VariantToClassName[variant], - VariantColorToClassName[variant]?.[color], + variant && !color && VariantToClassName[variant], + variant && color && VariantColorToClassName[variant]?.[color], className, )} onMouseEnter={(e: React.MouseEvent) => { @@ -127,10 +139,12 @@ function ButtonComponent( iconPosition, ) && getIconWithSize(icon, iconSecondaryOnHover ? isHovering : false)} - {hasChildren && ( + {shouldWrapLabel ? ( {children} + ) : ( + children )} {icon && iconPosition === ButtonIconPosition.Right && diff --git a/packages/shared/src/components/buttons/common.ts b/packages/shared/src/components/buttons/common.ts index 1e6d9d4584a..ef79c677bb5 100644 --- a/packages/shared/src/components/buttons/common.ts +++ b/packages/shared/src/components/buttons/common.ts @@ -232,13 +232,19 @@ export const useGetIconWithSize = ( size: icon.props?.size ?? buttonSizeToIconSize[size], className: classNames( icon.props.className, + 'btn-icon', !iconOnly && 'text-base', - !iconOnly && iconPosition === ButtonIconPosition.Left && 'mr-1', + !iconOnly && + iconPosition === ButtonIconPosition.Left && + 'btn-icon-left mr-1', !iconOnly && !icon.props?.size && iconPosition === ButtonIconPosition.Left && '-ml-2', - !iconOnly && iconPosition === ButtonIconPosition.Right && 'ml-1', + !iconOnly && + iconPosition === ButtonIconPosition.Right && + 'btn-icon-right ml-1', + !iconOnly && iconPosition === ButtonIconPosition.Top && 'btn-icon-top', !iconOnly && !icon.props?.size && iconPosition === ButtonIconPosition.Right && diff --git a/packages/shared/src/styles/components/buttons.css b/packages/shared/src/styles/components/buttons.css index 6ce05c9e7d9..b71309bdbe1 100644 --- a/packages/shared/src/styles/components/buttons.css +++ b/packages/shared/src/styles/components/buttons.css @@ -49,18 +49,14 @@ } &:not(.iconOnly) { - & .icon { + & .btn-icon-left { margin-left: -0.5rem; margin-right: 0.25rem; + } - &:not(:first-child) { - margin-left: 0.25rem; - margin-right: -0.5rem; - } - - &:only-child { - margin-right: -0.5rem; - } + & .btn-icon-right { + margin-left: 0.25rem; + margin-right: -0.5rem; } & .socialIcon {