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

[Popper][base] Drop component prop #37084

Merged
merged 15 commits into from
May 2, 2023
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
6 changes: 5 additions & 1 deletion docs/scripts/formattedTSDemos.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@
* List of demos or folders to ignore when transpiling
* Example: "app-bar/BottomAppBar.tsx"
*/
const ignoreList = ['/pages.ts', 'docs/data/joy/getting-started/templates'];
const ignoreList = [
'/pages.ts',
'docs/data/joy/getting-started/templates',
'docs/data/base/components/select/UnstyledSelectIntroduction.tsx',
Copy link
Member Author

Choose a reason for hiding this comment

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

This demo will create PropTypes and slotProps.popper gets wild... here's the snapshot... hence this change for now.. @michaldudak
Screenshot 2023-05-01 at 14 47 48

Copy link
Member

Choose a reason for hiding this comment

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

Let's create an issue for this prop, would be great to know what's causing it.

];

const fse = require('fs-extra');
const path = require('path');
Expand Down
1 change: 1 addition & 0 deletions docs/translations/api-docs/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.",
"components": "The components used for each slot inside the Popper. Either a string to use a HTML element or a component.",
"componentsProps": "The props used for each slot inside the Popper.",
"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.",
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 @@ -146,7 +146,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
8 changes: 3 additions & 5 deletions packages/mui-base/src/Menu/Menu.types.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as React from 'react';
import { OverrideProps, Simplify } from '@mui/types';
import { Simplify } from '@mui/types';
import Popper, { PopperProps } from '../Popper';
import { SlotComponentProps } from '../utils';
import { PolymorphicProps, SlotComponentProps } from '../utils';
import { UseMenuListboxSlotProps } from '../useMenu';
import { ListAction } from '../useList';

Expand Down Expand Up @@ -76,9 +76,7 @@ export interface MenuTypeMap<

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

export type MenuOwnerState = Simplify<
MenuOwnProps & {
Expand Down
15 changes: 10 additions & 5 deletions packages/mui-base/src/Popper/Popper.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,21 +26,26 @@ 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
<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
4 changes: 2 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 @@ -37,8 +38,7 @@ describe('<Popper />', () => {
<Popper
anchorEl={() => document.createElement('div')}
open
component={CustomComponent}
ownerState={{ id: 'id' }}
slots={{ root: CustomComponent }}
Copy link
Member

Choose a reason for hiding this comment

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

I am curious, why was the ownerState removed? Should it be moved to the slotProps? I am not saying to add it back, since it's clearly not needed, but I was wondering if there was another reason

Copy link
Member Author

Choose a reason for hiding this comment

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

Because external ownerState isn't supported in Base Popper any more. Yes, I think when I removed it, I should've done either slotProps={{id}} or just 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>;

/**
* 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 @@ -330,7 +330,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-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
4 changes: 2 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,7 @@ describe('useSlot', () => {
anchorEl: () => document.createElement('div'),
},
internalForwardedProps: {
component: ItemRoot,
slots: { root: ItemRoot },
},
});
return <SlotRoot {...rootProps} />;
Expand Down Expand Up @@ -237,7 +237,7 @@ describe('useSlot', () => {
anchorEl: () => document.createElement('div'),
},
internalForwardedProps: {
component: ItemListbox,
slots: { root: ItemListbox },
},
});
const [SlotOption, optionProps] = useSlot('option', {
Expand Down
12 changes: 9 additions & 3 deletions packages/mui-material/src/Popper/Popper.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ import * as React from 'react';
import { styled, Theme, useThemeProps } from '../styles';

export type PopperProps = Omit<BasePopperProps, 'direction'> & {
/**
* The component used for the root node.
* Either a string to use a HTML element or a component.
*/
component?: React.ElementType;
/**
* The components used for each slot inside the Popper.
* Either a string to use a HTML element or a component.
Expand Down Expand Up @@ -75,7 +80,6 @@ const Popper = React.forwardRef(function Popper(
const RootComponent = slots?.root ?? components?.Root;
const otherProps = {
anchorEl,
component,
container,
disablePortal,
keepMounted,
Expand All @@ -89,6 +93,7 @@ const Popper = React.forwardRef(function Popper(
};
return (
<PopperRoot
as={component}
direction={theme?.direction}
slots={{ root: RootComponent }}
slotProps={slotProps ?? componentsProps}
Expand Down Expand Up @@ -122,9 +127,10 @@ Popper.propTypes /* remove-proptypes */ = {
PropTypes.func,
]),
/**
* @ignore
* 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 components used for each slot inside the Popper.
* Either a string to use a HTML element or a component.
Expand Down