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

[Button][base] Drop component prop #36677

Merged
merged 36 commits into from
Apr 26, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
c955845
[ButtonUnstyled] Drop `component` prop
mnajdova Mar 29, 2023
8e7ebda
docs:typescript:formatted
mnajdova Mar 29, 2023
615cfaf
Fix lint issues
mnajdova Mar 29, 2023
3b88d2d
Fix tests related to the changes of the conformance test suite
mnajdova Mar 29, 2023
42c3a54
Use React.ButtonHTMLAttributes
mnajdova Mar 29, 2023
abb7d98
remove ButtonUnstyledTypeMap usages
mnajdova Mar 29, 2023
7eecd8a
Merge branch 'master' into base/remove-component-prop
michaldudak Apr 14, 2023
aced6ee
Created a Base UI version of OverridableComponent
michaldudak Apr 17, 2023
b96cc6f
Merge remote-tracking branch 'upstream/master' into base/remove-compo…
michaldudak Apr 17, 2023
1aca46b
revert some test changes, cleanup code
mnajdova Apr 18, 2023
ee4fdb7
Rename the OverridableComponent to PolymorphicComponent
mnajdova Apr 18, 2023
f49956d
Fixed regressions in names
mnajdova Apr 18, 2023
c709d0c
update the spec tests
mnajdova Apr 18, 2023
d61f431
Fix lint issues
mnajdova Apr 19, 2023
d86107a
Update packages/mui-base/src/ButtonUnstyled/ButtonUnstyled.spec.tsx
mnajdova Apr 19, 2023
e0cc64d
Docs updates on the genric usage
mnajdova Apr 19, 2023
10cca14
update button and overriding component structure docs
samuelsycamore Apr 19, 2023
83aaa1e
Merge branch 'master' into base/remove-component-prop
mnajdova Apr 25, 2023
aa50cdc
Merge branch 'base/remove-component-prop' of https://github.com/mnajd…
mnajdova Apr 25, 2023
0c78312
prettier
mnajdova Apr 25, 2023
5c8d684
docs:api
mnajdova Apr 25, 2023
641bd26
fix some issues
mnajdova Apr 25, 2023
27e0da1
fix wrong path separators
mnajdova Apr 25, 2023
cb62291
One more fix
mnajdova Apr 25, 2023
4ebe89f
more changes
mnajdova Apr 25, 2023
41845e6
Remove occurrences of unstyled from button doc
hbjORbj Apr 25, 2023
814df7c
update button api docs
hbjORbj Apr 25, 2023
253450a
remove old files
mnajdova Apr 25, 2023
8c7a61e
Merge branch 'base/remove-component-prop' of https://github.com/mnajd…
mnajdova Apr 25, 2023
920a682
address comments
hbjORbj Apr 25, 2023
508a008
use single quote
hbjORbj Apr 25, 2023
2bb9245
prettier
hbjORbj Apr 25, 2023
fbfbd3c
remove unstyled from overriding component structure doc
hbjORbj Apr 26, 2023
9ce855e
add 1 more ts test
hbjORbj Apr 26, 2023
e089918
resolve merge conflict
hbjORbj Apr 26, 2023
b7d62fb
Merge branch 'master' into base/remove-component-prop
hbjORbj Apr 26, 2023
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
2 changes: 1 addition & 1 deletion docs/data/base/components/button/UnstyledButtonCustom.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ const CustomButtonRoot = styled(ButtonRoot)(
);

const SvgButton = React.forwardRef(function SvgButton(props, ref) {
return <ButtonUnstyled {...props} component={CustomButtonRoot} ref={ref} />;
return <ButtonUnstyled {...props} slots={{ root: CustomButtonRoot }} ref={ref} />;
});

export default function UnstyledButtonCustom() {
Expand Down
2 changes: 1 addition & 1 deletion docs/data/base/components/button/UnstyledButtonCustom.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ const SvgButton = React.forwardRef(function SvgButton(
props: ButtonUnstyledProps,
ref: React.ForwardedRef<any>,
) {
return <ButtonUnstyled {...props} component={CustomButtonRoot} ref={ref} />;
return <ButtonUnstyled {...props} slots={{ root: CustomButtonRoot }} ref={ref} />;
});

export default function UnstyledButtonCustom() {
Expand Down
4 changes: 2 additions & 2 deletions docs/data/base/components/button/UnstyledButtonsSpan.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ const CustomButton = styled(ButtonUnstyled)`
export default function UnstyledButtonsSpan() {
return (
<Stack spacing={2} direction="row">
<CustomButton component="span">Button</CustomButton>
<CustomButton component="span" disabled>
<CustomButton slots={{ root: 'span' }}>Button</CustomButton>
<CustomButton slots={{ root: 'span' }} disabled>
Disabled
</CustomButton>
</Stack>
Expand Down
4 changes: 2 additions & 2 deletions docs/data/base/components/button/UnstyledButtonsSpan.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ const CustomButton = styled(ButtonUnstyled)`
export default function UnstyledButtonsSpan() {
return (
<Stack spacing={2} direction="row">
<CustomButton component="span">Button</CustomButton>
<CustomButton component="span" disabled>
<CustomButton slots={{ root: 'span' }}>Button</CustomButton>
<CustomButton slots={{ root: 'span' }} disabled>
Disabled
</CustomButton>
</Stack>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<CustomButton component="span">Button</CustomButton>
<CustomButton component="span" disabled>
<CustomButton slots={{ root: 'span' }}>Button</CustomButton>
<CustomButton slots={{ root: 'span' }} disabled>
Disabled
</CustomButton>
8 changes: 4 additions & 4 deletions docs/data/base/components/button/button.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,10 @@ The following props are available on all non-utility Base components.
See [Usage](/base/getting-started/usage/) for full details.
:::

Use the `component` prop to override the root slot with a custom element:
Use the `slots.root` prop to override the root slot with a custom element:

```jsx
<ButtonUnstyled component="div" />
<ButtonUnstyled slots={{ root: 'div' }} />
```

If you provide a non-interactive element such as a `<span>`, the Unstyled Button component will automatically add the necessary accessibility attributes.
Expand All @@ -72,8 +72,8 @@ Compare the attributes on the `<span>` in this demo with the Button from the pre
{{"demo": "UnstyledButtonsSpan.js"}}

:::warning
If an Unstyled Button is customized with a non-button element (i.e. `<ButtonUnstyled component="span" />`), it will not submit the form it's in when clicked.
Similarly, `<ButtonUnstyled component="span" type="reset">` will not reset its parent form.
If an Unstyled Button is customized with a non-button element (i.e. `<ButtonUnstyled slots={{ root: "span" }} />`), it will not submit the form it's in when clicked.
Similarly, `<ButtonUnstyled slots={{ root: "span" }} type="reset">` will not reset its parent form.
:::

## Hook
Expand Down
1 change: 0 additions & 1 deletion docs/pages/base/api/button-unstyled.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
"description": "func<br>&#124;&nbsp;{ current?: { focusVisible: func } }"
}
},
"component": { "type": { "name": "elementType" } },
"disabled": { "type": { "name": "bool" }, "default": "false" },
"focusableWhenDisabled": { "type": { "name": "bool" }, "default": "false" },
"slotProps": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
"componentDescription": "The foundation for building custom-styled buttons.",
"propDescriptions": {
"action": "A ref for imperative actions. It currently only supports <code>focusVisible()</code> action.",
"component": "The component used for the root node. Either a string to use a HTML element or a component.",
"disabled": "If <code>true</code>, the component is disabled.",
"focusableWhenDisabled": "If <code>true</code>, allows a disabled button to receive focus.",
"slotProps": "The props used for each slot inside the Button.",
Expand Down
35 changes: 0 additions & 35 deletions packages/mui-base/src/ButtonUnstyled/ButtonUnstyled.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,38 +22,3 @@ const CustomButtonRoot = React.forwardRef(function CustomButtonRoot(
function ButtonWithCustomRoot(props: ButtonUnstyledProps) {
return <ButtonUnstyled {...props} slots={{ root: CustomButtonRoot }} />;
}

const polymorphicComponentTest = () => {
const CustomComponent: React.FC<{ stringProp: string; numberProp: number }> =
function CustomComponent() {
return <div />;
};

return (
<div>
{/* @ts-expect-error */}
<ButtonUnstyled invalidProp={0} />

<ButtonUnstyled component="a" href="#" />

<ButtonUnstyled component={CustomComponent} stringProp="test" numberProp={0} />
{/* @ts-expect-error */}
<ButtonUnstyled component={CustomComponent} />

<ButtonUnstyled
component="button"
onClick={(e: React.MouseEvent<HTMLButtonElement>) => e.currentTarget.checkValidity()}
/>

<ButtonUnstyled<'div'>
component="div"
ref={(elem) => {
expectType<HTMLDivElement | null, typeof elem>(elem);
}}
onClick={(e) => {
expectType<React.MouseEvent<HTMLDivElement, MouseEvent>, typeof e>(e);
}}
/>
</div>
);
};
21 changes: 13 additions & 8 deletions packages/mui-base/src/ButtonUnstyled/ButtonUnstyled.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ describe('<ButtonUnstyled />', () => {

describe('role attribute', () => {
it('is set when the root component is an HTML element other than a button', () => {
const { getByRole } = render(<ButtonUnstyled component="span" />);
const { getByRole } = render(<ButtonUnstyled slots={{ root: 'span' }} />);
expect(getByRole('button')).not.to.equal(null);
});

Expand All @@ -42,7 +42,7 @@ describe('<ButtonUnstyled />', () => {
) => <span role={props.role} ref={ref} />,
);

const { getByRole } = render(<ButtonUnstyled component={WrappedSpan} />);
const { getByRole } = render(<ButtonUnstyled slots={{ root: WrappedSpan }} />);
expect(getByRole('button')).not.to.equal(null);
});

Expand All @@ -54,7 +54,7 @@ describe('<ButtonUnstyled />', () => {
) => <button role={props.role} ref={ref} />,
);

const { getByRole } = render(<ButtonUnstyled component={WrappedButton} />);
const { getByRole } = render(<ButtonUnstyled slots={{ root: WrappedButton }} />);
expect(getByRole('button')).not.to.have.attribute('role');
});
});
Expand Down Expand Up @@ -104,7 +104,7 @@ describe('<ButtonUnstyled />', () => {
describe('as non-button element', () => {
it('can receive focus when focusableWhenDisabled is set', () => {
const { getByRole } = render(
<ButtonUnstyled component="span" focusableWhenDisabled disabled />,
<ButtonUnstyled slots={{ root: 'span' }} focusableWhenDisabled disabled />,
);

const button = getByRole('button');
Expand All @@ -117,7 +117,7 @@ describe('<ButtonUnstyled />', () => {

it('has aria-disabled and tabIndex attributes set', () => {
const { getByRole } = render(
<ButtonUnstyled component="span" focusableWhenDisabled disabled />,
<ButtonUnstyled slots={{ root: 'span' }} focusableWhenDisabled disabled />,
);

const button = getByRole('button');
Expand All @@ -129,7 +129,12 @@ describe('<ButtonUnstyled />', () => {
it('does not respond to user actions when disabled and focused', () => {
const handleClick = spy();
const { getByRole } = render(
<ButtonUnstyled component="span" focusableWhenDisabled disabled onClick={handleClick} />,
<ButtonUnstyled
slots={{ root: 'span' }}
focusableWhenDisabled
disabled
onClick={handleClick}
/>,
);

const button = getByRole('button');
Expand All @@ -155,7 +160,7 @@ describe('<ButtonUnstyled />', () => {
});

it('renders as the element provided in the "component" prop, even with a "href" prop', () => {
const { getByRole } = render(<ButtonUnstyled component="h1" href="#" />);
const { getByRole } = render(<ButtonUnstyled slots={{ root: 'h1' }} href="#" />);
expect(getByRole('heading')).not.to.equal(null);
});

Expand All @@ -172,7 +177,7 @@ describe('<ButtonUnstyled />', () => {
});

it('renders as the element provided in the "component" prop, even with a "to" prop', () => {
const { getByRole } = render(<ButtonUnstyled component="h1" to="#" />);
const { getByRole } = render(<ButtonUnstyled slots={{ root: 'h1' }} to="#" />);
expect(getByRole('heading')).not.to.equal(null);
});

Expand Down
19 changes: 6 additions & 13 deletions packages/mui-base/src/ButtonUnstyled/ButtonUnstyled.tsx
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if the ButtonUnstyled definition shouldn't be function ButtonUnstyled(props: ButtonUnstyledProps, forwardedRef: React.ForwardedRef<any>) (without the generics). This way, we get strongly typed props inside the component definition. Nothing would change with the exported definition as we cast it to PloymorphicComponent anyway.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the explicitness that we have now. We can open a separate issue for this and see what other things I guess. It's anyway internal change

Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
import * as React from 'react';
import PropTypes from 'prop-types';
import { OverridableComponent } from '@mui/types';
import composeClasses from '../composeClasses';
import { getButtonUnstyledUtilityClass } from './buttonUnstyledClasses';
import {
ButtonUnstyledProps,
ButtonUnstyledOwnProps,
ButtonUnstyledTypeMap,
ButtonUnstyledRootSlotProps,
} from './ButtonUnstyled.types';
import useButton from '../useButton';
Expand Down Expand Up @@ -39,13 +37,13 @@ const useUtilityClasses = (ownerState: ButtonUnstyledOwnerState) => {
*
* - [ButtonUnstyled API](https://mui.com/base/api/button-unstyled/)
*/
const ButtonUnstyled = React.forwardRef(function ButtonUnstyled<
BaseComponentType extends React.ElementType = ButtonUnstyledTypeMap['defaultComponent'],
>(props: ButtonUnstyledProps<BaseComponentType>, forwardedRef: React.ForwardedRef<any>) {
const ButtonUnstyled = React.forwardRef(function ButtonUnstyled(
props: ButtonUnstyledProps,
forwardedRef: React.ForwardedRef<any>,
) {
const {
action,
children,
component,
disabled,
focusableWhenDisabled = false,
onBlur,
Expand Down Expand Up @@ -88,7 +86,7 @@ const ButtonUnstyled = React.forwardRef(function ButtonUnstyled<
const classes = useUtilityClasses(ownerState);

const defaultElement = other.href || other.to ? 'a' : 'button';
const Root: React.ElementType = component ?? slots.root ?? defaultElement;
const Root: React.ElementType = slots.root ?? defaultElement;
const rootProps: WithOptionalOwnerState<ButtonUnstyledRootSlotProps> = useSlotProps({
elementType: Root,
getSlotProps: getRootProps,
Expand All @@ -102,7 +100,7 @@ const ButtonUnstyled = React.forwardRef(function ButtonUnstyled<
});

return <Root {...rootProps}>{children}</Root>;
}) as OverridableComponent<ButtonUnstyledTypeMap>;
});

ButtonUnstyled.propTypes /* remove-proptypes */ = {
// ----------------------------- Warning --------------------------------
Expand All @@ -124,11 +122,6 @@ ButtonUnstyled.propTypes /* remove-proptypes */ = {
* @ignore
*/
children: PropTypes.node,
/**
* The component used for the root node.
* Either a string to use a HTML element or a component.
*/
component: PropTypes.elementType,
/**
* If `true`, the component is disabled.
* @default false
Expand Down
8 changes: 2 additions & 6 deletions packages/mui-base/src/ButtonUnstyled/ButtonUnstyled.types.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as React from 'react';
import { OverrideProps, Simplify } from '@mui/types';
import { DefaultComponentProps, Simplify } from '@mui/types';
import { UseButtonParameters, UseButtonRootSlotProps } from '../useButton';
import { SlotComponentProps } from '../utils';

Expand Down Expand Up @@ -43,11 +43,7 @@ export interface ButtonUnstyledSlots {
root?: React.ElementType;
}

export type ButtonUnstyledProps<
D extends React.ElementType = ButtonUnstyledTypeMap['defaultComponent'],
> = OverrideProps<ButtonUnstyledTypeMap<{}, D>, D> & {
component?: D;
};
export type ButtonUnstyledProps = DefaultComponentProps<ButtonUnstyledTypeMap>;
mnajdova marked this conversation as resolved.
Show resolved Hide resolved

export interface ButtonUnstyledTypeMap<P = {}, D extends React.ElementType = 'button'> {
props: P & ButtonUnstyledOwnProps;
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-base/src/MenuUnstyled/MenuUnstyled.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ describe('<MenuUnstyled />', () => {
expectedClassName: menuUnstyledClasses.listbox,
},
},
skip: ['reactTestRenderer', 'propsSpread', 'componentProp', 'slotsProp'],
skip: ['reactTestRenderer', 'propsSpread', 'slotsProp'],
}));

describe('after initialization', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ describe('<TextareaAutosize />', () => {
slots: {},
skip: [
// doesn't have slots, so these tests are irrelevant:
'componentProp',
'mergeClassName',
'ownerStatePropagation',
'propsSpread',
Expand Down
34 changes: 1 addition & 33 deletions test/utils/describeConformanceUnstyled.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import {
describeRef,
randomStringValue,
testClassName,
testComponentProp,
testReactTestRenderer,
} from './describeConformance';

Expand Down Expand Up @@ -102,7 +101,7 @@ function testPropForwarding(
}

function testSlotsProp(element: React.ReactElement, getOptions: () => UnstyledConformanceOptions) {
const { render, slots, testComponentPropWith: Element = 'div' } = getOptions();
const { render, slots } = getOptions();

if (!render) {
throwMissingPropError('render');
Expand Down Expand Up @@ -163,36 +162,6 @@ function testSlotsProp(element: React.ReactElement, getOptions: () => UnstyledCo
});
}
});

it('uses the component provided in the `component` prop when both `component` and `slots.root` are provided', () => {
const RootComponentA = React.forwardRef(
({ children }: React.PropsWithChildren<{}>, ref: React.Ref<any>) => (
// @ts-ignore
<Element data-testid="a" ref={ref}>
{children}
</Element>
),
);

const RootComponentB = React.forwardRef(
({ children }: React.PropsWithChildren<{}>, ref: React.Ref<any>) => (
// @ts-ignore
<Element data-testid="b" ref={ref}>
{children}
</Element>
),
);

const { queryByTestId } = render(
React.cloneElement(element, {
component: RootComponentA,
slots: { root: RootComponentB },
}),
);

expect(queryByTestId('a')).not.to.equal(null);
expect(queryByTestId('b')).to.equal(null);
});
}

function testSlotPropsProp(
Expand Down Expand Up @@ -342,7 +311,6 @@ function testDisablingClassGeneration(
}

const fullSuite = {
componentProp: testComponentProp,
slotsProp: testSlotsProp,
slotPropsProp: testSlotPropsProp,
slotPropsCallbacks: testSlotPropsCallbacks,
Expand Down