Skip to content

[Popper][base] Drop component prop #37060

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

Closed
wants to merge 12 commits into from
Closed
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
24 changes: 24 additions & 0 deletions docs/data/base/components/popper/popper.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,30 @@ By default, clicking outside the popper does not hide it.
If you need this behavior, you can use the [Click-Away Listener](/base/react-click-away-listener/) component.
:::

### Custom structure

```jsx
<Popper slots={{ root: 'span' }} />
```

:::info
The `slots` prop is available on all non-utility Base components.
See [Overriding component structure](/base/guides/overriding-component-structure/) for full details.

#### Usage with TypeScript

In TypeScript, you can specify the custom component type used in the `slots.root` as a generic to the unstyled component. This way, you can safely provide the custom component's props directly on the component:

```tsx
<Popper<typeof CustomComponent> slots={{ root: CustomComponent }} customProp />
```

The same applies for props specific to custom primitive elements:

```tsx
<Popper<'button'> slots={{ root: 'button' }} onClick={() => {}} />
```

## Customization

### Placement
Expand Down
1 change: 1 addition & 0 deletions docs/pages/material-ui/api/popper.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
}
},
"children": { "type": { "name": "union", "description": "node<br>&#124;&nbsp;func" } },
"component": { "type": { "name": "elementType" } },
"components": {
"type": { "name": "shape", "description": "{ Root?: elementType }" },
"default": "{}"
Expand Down
1 change: 1 addition & 0 deletions docs/translations/api-docs-base/popper/popper.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"propDescriptions": {
"anchorEl": "An HTML element, <a href=\"https://popper.js.org/docs/v2/virtual-elements/\">virtualElement</a>, or a function that returns either. It&#39;s used to set the position of the popper. The return value will passed as the reference object of the Popper instance.",
"children": "Popper render function or node.",
"component": "The component used for the root node. Either a string to use a HTML element or a component.",
"container": "An HTML element or function that returns one. The <code>container</code> will have the portal children appended to it.<br>By default, it uses the body of the top-level document object, so it&#39;s simply <code>document.body</code> most of the time.",
"direction": "Direction of the text.",
"disablePortal": "The <code>children</code> will be under the DOM hierarchy of the parent component.",
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-base/src/Menu/Menu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ Menu.propTypes /* remove-proptypes */ = {
* The props used for each slot inside the Menu.
* @default {}
*/
slotProps: PropTypes.shape({
slotProps: PropTypes /* @typescript-to-proptypes-ignore */.shape({
listbox: PropTypes.oneOfType([PropTypes.func, PropTypes.object]),
root: PropTypes.oneOfType([PropTypes.func, PropTypes.object]),
}),
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-base/src/Menu/Menu.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export interface MenuOwnProps {
* @default {}
*/
slotProps?: {
root?: SlotComponentProps<typeof Popper, MenuRootSlotPropsOverrides, MenuOwnerState>;
root?: SlotComponentProps<React.FC<PopperProps>, MenuRootSlotPropsOverrides, MenuOwnerState>;
listbox?: SlotComponentProps<'ul', MenuListboxSlotPropsOverrides, MenuOwnerState>;
};
/**
Expand Down
34 changes: 28 additions & 6 deletions packages/mui-base/src/Popper/Popper.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,21 +26,43 @@ const polymorphicComponentTest = () => {
{/* @ts-expect-error */}
<Popper invalidProp={0} open />

<Popper open component="a" href="#" />
<Popper<'a'>
open
slots={{
root: 'a',
}}
href="#"
/>

<Popper open component={CustomComponent} stringProp="test" numberProp={0} />
<Popper<typeof CustomComponent>
open
slots={{
root: CustomComponent,
}}
stringProp="test"
numberProp={0}
/>
{/* @ts-expect-error */}
<Popper open component={CustomComponent} />
<Popper<typeof CustomComponent>
open
slots={{
root: CustomComponent,
}}
/>

<Popper
<Popper<'button'>
open
component="button"
slots={{
root: 'button',
}}
onClick={(e: React.MouseEvent<HTMLButtonElement>) => e.currentTarget.checkValidity()}
/>

<Popper<'button'>
open
component="button"
slots={{
root: 'button',
}}
ref={(elem) => {
expectType<HTMLButtonElement | null, typeof elem>(elem);
}}
Expand Down
7 changes: 5 additions & 2 deletions packages/mui-base/src/Popper/Popper.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ describe('<Popper />', () => {
skip: [
// https://github.com/facebook/react/issues/11565
'reactTestRenderer',
'componentProp',
],
slots: {
root: {
Expand All @@ -34,10 +35,12 @@ describe('<Popper />', () => {
<div ref={ref} data-testid="foo" id={ownerState.id} />
));
render(
<Popper
<Popper<typeof CustomComponent>
anchorEl={() => document.createElement('div')}
open
component={CustomComponent}
slots={{
root: CustomComponent,
}}
ownerState={{ id: 'id' }}
/>,
);
Expand Down
14 changes: 4 additions & 10 deletions packages/mui-base/src/Popper/Popper.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import * as React from 'react';
import { OverridableComponent } from '@mui/types';
import {
chainPropTypes,
HTMLElementType,
Expand All @@ -13,7 +12,7 @@ import PropTypes from 'prop-types';
import composeClasses from '../composeClasses';
import Portal from '../Portal';
import { getPopperUtilityClass } from './popperClasses';
import { useSlotProps, WithOptionalOwnerState } from '../utils';
import { PolymorphicComponent, useSlotProps, WithOptionalOwnerState } from '../utils';
import {
PopperPlacementType,
PopperTooltipProps,
Expand Down Expand Up @@ -81,7 +80,6 @@ const PopperTooltip = React.forwardRef(function PopperTooltip<
const {
anchorEl,
children,
component,
direction,
disablePortal,
modifiers,
Expand Down Expand Up @@ -216,7 +214,7 @@ const PopperTooltip = React.forwardRef(function PopperTooltip<
}

const classes = useUtilityClasses();
const Root = component ?? slots.root ?? 'div';
const Root = slots.root ?? 'div';

const rootProps: WithOptionalOwnerState<PopperRootSlotProps> = useSlotProps({
elementType: Root,
Expand All @@ -233,7 +231,7 @@ const PopperTooltip = React.forwardRef(function PopperTooltip<
return (
<Root {...rootProps}>{typeof children === 'function' ? children(childProps) : children}</Root>
);
}) as OverridableComponent<PopperTooltipTypeMap>;
}) as PolymorphicComponent<PopperTooltipTypeMap>;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@michaldudak this change seems to make yarn proptypes change the proptypes of Menu.tsx and Select.tsx (both use Popper as default Root component) like in the picture. Any idea what might causes this?
image

Copy link
Member

Choose a reason for hiding this comment

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

You can add /* @typescript-to-proptypes-ignore */ before running yarn proptypes to prevent that change

  container: PropTypes /* @typescript-to-proptypes-ignore */.oneOfType([
    HTMLElementType,
    PropTypes.func,
  ]),

Copy link
Contributor Author

@nicolas-ot nicolas-ot Apr 27, 2023

Choose a reason for hiding this comment

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

Thank you!

ps: would be cool if you can add this PR to #36679

Copy link
Member

Choose a reason for hiding this comment

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

In fact, ignore my comment above, that's not how we should handle it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

have you found another way to deal with the problem?


/**
* Poppers rely on the 3rd party library [Popper.js](https://popper.js.org/docs/v2/) for positioning.
Expand Down Expand Up @@ -335,7 +333,7 @@ const Popper = React.forwardRef(function Popper<RootComponentType extends React.
</PopperTooltip>
</Portal>
);
}) as OverridableComponent<PopperTypeMap>;
}) as PolymorphicComponent<PopperTypeMap>;

Popper.propTypes /* remove-proptypes */ = {
// ----------------------------- Warning --------------------------------
Expand Down Expand Up @@ -533,10 +531,6 @@ Popper.propTypes /* remove-proptypes */ = {
slots: PropTypes.shape({
root: PropTypes.elementType,
}),
/**
* @ignore
*/
style: PropTypes.object,
/**
* Help supporting a react-transition-group/Transition component.
* @default false
Expand Down
11 changes: 3 additions & 8 deletions packages/mui-base/src/Popper/Popper.types.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import * as React from 'react';
import { OverrideProps } from '@mui/types';
import { Instance, Options, OptionsGeneric, VirtualElement } from '@popperjs/core';
import { PortalProps } from '../Portal';
import { SlotComponentProps } from '../utils';
import { PolymorphicProps, SlotComponentProps } from '../utils';

export type PopperPlacementType = Options['placement'];

Expand Down Expand Up @@ -124,9 +123,7 @@ export interface PopperTypeMap<

export type PopperProps<
RootComponentType extends React.ElementType = PopperTypeMap['defaultComponent'],
> = OverrideProps<PopperTypeMap<{}, RootComponentType>, RootComponentType> & {
component?: RootComponentType;
};
> = PolymorphicProps<PopperTypeMap<{}, RootComponentType>, RootComponentType>;

export type PopperTooltipOwnProps = Omit<
PopperOwnProps,
Expand All @@ -145,9 +142,7 @@ export interface PopperTooltipTypeMap<

export type PopperTooltipProps<
RootComponentType extends React.ElementType = PopperTooltipTypeMap['defaultComponent'],
> = OverrideProps<PopperTooltipTypeMap<{}, RootComponentType>, RootComponentType> & {
component?: RootComponentType;
};
> = PolymorphicProps<PopperTooltipTypeMap<{}, RootComponentType>, RootComponentType>;

export interface PopperRootSlotProps {
className?: string;
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-base/src/Select/Select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ Select.propTypes /* remove-proptypes */ = {
* The props used for each slot inside the Input.
* @default {}
*/
slotProps: PropTypes.shape({
slotProps: PropTypes /* @typescript-to-proptypes-ignore */.shape({
listbox: PropTypes.oneOfType([PropTypes.func, PropTypes.object]),
popper: PropTypes.oneOfType([PropTypes.func, PropTypes.object]),
root: PropTypes.oneOfType([PropTypes.func, PropTypes.object]),
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-base/src/Select/Select.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ export interface SelectOwnProps<OptionValue extends {}, Multiple extends boolean
SelectOwnerState<OptionValue, Multiple>
>;
popper?: SlotComponentProps<
typeof Popper,
React.FC<PopperProps>,
SelectPopperSlotPropsOverrides,
SelectOwnerState<OptionValue, Multiple>
>;
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-joy/src/Menu/Menu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ Menu.propTypes /* remove-proptypes */ = {
* The component used for the root node.
* Either a string to use a HTML element or a component.
*/
component: PropTypes /* @typescript-to-proptypes-ignore */.elementType,
component: PropTypes.elementType,
/**
* The `children` will be under the DOM hierarchy of the parent component.
* @default false
Expand Down
5 changes: 5 additions & 0 deletions packages/mui-joy/src/Menu/MenuProps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ export interface MenuTypeMap<P = {}, D extends React.ElementType = 'ul'> {
* @default 'neutral'
*/
color?: OverridableStringUnion<ColorPaletteProp, MenuPropsColorOverrides>;
/**
* The component used for the root node.
* Either a string to use a HTML element or a component.
*/
component?: React.ElementType;
/**
* If `true`, the children with an implicit color prop invert their colors to match the component's variant and color.
* @default false
Expand Down
8 changes: 6 additions & 2 deletions packages/mui-joy/src/utils/useSlot.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,9 @@ describe('useSlot', () => {
anchorEl: () => document.createElement('div'),
},
internalForwardedProps: {
component: ItemRoot,
slots: {
root: ItemRoot,
},
},
});
return <SlotRoot {...rootProps} />;
Expand Down Expand Up @@ -237,7 +239,9 @@ describe('useSlot', () => {
anchorEl: () => document.createElement('div'),
},
internalForwardedProps: {
component: ItemListbox,
slots: {
root: ItemListbox,
Copy link
Member

Choose a reason for hiding this comment

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

why are you doing this change?

Copy link
Contributor Author

@nicolas-ot nicolas-ot Apr 28, 2023

Choose a reason for hiding this comment

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

that is a test that use base Popper as describeb here elementType: Popper as unknown as 'ul',. I believe internalForwardedProps is a prop that is forwarded to the Popper, hence the change.

},
},
});
const [SlotOption, optionProps] = useSlot('option', {
Expand Down
4 changes: 0 additions & 4 deletions packages/mui-material/src/Popper/Popper.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -121,10 +121,6 @@ Popper.propTypes /* remove-proptypes */ = {
PropTypes.node,
PropTypes.func,
]),
/**
* @ignore
*/
component: PropTypes /* @typescript-to-proptypes-ignore */.elementType,
/**
* The components used for each slot inside the Popper.
* Either a string to use a HTML element or a component.
Expand Down