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

[Joy] Refine componentsProps for all components #34077

Merged
merged 49 commits into from Aug 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
93b774e
update aspect ratio
siriwatknp Aug 25, 2022
a307f94
update Avatar
siriwatknp Aug 25, 2022
958c66f
remove imgProps from Avatar
siriwatknp Aug 25, 2022
83304e1
update avatar group
siriwatknp Aug 25, 2022
a13296a
fixes
siriwatknp Aug 25, 2022
077cd7a
update Badge
siriwatknp Aug 25, 2022
51bfb0f
update breadcrumbs
siriwatknp Aug 25, 2022
9c49714
update Button
siriwatknp Aug 25, 2022
04dc8a3
update card components ownerState types
siriwatknp Aug 25, 2022
43ec99d
update checkbox
siriwatknp Aug 25, 2022
3a4d7e0
update checkbox demos
siriwatknp Aug 25, 2022
3572b8c
update Chip
siriwatknp Aug 25, 2022
81090cd
update ChipDelete
siriwatknp Aug 26, 2022
c6ed861
update form helper text
siriwatknp Aug 26, 2022
5f8609c
update form label
siriwatknp Aug 26, 2022
8ffc2db
update icon button
siriwatknp Aug 26, 2022
da53110
update Input
siriwatknp Aug 26, 2022
2ba3c83
update Link
siriwatknp Aug 26, 2022
2990227
update list
siriwatknp Aug 26, 2022
fc9540d
update list divider
siriwatknp Aug 26, 2022
0ec152b
update list item
siriwatknp Aug 26, 2022
eb9d014
update list item button
siriwatknp Aug 26, 2022
3d9eb11
update list item content
siriwatknp Aug 26, 2022
184470b
update list item decorator
siriwatknp Aug 26, 2022
7ef2e66
update menu
siriwatknp Aug 26, 2022
360df35
update menu item
siriwatknp Aug 26, 2022
9976b37
update menu list
siriwatknp Aug 26, 2022
52431c0
update option
siriwatknp Aug 26, 2022
a321d09
update radio
siriwatknp Aug 26, 2022
7c7ad8d
update radio group
siriwatknp Aug 26, 2022
4e61245
update select
siriwatknp Aug 26, 2022
78a1a0c
update sheet
siriwatknp Aug 26, 2022
afca797
update slider
siriwatknp Aug 26, 2022
8f3b6d3
update switch
siriwatknp Aug 26, 2022
bfce007
update tab
siriwatknp Aug 26, 2022
1bee444
change to div
siriwatknp Aug 26, 2022
91e6f49
update tablist
siriwatknp Aug 26, 2022
44ecc8c
update tabpanel
siriwatknp Aug 26, 2022
95dc848
update tabs
siriwatknp Aug 26, 2022
da3e6e7
fix switch
siriwatknp Aug 26, 2022
6d905fd
update textarea
siriwatknp Aug 26, 2022
5b3fbe2
update TextField
siriwatknp Aug 26, 2022
f63738e
update typography
siriwatknp Aug 26, 2022
fe11b6a
update ownerState in components
siriwatknp Aug 26, 2022
86fafbf
revert change
siriwatknp Aug 27, 2022
bee2f42
update spec
siriwatknp Aug 27, 2022
42716c8
yarn proptypes
siriwatknp Aug 27, 2022
f0e837f
run proptypes
siriwatknp Aug 28, 2022
eb4d19e
add custom variant test
siriwatknp Aug 28, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 10 additions & 10 deletions docs/data/joy/components/checkbox/ExampleButtonCheckbox.js
@@ -1,5 +1,5 @@
import * as React from 'react';
import Checkbox, { checkboxClasses } from '@mui/joy/Checkbox';
import Checkbox from '@mui/joy/Checkbox';
import List from '@mui/joy/List';
import ListItem from '@mui/joy/ListItem';
import ListItemDecorator from '@mui/joy/ListItemDecorator';
Expand Down Expand Up @@ -53,16 +53,16 @@ export default function ExampleButtonCheckbox() {
setValue((val) => val.filter((text) => text !== item));
}
}}
sx={{
[`& .${checkboxClasses.action}`]: {
bgcolor: value.includes(item) ? 'background.surface' : 'transparent',
boxShadow: value.includes(item) ? 'sm' : 'none',
'&:hover': {
bgcolor: value.includes(item)
? 'background.surface'
: 'transparent',
componentsProps={{
action: ({ checked }) => ({
sx: {
bgcolor: checked ? 'background.surface' : 'transparent',
boxShadow: checked ? 'sm' : 'none',
'&:hover': {
bgcolor: checked ? 'background.surface' : 'transparent',
},
},
},
}),
}}
/>
</ListItem>
Expand Down
18 changes: 10 additions & 8 deletions docs/data/joy/components/checkbox/ExampleChoiceChipCheckbox.js
@@ -1,6 +1,6 @@
import * as React from 'react';
import Box from '@mui/joy/Box';
import Checkbox, { checkboxClasses } from '@mui/joy/Checkbox';
import Checkbox from '@mui/joy/Checkbox';
import List from '@mui/joy/List';
import ListItem from '@mui/joy/ListItem';
import Typography from '@mui/joy/Typography';
Expand Down Expand Up @@ -49,13 +49,15 @@ export default function ExampleChoiceChipCheckbox() {
setValue((val) => val.filter((text) => text !== item));
}
}}
sx={{
Copy link
Member

Choose a reason for hiding this comment

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

While improving the support for componentsProps in all the Joy UI components sounds great, for adding custom attributes, and events. I wonder if using it for customization over the CSS selectors is a step forward. Personally, I found the previous approach better. We teach for customizations that are not one-off to use the styled() API, which was easier to migrate to with the sx demo.

Another way to look at it, SASS is still the most popular way to style: https://2021.stateofcss.com/en-US/technologies/#scatterplot_overview. How would somebody using SASS would figure out how to migrate the customization?

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 think this is not up to Joy and I don't even think which one is better. It's all up to the developers to choose the way they prefer. I will ensure that both ways are documented.

Copy link
Member

Choose a reason for hiding this comment

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

It's all up to the developers to choose the way they prefer.

@siriwatknp It's an interesting perspective. I had tried this path in the past (my initial intuition of how it should be done) and stopped once I saw too many developers struggling with deciding which option they should use. I saw them ask for the documentation to give them "the way". For example, in our last developer survey: https://www.notion.so/mui-org/Raw-data-aa374141dcb3453dbfea301dcc437126#8830e74c200946dcb29ce385684f1dc4

10 | more opinionated
2 | less opinionated

Maybe it would make sense to first decide on which approach is better, and then stick to it everywhere in the docs 😁

[`&.${checkboxClasses.checked}`]: {
[`& .${checkboxClasses.action}`]: {
border: '1px solid',
borderColor: 'primary.500',
},
},
componentsProps={{
action: ({ checked }) => ({
sx: checked
? {
border: '1px solid',
borderColor: 'primary.500',
}
: {},
}),
}}
/>
</ListItem>
Expand Down
18 changes: 12 additions & 6 deletions docs/data/joy/components/checkbox/HoverCheckbox.js
@@ -1,17 +1,23 @@
import * as React from 'react';
import Checkbox, { checkboxClasses } from '@mui/joy/Checkbox';
import Checkbox from '@mui/joy/Checkbox';
import Done from '@mui/icons-material/Done';

export default function HoverCheckbox() {
return (
<Checkbox
uncheckedIcon={<Done />}
label="Label"
sx={{
[`&:not(.${checkboxClasses.checked})`]: {
'& svg': { opacity: 0 },
[`&:hover svg, &.${checkboxClasses.focusVisible} svg`]: { opacity: 1 },
},
componentsProps={{
root: ({ checked, focusVisible }) => ({
sx: !checked
? {
'& svg': { opacity: focusVisible ? 0.32 : 0 },
'&:hover svg': {
opacity: 0.32,
},
}
: {},
}),
}}
/>
);
Expand Down
1 change: 1 addition & 0 deletions packages/mui-joy/src/AspectRatio/AspectRatio.test.js
Expand Up @@ -17,6 +17,7 @@ describe('<AspectRatio />', () => {
refInstanceof: window.HTMLDivElement,
testComponentPropWith: 'span',
testVariantProps: { variant: 'solid' },
testCustomVariant: true,
skip: ['classesRoot', 'componentsProp'],
}));

Expand Down
55 changes: 30 additions & 25 deletions packages/mui-joy/src/AspectRatio/AspectRatio.tsx
@@ -1,15 +1,15 @@
import * as React from 'react';
import clsx from 'clsx';
import PropTypes from 'prop-types';
import { unstable_composeClasses as composeClasses } from '@mui/base';
import { useSlotProps } from '@mui/base/utils';
import { OverridableComponent } from '@mui/types';
import { unstable_capitalize as capitalize } from '@mui/utils';
import { useThemeProps } from '../styles';
import styled from '../styles/styled';
import { getAspectRatioUtilityClass } from './aspectRatioClasses';
import { AspectRatioProps, AspectRatioTypeMap } from './AspectRatioProps';
import { AspectRatioProps, AspectRatioOwnerState, AspectRatioTypeMap } from './AspectRatioProps';

const useUtilityClasses = (ownerState: AspectRatioProps) => {
const useUtilityClasses = (ownerState: AspectRatioOwnerState) => {
const { variant, color } = ownerState;
const slots = {
root: ['root'],
Expand All @@ -28,7 +28,7 @@ const AspectRatioRoot = styled('div', {
name: 'JoyAspectRatio',
slot: 'Root',
overridesResolver: (props, styles) => styles.root,
})<{ ownerState: AspectRatioProps }>(({ ownerState }) => {
})<{ ownerState: AspectRatioOwnerState }>(({ ownerState }) => {
const minHeight =
typeof ownerState.minHeight === 'number' ? `${ownerState.minHeight}px` : ownerState.minHeight;
const maxHeight =
Expand All @@ -49,8 +49,8 @@ const AspectRatioRoot = styled('div', {
const AspectRatioContent = styled('div', {
name: 'JoyAspectRatio',
slot: 'Content',
overridesResolver: (props, styles) => styles.root,
})<{ ownerState: AspectRatioProps }>(({ theme, ownerState }) => [
overridesResolver: (props, styles) => styles.content,
})<{ ownerState: AspectRatioOwnerState }>(({ theme, ownerState }) => [
{
flex: 1,
position: 'relative',
Expand Down Expand Up @@ -89,10 +89,9 @@ const AspectRatio = React.forwardRef(function AspectRatio(inProps, ref) {
});

const {
className,
component = 'div',
children,
componentsProps,
componentsProps = {},
ratio = '16 / 9',
minHeight,
maxHeight,
Expand All @@ -115,19 +114,28 @@ const AspectRatio = React.forwardRef(function AspectRatio(inProps, ref) {

const classes = useUtilityClasses(ownerState);

const rootProps = useSlotProps({
elementType: AspectRatioRoot,
ownerState,
externalSlotProps: componentsProps.root,
externalForwardedProps: other,
additionalProps: {
ref,
as: component,
},
className: classes.root,
});

const contentProps = useSlotProps({
elementType: AspectRatioContent,
ownerState,
externalSlotProps: componentsProps.content,
className: classes.content,
});

return (
<AspectRatioRoot
as={component}
ownerState={ownerState}
className={clsx(classes.root, className)}
ref={ref}
{...other}
>
<AspectRatioContent
ownerState={ownerState}
{...componentsProps?.content}
className={clsx(classes.content, componentsProps?.content?.className, className)}
>
<AspectRatioRoot {...rootProps}>
<AspectRatioContent {...contentProps}>
{React.Children.map(children, (child, index) =>
index === 0 && React.isValidElement(child)
? React.cloneElement(child, { 'data-first-child': '' })
Expand All @@ -148,10 +156,6 @@ AspectRatio.propTypes /* remove-proptypes */ = {
* This can be an element, or just a string.
*/
children: PropTypes.node,
/**
* @ignore
*/
className: PropTypes.string,
/**
* The color of the component. It supports those theme colors that make sense for this component.
* @default 'neutral'
Expand All @@ -167,7 +171,8 @@ AspectRatio.propTypes /* remove-proptypes */ = {
* @default {}
*/
componentsProps: PropTypes.shape({
content: PropTypes.object.isRequired,
content: PropTypes.oneOfType([PropTypes.func, PropTypes.object]),
root: PropTypes.oneOfType([PropTypes.func, PropTypes.object]),
}),
/**
* The maximum calculated height of the element (not the CSS height).
Expand Down
12 changes: 9 additions & 3 deletions packages/mui-joy/src/AspectRatio/AspectRatioProps.ts
@@ -1,12 +1,18 @@
import * as React from 'react';
import { OverridableStringUnion, OverrideProps } from '@mui/types';
import { SlotComponentProps } from '@mui/base/utils';
import { ColorPaletteProp, VariantProp, SxProps } from '../styles/types';

export type AspectRatioSlot = 'root' | 'content';

export interface AspectRatioPropsColorOverrides {}
export interface AspectRatioPropsVariantOverrides {}

interface ComponentsProps {
root?: SlotComponentProps<'div', { sx?: SxProps }, AspectRatioOwnerState>;
content?: SlotComponentProps<'div', { sx?: SxProps }, AspectRatioOwnerState>;
}

export interface AspectRatioTypeMap<P = {}, D extends React.ElementType = 'div'> {
props: P & {
/**
Expand All @@ -23,9 +29,7 @@ export interface AspectRatioTypeMap<P = {}, D extends React.ElementType = 'div'>
* The props used for each slot inside the AspectRatio.
* @default {}
*/
componentsProps?: {
content: React.HTMLAttributes<HTMLDivElement> & { sx: SxProps };
};
componentsProps?: ComponentsProps;
/**
* The minimum calculated height of the element (not the CSS height).
*/
Expand Down Expand Up @@ -61,3 +65,5 @@ export type AspectRatioProps<
D extends React.ElementType = AspectRatioTypeMap['defaultComponent'],
P = { component?: React.ElementType },
> = OverrideProps<AspectRatioTypeMap<P, D>, D>;

export interface AspectRatioOwnerState extends AspectRatioProps {}
9 changes: 7 additions & 2 deletions packages/mui-joy/src/Avatar/Avatar.test.js
Expand Up @@ -20,6 +20,7 @@ describe('<Avatar />', () => {
testComponentPropWith: 'span',
testDeepOverrides: { slotName: 'fallback', slotClassName: classes.fallback },
testVariantProps: { variant: 'solid' },
testCustomVariant: true,
skip: ['classesRoot', 'componentsProp'],
}));

Expand Down Expand Up @@ -95,7 +96,9 @@ describe('<Avatar />', () => {

it('should be able to add more props to the image', () => {
const onError = spy();
const { container } = render(<Avatar src="/fake.png" imgProps={{ onError }} />);
const { container } = render(
<Avatar src="/fake.png" componentsProps={{ img: { onError } }} />,
);
const img = container.querySelector('img');
fireEvent.error(img);
expect(onError.callCount).to.equal(1);
Expand All @@ -113,7 +116,9 @@ describe('<Avatar />', () => {

it('should be able to add more props to the image', () => {
const onError = spy();
const { container } = render(<Avatar src="/fake.png" imgProps={{ onError }} />);
const { container } = render(
<Avatar src="/fake.png" componentsProps={{ img: { onError } }} />,
);
const img = container.querySelector('img');
fireEvent.error(img);
expect(onError.callCount).to.equal(1);
Expand Down