Skip to content
Open
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
12 changes: 12 additions & 0 deletions docs/migration/Button-diff.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"component": "Button",
"removed": [
{
"prop": "classes",
"reason": "Type-level removal via Omit<StandardProps, 'classes'> on both Button.Props and ButtonBase.Props. Pre-migration the prop was vestigial — inherited via StandardProps/JssProps but never extracted by either component, so it leaked through {...rest} to @mui/base/Button (no public classes prop on MUIButtonBase), which dropped it as an unknown HTML attribute (React dev warning + silent prod drop). Post-migration the leak path was identical against @base-ui/react/button. Reviewer asked us to make the API honest about not using it. Zero internal call-sites pass classes={...} to picasso Button (verified by grep across packages/**). External 23-repo portfolio not audited — if any consumer types classes={...}, they will see a TS error and need to remove the prop.",
"codemod": "required"
}
],
"renamed": [],
"narrowed": []
}
4 changes: 2 additions & 2 deletions packages/base/Button/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
"@toptal/picasso-utils": "4.0.0",
"@toptal/picasso-link": "4.0.0",
"ap-style-title-case": "^1.1.2",
"@mui/base": "5.0.0-beta.58",
"@base-ui/react": "^1.4.1",
"classnames": "^2.5.1"
},
"sideEffects": [
Expand All @@ -43,7 +43,7 @@
"@toptal/picasso-tailwind-merge": "^2.0.0",
"@toptal/picasso-tailwind": ">=2.7",
"@toptal/picasso-provider": "*",
"react": ">=16.12.0 < 19.0.0"
"react": ">=16.12.0"
},
"exports": {
".": "./dist-package/src/index.js"
Expand Down
2 changes: 1 addition & 1 deletion packages/base/Button/src/Button/Button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export type VariantType =
export type IconPositionType = 'left' | 'right'

export interface Props
extends StandardProps,
extends Omit<StandardProps, 'classes'>,
TextLabelProps,
ButtonOrAnchorProps {
/** Show button in the active state (left mouse button down) */
Expand Down
6 changes: 4 additions & 2 deletions packages/base/Button/src/Button/__snapshots__/test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@ exports[`Button disabled button as link renders disabled version 1`] = `
>
<a
aria-disabled="true"
class="font-inherit focus:outline-hidden base-Button base- text-lg inline-flex items-center justify-center select-none appearance-none m-0 relative normal-case align-middle transition-colors duration-350 ease-out shrink-0 outline-hidden [[data-component-type="button"]+&]:ml cursor-default pointer-events no-underline hover:no-underline rounded-sm shadow-none border-none text-white visited:text-white bg-gray min-w h-8 py-0 px-4"
class="font-inherit focus:outline-hidden base-Button text-lg inline-flex items-center justify-center select-none appearance-none m-0 relative normal-case align-middle transition-colors duration-350 ease-out shrink-0 outline-hidden [[data-component-type="button"]+&]:ml cursor-default pointer-events no-underline hover:no-underline rounded-sm shadow-none border-none text-white visited:text-white bg-gray min-w h-8 py-0 px-4"
data-component-type="button"
data-disabled=""
href="URL"
role="button"
tabindex="-1"
Expand All @@ -31,8 +32,9 @@ exports[`Button disabled button renders disabled version 1`] = `
>
<button
aria-disabled="true"
class="base-Button base- text-lg inline-flex items-center justify-center select-none appearance-none m-0 relative normal-case align-middle transition-colors duration-350 ease-out shrink-0 outline-hidden [[data-component-type="button"]+&]:ml cursor-default pointer-events no-underline hover:no-underline rounded-sm shadow-none border-none text-white visited:text-white bg-gray min-w h-8 py-0 px-4"
class="base-Button text-lg inline-flex items-center justify-center select-none appearance-none m-0 relative normal-case align-middle transition-colors duration-350 ease-out shrink-0 outline-hidden [[data-component-type="button"]+&]:ml cursor-default pointer-events no-underline hover:no-underline rounded-sm shadow-none border-none text-white visited:text-white bg-gray min-w h-8 py-0 px-4"
data-component-type="button"
data-disabled=""
disabled=""
role="button"
tabindex="-1"
Expand Down
44 changes: 20 additions & 24 deletions packages/base/Button/src/ButtonBase/ButtonBase.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ import type {
TextLabelProps,
} 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 { Button as BaseUIButton } from '@base-ui/react/button'
import { Loader } from '@toptal/picasso-loader'
import { Container } from '@toptal/picasso-container'
import { noop, toTitleCase } from '@toptal/picasso-utils'
Expand All @@ -20,7 +19,7 @@ import { createCoreClassNames } from './styles'
export type IconPositionType = 'left' | 'right'

export interface Props
extends StandardProps,
extends Omit<StandardProps, 'classes'>,
TextLabelProps,
ButtonOrAnchorProps {
as?: ElementType
Expand All @@ -46,8 +45,11 @@ export interface Props
type?: 'button' | 'reset' | 'submit'
}

const getClickHandler = (loading?: boolean, handler?: Props['onClick']) =>
loading ? noop : handler
const getClickHandler = (
loading?: boolean,
handler?: Props['onClick']
): BaseUIButton.Props['onClick'] =>
(loading ? noop : handler) as BaseUIButton.Props['onClick']

const getIcon = ({ icon }: { icon?: ReactElement }) => {
if (!icon) {
Expand All @@ -60,15 +62,6 @@ const getIcon = ({ icon }: { icon?: ReactElement }) => {
})
}

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,
Props
Expand Down Expand Up @@ -98,7 +91,8 @@ export const ButtonBase: OverridableComponent<Props> = forwardRef<

const titleCase = useTitleCase(propsTitleCase)
const finalChildren = [titleCase ? toTitleCase(children) : children]
const finalRootElementName = typeof as === 'string' ? as : 'a'
Comment thread
vedrani marked this conversation as resolved.
const finalAs: ElementType = as ?? 'button'
const isNativeButton = finalAs === 'button'

if (icon) {
const iconComponent = getIcon({ icon })
Expand All @@ -110,12 +104,18 @@ export const ButtonBase: OverridableComponent<Props> = forwardRef<
}
}

const finalClassName = twMerge(createCoreClassNames({ disabled }), className)
const finalClassName = twMerge(
'base-Button-root',
createCoreClassNames({ disabled }),
className
)

return (
<MUIButtonBase
<BaseUIButton
{...rest}
ref={ref}
ref={ref as React.Ref<HTMLElement>}
nativeButton={isNativeButton}
render={isNativeButton ? undefined : React.createElement(finalAs)}
onClick={getClickHandler(loading, onClick)}
className={finalClassName}
style={style}
Expand All @@ -125,12 +125,8 @@ export const ButtonBase: OverridableComponent<Props> = forwardRef<
value={value}
type={type}
data-component-type='button'
tabIndex={rest.tabIndex ?? disabled ? -1 : 0}
tabIndex={rest.tabIndex ?? (disabled ? -1 : 0)}
role={rest.role ?? 'button'}
rootElementName={finalRootElementName as keyof HTMLElementTagNameMap}
slots={{ root: RootElement }}
// @ts-ignore
slotProps={{ root: { as } }}
>
<Container
as='span'
Expand All @@ -151,7 +147,7 @@ export const ButtonBase: OverridableComponent<Props> = forwardRef<
size='small'
/>
)}
</MUIButtonBase>
</BaseUIButton>
)
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,9 @@ exports[`ButtonBase when "as" prop is passed when "as" prop equals "Link" compon
>
<a
aria-disabled="true"
class="font-inherit focus:outline-hidden hover:underline text-blue visited:text-purple no-underline base-Button base- text-lg inline-flex items-center justify-center select-none appearance-none m-0 relative normal-case align-middle transition-colors duration-350 ease-out shrink-0 outline-hidden [[data-component-type="button"]+&]:ml cursor-default pointer-events"
class="font-inherit focus:outline-hidden hover:underline text-blue visited:text-purple no-underline base-Button text-lg inline-flex items-center justify-center select-none appearance-none m-0 relative normal-case align-middle transition-colors duration-350 ease-out shrink-0 outline-hidden [[data-component-type="button"]+&]:ml cursor-default pointer-events"
data-component-type="button"
data-disabled=""
href="URL"
role="button"
tabindex="-1"
Expand Down Expand Up @@ -174,8 +175,9 @@ exports[`ButtonBase when "disabled" prop is true renders Button and does not tri
>
<button
aria-disabled="true"
class="base-Button base- text-lg inline-flex items-center justify-center select-none appearance-none m-0 relative normal-case align-middle transition-colors duration-350 ease-out shrink-0 outline-hidden [[data-component-type="button"]+&]:ml cursor-default pointer-events"
class="base-Button text-lg inline-flex items-center justify-center select-none appearance-none m-0 relative normal-case align-middle transition-colors duration-350 ease-out shrink-0 outline-hidden [[data-component-type="button"]+&]:ml cursor-default pointer-events"
data-component-type="button"
data-disabled=""
disabled=""
role="button"
tabindex="-1"
Expand Down
Loading
Loading