Skip to content

Commit

Permalink
[PopperUnstyled] Do not merge internal ownerState with ownerState
Browse files Browse the repository at this point in the history
… from props (#36599)
  • Loading branch information
hbjORbj authored Apr 10, 2023
1 parent 1f66dbe commit 41889f4
Show file tree
Hide file tree
Showing 24 changed files with 248 additions and 117 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ Sidebar.propTypes = {
description: PropTypes.string.isRequired,
social: PropTypes.arrayOf(
PropTypes.shape({
icon: PropTypes.elementType.isRequired,
icon: PropTypes.elementType,
name: PropTypes.string.isRequired,
}),
).isRequired,
Expand Down
4 changes: 4 additions & 0 deletions docs/pages/joy-ui/api/menu.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@
},
"default": "'md'"
},
"slots": {
"type": { "name": "shape", "description": "{ root?: elementType }" },
"default": "{}"
},
"sx": {
"type": {
"name": "union",
Expand Down
1 change: 1 addition & 0 deletions docs/pages/joy-ui/api/tooltip.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
},
"default": "'neutral'"
},
"component": { "type": { "name": "elementType" } },
"describeChild": { "type": { "name": "bool" }, "default": "false" },
"direction": {
"type": { "name": "enum", "description": "'ltr'<br>&#124;&nbsp;'rtl'" },
Expand Down
1 change: 1 addition & 0 deletions docs/translations/api-docs-joy/menu/menu.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
"onClose": "Triggered when focus leaves the menu and the menu should close.",
"open": "Controls whether the menu is displayed.",
"size": "The size of the component (affect other nested list* components because the <code>Menu</code> inherits <code>List</code>). To learn how to add custom sizes to the component, check out <a href=\"/joy-ui/customization/themed-components/#extend-sizes\">Themed components—Extend sizes</a>.",
"slots": "The components used for each slot inside the Popper. Either a string to use a HTML element or a component. See <a href=\"#slots\">Slots API</a> below for more details.",
"sx": "The system prop that allows defining system overrides as well as additional CSS styles. See the <a href=\"/system/getting-started/the-sx-prop/\">`sx` page</a> for more details.",
"variant": "The <a href=\"https://mui.com/joy-ui/main-features/global-variants/\">global variant</a> to use. To learn how to add your own variants, check out <a href=\"/joy-ui/customization/themed-components/#extend-variants\">Themed components—Extend variants</a>."
},
Expand Down
1 change: 1 addition & 0 deletions docs/translations/api-docs-joy/tooltip/tooltip.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"arrow": "If <code>true</code>, adds an arrow to the tooltip.",
"children": "Tooltip reference element.",
"color": "The color of the component. It supports those theme colors that make sense for this component. To learn how to add your own colors, check out <a href=\"/joy-ui/customization/themed-components/#extend-colors\">Themed components—Extend colors</a>.",
"component": "The component used for the root node. Either a string to use a HTML element or a component.",
"describeChild": "Set to <code>true</code> if the <code>title</code> acts as an accessible description. By default the <code>title</code> acts as an accessible label for the child.",
"direction": "Direction of the text.",
"disableFocusListener": "Do not respond to focus-visible events.",
Expand Down
5 changes: 3 additions & 2 deletions packages/mui-base/src/MenuUnstyled/MenuUnstyled.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import composeClasses from '../composeClasses';
import PopperUnstyled from '../PopperUnstyled';
import useSlotProps from '../utils/useSlotProps';
import { useClassNamesOverride } from '../utils/ClassNameConfigurator';
import { WithOptionalOwnerState } from '../utils';

function useUtilityClasses(ownerState: MenuUnstyledOwnerState) {
const { open } = ownerState;
Expand Down Expand Up @@ -75,7 +76,7 @@ const MenuUnstyled = React.forwardRef(function MenuUnstyled<
const classes = useUtilityClasses(ownerState);

const Root = component ?? slots.root ?? PopperUnstyled;
const rootProps: MenuUnstyledRootSlotProps = useSlotProps({
const rootProps: WithOptionalOwnerState<MenuUnstyledRootSlotProps> = useSlotProps({
elementType: Root,
externalForwardedProps: other,
externalSlotProps: slotProps.root,
Expand All @@ -88,7 +89,7 @@ const MenuUnstyled = React.forwardRef(function MenuUnstyled<
},
className: classes.root,
ownerState,
}) as MenuUnstyledRootSlotProps;
});

const Listbox = slots.listbox ?? 'ul';
const listboxProps = useSlotProps({
Expand Down
8 changes: 4 additions & 4 deletions packages/mui-base/src/PopperUnstyled/PopperUnstyled.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,19 @@ describe('<PopperUnstyled />', () => {
},
}));

it('should pass ownerState to overridable component', () => {
it('should not pass ownerState to overridable component', () => {
const CustomComponent = React.forwardRef<HTMLDivElement, any>(({ ownerState }, ref) => (
<div ref={ref} data-testid={ownerState.foo} />
<div ref={ref} data-testid="foo" id={ownerState.id} />
));
render(
<PopperUnstyled
anchorEl={() => document.createElement('div')}
open
component={CustomComponent}
ownerState={{ foo: 'foo' }}
ownerState={{ id: 'id' }}
/>,
);

expect(screen.getByTestId('foo')).toBeVisible();
expect(screen.getByTestId('foo')).to.not.have.attribute('id', 'id');
});
});
10 changes: 4 additions & 6 deletions packages/mui-base/src/PopperUnstyled/PopperUnstyled.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -87,13 +87,14 @@ const PopperTooltip = React.forwardRef(function PopperTooltip(
disablePortal,
modifiers,
open,
ownerState,
placement: initialPlacement,
popperOptions,
popperRef: popperRefProp,
slotProps = {},
slots = {},
TransitionProps,
// @ts-ignore internal logic
ownerState: ownerStateProp, // prevent from spreading to DOM, it can come from the parent component e.g. SelectUnstyled.
...other
} = props;

Expand Down Expand Up @@ -217,6 +218,7 @@ const PopperTooltip = React.forwardRef(function PopperTooltip(

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

const rootProps: WithOptionalOwnerState<PopperUnstyledRootSlotProps> = useSlotProps({
elementType: Root,
externalSlotProps: slotProps.root,
Expand All @@ -225,11 +227,7 @@ const PopperTooltip = React.forwardRef(function PopperTooltip(
role: 'tooltip',
ref: ownRef,
},
ownerState: {
// shallow merge ownerState from external component, e.g. Joy Menu.
...props,
...ownerState,
},
ownerState: props,
className: classes.root,
});

Expand Down
4 changes: 1 addition & 3 deletions packages/mui-base/src/PopperUnstyled/PopperUnstyled.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,6 @@ export interface PopperUnstyledOwnProps {
* @default false
*/
transition?: boolean;

ownerState?: any;
}

export interface PopperUnstyledSlots {
Expand All @@ -118,7 +116,7 @@ export interface PopperUnstyledSlots {
root?: React.ElementType;
}

export type PopperUnstyledOwnerState = Omit<PopperUnstyledOwnProps, 'ownerState'>;
export type PopperUnstyledOwnerState = PopperUnstyledOwnProps;

export interface PopperUnstyledTypeMap<P = {}, D extends React.ElementType = 'div'> {
props: P & PopperUnstyledOwnProps;
Expand Down
3 changes: 2 additions & 1 deletion packages/mui-base/src/SelectUnstyled/SelectUnstyled.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
SelectUnstyledRootSlotProps,
SelectUnstyledPopperSlotProps,
PopperUnstyled,
WithOptionalOwnerState,
} from '@mui/base';

const SelectUnstyledSlotPropsOverridesTest = (
Expand Down Expand Up @@ -35,7 +36,7 @@ function CustomRoot<TValue extends {}, Multiple extends boolean>(
}

function CustomPopper<TValue extends {}, Multiple extends boolean>(
props: SelectUnstyledPopperSlotProps<TValue, Multiple>,
props: WithOptionalOwnerState<SelectUnstyledPopperSlotProps<TValue, Multiple>>,
) {
const { ownerState, ...other } = props;
return <PopperUnstyled {...other} />;
Expand Down
27 changes: 14 additions & 13 deletions packages/mui-base/src/SelectUnstyled/SelectUnstyled.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -239,19 +239,20 @@ const SelectUnstyled = React.forwardRef(function SelectUnstyled<
className: classes.listbox,
});

const popperProps: SelectUnstyledPopperSlotProps<TValue, Multiple> = useSlotProps({
elementType: Popper,
externalSlotProps: slotProps.popper,
additionalProps: {
anchorEl: buttonRef.current,
disablePortal: true,
open: listboxOpen,
placement: 'bottom-start' as const,
role: undefined,
},
ownerState,
className: classes.popper,
});
const popperProps: WithOptionalOwnerState<SelectUnstyledPopperSlotProps<TValue, Multiple>> =
useSlotProps({
elementType: Popper,
externalSlotProps: slotProps.popper,
additionalProps: {
anchorEl: buttonRef.current,
disablePortal: true,
open: listboxOpen,
placement: 'bottom-start' as const,
role: undefined,
},
ownerState,
className: classes.popper,
});

return (
<React.Fragment>
Expand Down
6 changes: 4 additions & 2 deletions packages/mui-base/src/SelectUnstyled/SelectUnstyled.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
UseSelectListboxSlotProps,
} from '../useSelect';
import PopperUnstyled, { PopperUnstyledProps } from '../PopperUnstyled';
import { SlotComponentProps } from '../utils';
import { SlotComponentProps, WithOptionalOwnerState } from '../utils';

export interface SelectUnstyledRootSlotPropsOverrides {}
export interface SelectUnstyledListboxSlotPropsOverrides {}
Expand Down Expand Up @@ -137,7 +137,9 @@ export interface SelectUnstyledSlots<TValue extends {}, Multiple extends boolean
* The component that renders the popper.
* @default PopperUnstyled
*/
popper?: React.ComponentType<SelectUnstyledPopperSlotProps<TValue, Multiple>>;
popper?: React.ComponentType<
WithOptionalOwnerState<SelectUnstyledPopperSlotProps<TValue, Multiple>>
>;
}

export interface SelectUnstyledTypeMap<
Expand Down
33 changes: 30 additions & 3 deletions packages/mui-joy/src/Autocomplete/Autocomplete.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,10 @@ import Autocomplete, {
autocompleteClasses as classes,
createFilterOptions,
} from '@mui/joy/Autocomplete';
import AutocompleteListbox from '@mui/joy/AutocompleteListbox';
import Chip, { chipClasses } from '@mui/joy/Chip';
import ChipDelete from '@mui/joy/ChipDelete';
import { ThemeProvider } from '@mui/joy/styles';
import { ThemeProvider, styled } from '@mui/joy/styles';

function checkHighlightIs(listbox: HTMLElement, expected: string | null) {
const focused = listbox.querySelector(`.${classes.focused}`);
Expand All @@ -37,7 +38,9 @@ function checkHighlightIs(listbox: HTMLElement, expected: string | null) {
describe('Joy <Autocomplete />', () => {
const { render } = createRenderer();

describeConformance(<Autocomplete options={[]} />, () => ({
const StyledInput = styled('input')({});

describeConformance(<Autocomplete options={['one', 'two']} defaultValue="one" open />, () => ({
classes,
inheritComponent: 'div',
render,
Expand All @@ -46,7 +49,31 @@ describe('Joy <Autocomplete />', () => {
muiName: 'JoyAutocomplete',
testDeepOverrides: { slotName: 'popupIndicator', slotClassName: classes.popupIndicator },
testVariantProps: { size: 'lg' },
skip: ['componentsProp', 'classesRoot'],
skip: [
'componentsProp',
'classesRoot',
// https://github.com/facebook/react/issues/11565
'reactTestRenderer',
],
slots: {
root: {
expectedClassName: classes.root,
},
input: {
testWithComponent: React.forwardRef<HTMLInputElement>((props, ref) => (
<StyledInput ref={ref} {...props} data-testid="custom" />
)),
testWithElement: null,
expectedClassName: classes.input,
},
listbox: {
testWithComponent: React.forwardRef<HTMLUListElement>((props, ref) => (
<AutocompleteListbox ref={ref} {...props} data-testid="custom" />
)),
testWithElement: null,
expectedClassName: classes.listbox,
},
},
}));

describeJoyColorInversion(<Autocomplete options={[]} open />, {
Expand Down
31 changes: 25 additions & 6 deletions packages/mui-joy/src/Autocomplete/Autocomplete.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import * as React from 'react';
import PropTypes from 'prop-types';
import clsx from 'clsx';
import { OverridableComponent } from '@mui/types';
import {
chainPropTypes,
integerPropType,
Expand All @@ -13,7 +12,7 @@ import useAutocomplete, {
AutocompleteGroupedOption,
UseAutocompleteProps,
} from '@mui/base/useAutocomplete';
import PopperUnstyled, { PopperUnstyledTypeMap } from '@mui/base/PopperUnstyled';
import PopperUnstyled from '@mui/base/PopperUnstyled';
import { useThemeProps } from '../styles';
import ClearIcon from '../internal/svg-icons/Close';
import ArrowDropDownIcon from '../internal/svg-icons/ArrowDropDown';
Expand Down Expand Up @@ -564,7 +563,7 @@ const Autocomplete = React.forwardRef(function Autocomplete(

const [SlotListbox, listboxProps] = useSlot('listbox', {
className: classes.listbox,
elementType: PopperUnstyled as OverridableComponent<PopperUnstyledTypeMap<{}, 'ul'>>,
elementType: AutocompleteListbox,
getSlotProps: getListboxProps,
externalForwardedProps: other,
ownerState,
Expand All @@ -583,9 +582,6 @@ const Autocomplete = React.forwardRef(function Autocomplete(
}
: {},
},
internalForwardedProps: {
component: AutocompleteListbox,
},
});

const [SlotLoading, loadingProps] = useSlot('loading', {
Expand Down Expand Up @@ -660,6 +656,7 @@ const Autocomplete = React.forwardRef(function Autocomplete(
});
};

// Wait for `listboxProps` because `slotProps.listbox` could be a function.
const modifiers = React.useMemo(
() => [
{
Expand All @@ -683,7 +680,12 @@ const Autocomplete = React.forwardRef(function Autocomplete(
listboxProps.className,
listboxProps.ownerState?.color === 'context' && autocompleteClasses.colorContext,
)}
// @ts-ignore internal logic (too complex to typed PopperUnstyledOwnProps to SlotListbox but this should be removed when we have `usePopper`)
modifiers={modifiers}
{...(!props.slots?.listbox && {
as: PopperUnstyled,
slots: { root: listboxProps.as || 'ul' },
})}
>
{groupedOptions.map((option, index) => {
if (groupBy) {
Expand Down Expand Up @@ -1050,6 +1052,23 @@ Autocomplete.propTypes /* remove-proptypes */ = {
PropTypes.oneOf(['sm', 'md', 'lg']),
PropTypes.string,
]),
/**
* @ignore
*/
slots: PropTypes.shape({
clearIndicator: PropTypes.elementType,
endDecorator: PropTypes.elementType,
input: PropTypes.elementType,
limitTag: PropTypes.elementType,
listbox: PropTypes.elementType,
loading: PropTypes.elementType,
noOptions: PropTypes.elementType,
option: PropTypes.elementType,
popupIndicator: PropTypes.elementType,
root: PropTypes.elementType,
startDecorator: PropTypes.elementType,
wrapper: PropTypes.elementType,
}),
/**
* Leading adornment for this input.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,13 @@ describe('<CircularProgress />', () => {
testCustomVariant: true,
slots: {
root: { expectedClassName: classes.root },
svg: { expectedClassName: classes.svg, testWithElement: 'svg' },
svg: {
expectedClassName: classes.svg,
testWithComponent: ({ className }: any) => (
<svg className={className} data-testid="custom" />
),
testWithElement: null,
},
track: {
expectedClassName: classes.track,
},
Expand Down
5 changes: 5 additions & 0 deletions packages/mui-joy/src/Menu/Menu.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ describe('Joy <Menu />', () => {
},
);

it('should render with `ul` by default', () => {
render(<Menu anchorEl={document.createElement('div')} open data-testid="popover" />);
expect(screen.getByTestId('popover')).to.have.tagName('ul');
});

it('should pass onClose prop to Popover', () => {
const handleClose = spy();
render(
Expand Down
Loading

0 comments on commit 41889f4

Please sign in to comment.