Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix button errors #4372

Merged
merged 1 commit into from
Jun 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/angry-colts-scream.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@toptal/picasso-button': patch
---

- fix "Maximum update depth exceeded" issue
8 changes: 4 additions & 4 deletions packages/base/Button/src/ButtonAction/ButtonAction.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,13 @@ export const ButtonAction: OverridableComponent<Props> = forwardRef<
Props
>(function ButtonAction(props, ref) {
const {
className,
/* eslint-disable @typescript-eslint/no-unused-vars */
// We use these props only to determine styles
active,
TomasSlama marked this conversation as resolved.
Show resolved Hide resolved
focused,
hovered,
/* eslint-enable @typescript-eslint/no-unused-vars */
className,
disabled,
loading,
icon,
Expand Down Expand Up @@ -95,9 +98,6 @@ export const ButtonAction: OverridableComponent<Props> = forwardRef<
onClick={loading ? undefined : onClick}
className={finalClassName}
contentClassName='font-semibold text-blue-500 text-md'
active={active}
hovered={hovered}
focused={focused}
disabled={disabled}
/>
)
Expand Down
51 changes: 19 additions & 32 deletions packages/base/Button/src/ButtonBase/ButtonBase.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,4 @@
import type {
ReactNode,
ReactElement,
MouseEvent,
ElementType,
FC,
} from 'react'
import type { ReactNode, ReactElement, MouseEvent, ElementType } from 'react'
import React, { forwardRef } from 'react'
import { twMerge } from 'tailwind-merge'
import type {
Expand All @@ -15,6 +9,7 @@ import type {
} from '@toptal/picasso-shared'
import { useTitleCase } from '@toptal/picasso-shared'
import { Button as MUIButtonBase } from '@mui/base/Button'
import type { ButtonRootSlotProps } from '@mui/base/Button'
import { Loader } from '@toptal/picasso-loader'
import { Container } from '@toptal/picasso-container'
import { noop, toTitleCase } from '@toptal/picasso-utils'
Expand Down Expand Up @@ -48,6 +43,8 @@ export interface Props
title?: string
/** HTML type of Button component */
type?: 'button' | 'reset' | 'submit'
/** The HTML element that is ultimately rendered, for example 'button', 'a' or 'label */
rootElementName?: keyof HTMLElementTagNameMap
}

const getClickHandler = (loading?: boolean, handler?: Props['onClick']) =>
Expand All @@ -64,14 +61,14 @@ const getIcon = ({ icon }: { icon?: ReactElement }) => {
})
}

// eslint-disable-next-line @typescript-eslint/no-explicit-any
const isReactComponent = (component: any) => {
return (
component &&
(component.$$typeof === Symbol.for('react.forward_ref') ||
typeof component === 'function')
)
}
const RootElement = forwardRef(
(props: ButtonRootSlotProps & { as: ElementType }, ref) => {
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const { ownerState, as: Root, ...rest } = props

return <Root {...rest} ref={ref} />
}
)

export const ButtonBase: OverridableComponent<Props> = forwardRef<
HTMLButtonElement,
TomasSlama marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -91,32 +88,20 @@ export const ButtonBase: OverridableComponent<Props> = forwardRef<
value,
type,
as = 'button',
rootElementName,
titleCase: propsTitleCase,
...rest
} = props

let RootElement: ElementType | FC = as

if (isReactComponent(RootElement)) {
RootElement = forwardRef(
// We don't need to pass ownerState to the root component
// eslint-disable-next-line @typescript-eslint/no-unused-vars
({ ownerState, ...restProps }: { ownerState: object }, rootRef) => {
const Root = as

return <Root ref={rootRef} {...restProps} />
}
)
}

const titleCase = useTitleCase(propsTitleCase)
const finalChildren = [titleCase ? toTitleCase(children) : children]
/*
Workaround for the case: <Button as={Link} href='' /> (with empty href!), we have to determine "rootElementName" like below
Mui/base throws an error when "href" or "to" are empty
*/
const rootElementName =
as !== 'button' && ('href' in props || 'to' in props) ? 'a' : undefined
const finalRootElementName =
rootElementName ||
(as !== 'button' && ('href' in props || 'to' in props) ? 'a' : undefined)

if (icon) {
const iconComponent = getIcon({ icon })
Expand Down Expand Up @@ -145,8 +130,10 @@ export const ButtonBase: OverridableComponent<Props> = forwardRef<
data-component-type='button'
tabIndex={rest.tabIndex ?? disabled ? -1 : 0}
role={rest.role ?? 'button'}
rootElementName={rootElementName}
rootElementName={finalRootElementName}
slots={{ root: RootElement }}
// @ts-ignore
slotProps={{ root: { as } }}
>
<Container
as='span'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ const ButtonControlLabel = ({
variant='secondary'
size={size}
as='label'
rootElementName='label'
htmlFor={id}
disabled={disabled}
>
Expand Down
Loading