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.

Sign up for GitHub

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

Jump to bottom

[TypeScript] Fix Button component props accepts a record #7764

Merged
merged 9 commits into from
May 30, 2022
Merged

[TypeScript] Fix Button component props accepts a record #7764

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
536ae3a
Fix Button component accepts a record prop
fzaninotto May 30, 2022
e9ebd00
fix demo
fzaninotto May 30, 2022
57ca12d
Fix EditButton prop type
fzaninotto May 30, 2022
9dcfbf1
cleanup
fzaninotto May 30, 2022
51f2dab
Fix resource prop
fzaninotto May 30, 2022
84a1105
review
fzaninotto May 30, 2022
d440bb6
review
fzaninotto May 30, 2022
2f71f6b
Remove useless test
fzaninotto May 30, 2022
7d55633
Fix tests
fzaninotto May 30, 2022
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 examples/demo/src/reviews/ReviewEditToolbar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import AcceptButton from './AcceptButton';
import RejectButton from './RejectButton';
import { Review } from '../types';

const ReviewEditToolbar = (props: ToolbarProps<Review>) => {
const ReviewEditToolbar = (props: ToolbarProps) => {
const { resource } = props;
const redirect = useRedirect();
const notify = useNotify();
Expand Down
1 change: 0 additions & 1 deletion packages/ra-ui-materialui/src/button/Button.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { AdminContext } from '../AdminContext';
const invalidButtonDomProps = {
invalid: false,
pristine: false,
fzaninotto marked this conversation as resolved.
Show resolved Hide resolved
record: { id: 123, foo: 'bar' },
resource: 'posts',
mutationMode: 'pessimistic' as MutationMode,
fzaninotto marked this conversation as resolved.
Show resolved Hide resolved
};
Expand Down
21 changes: 4 additions & 17 deletions packages/ra-ui-materialui/src/button/Button.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,7 @@ import {
Theme,
} from '@mui/material';
import { styled } from '@mui/material/styles';
import {
MutationMode,
RaRecord,
RedirectionSideEffect,
useTranslate,
} from 'ra-core';
import { MutationMode, RedirectionSideEffect, useTranslate } from 'ra-core';
import { Path } from 'react-router';

/**
Expand All @@ -32,9 +27,7 @@ import { Path } from 'react-router';
* </Button>
*
*/
export const Button = <RecordType extends RaRecord = RaRecord>(
props: ButtonProps<RecordType>
) => {
export const Button = (props: ButtonProps) => {
const {
alignIcon = 'left',
children,
Expand Down Expand Up @@ -98,7 +91,7 @@ export const Button = <RecordType extends RaRecord = RaRecord>(
);
};

interface Props<RecordType extends RaRecord = RaRecord> {
interface Props {
alignIcon?: 'left' | 'right';
children?: ReactElement;
className?: string;
Expand All @@ -110,22 +103,16 @@ interface Props<RecordType extends RaRecord = RaRecord> {
size?: 'small' | 'medium' | 'large';
redirect?: RedirectionSideEffect;
fzaninotto marked this conversation as resolved.
Show resolved Hide resolved
variant?: string;
// May be provided manually
record?: RecordType;
resource?: string;
mutationMode?: MutationMode;
fzaninotto marked this conversation as resolved.
Show resolved Hide resolved
}

export type ButtonProps<RecordType extends RaRecord = RaRecord> = Props<
RecordType
> &
MuiButtonProps;
export type ButtonProps = Props & MuiButtonProps;

export const sanitizeButtonRestProps = ({
// The next props are injected by Toolbar
invalid,
pristine,
record,
redirect,
resource,
mutationMode,
Expand Down
13 changes: 4 additions & 9 deletions packages/ra-ui-materialui/src/button/CloneButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,7 @@ import PropTypes from 'prop-types';
import Queue from '@mui/icons-material/Queue';
import { Link } from 'react-router-dom';
import { stringify } from 'query-string';
import {
RaRecord,
useResourceContext,
useRecordContext,
useCreatePath,
} from 'ra-core';
import { useResourceContext, useRecordContext, useCreatePath } from 'ra-core';

import { Button, ButtonProps } from './Button';

Expand Down Expand Up @@ -52,7 +47,7 @@ const defaultIcon = <Queue />;
// useful to prevent click bubbling in a datagrid with rowClick
const stopPropagation = e => e.stopPropagation();

const omitId = ({ id, ...rest }: Partial<RaRecord>) => rest;
const omitId = ({ id, ...rest }: any) => rest;

const sanitizeRestProps = ({
resource,
Expand All @@ -61,12 +56,12 @@ const sanitizeRestProps = ({
}: Omit<CloneButtonProps, 'label' | 'scrollToTop' | 'icon'>) => rest;

interface Props {
record?: Partial<RaRecord>;
record?: any;
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably prefer Record<string, unknown> instead of any. Same everywhere we accept a RecordType

Copy link
Member Author

Choose a reason for hiding this comment

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

here, we used to accept a Partial<RaRecord>, which is indeed any

Copy link
Contributor

Choose a reason for hiding this comment

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

We should fix that everywhere in a dedicated PR. Passing a number as record does not make sense for instance

icon?: ReactElement;
scrollToTop?: boolean;
}

export type CloneButtonProps = Props & Omit<ButtonProps, 'record'>;
export type CloneButtonProps = Props & ButtonProps;

CloneButton.propTypes = {
icon: PropTypes.element,
Expand Down
3 changes: 2 additions & 1 deletion packages/ra-ui-materialui/src/button/DeleteButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ export const DeleteButton = <RecordType extends RaRecord = any>(
export interface DeleteButtonProps<
RecordType extends RaRecord = any,
MutationOptionsError = unknown
> extends ButtonProps<RecordType>,
> extends ButtonProps,
SaveContextValue {
confirmTitle?: string;
confirmContent?: string;
Expand All @@ -92,6 +92,7 @@ export interface DeleteButtonProps<
MutationOptionsError,
DeleteParams<RecordType>
>;
record?: RecordType;
}

DeleteButton.propTypes = {
Expand Down
3 changes: 2 additions & 1 deletion packages/ra-ui-materialui/src/button/DeleteWithConfirmButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ const defaultIcon = <ActionDelete />;
export interface DeleteWithConfirmButtonProps<
RecordType extends RaRecord = any,
MutationOptionsError = unknown
> extends ButtonProps<RecordType> {
> extends ButtonProps {
confirmTitle?: string;
confirmContent?: React.ReactNode;
icon?: ReactElement;
Expand All @@ -109,6 +109,7 @@ export interface DeleteWithConfirmButtonProps<
MutationOptionsError,
DeleteParams<RecordType>
>;
record?: RecordType;
}

DeleteWithConfirmButton.propTypes = {
Expand Down
3 changes: 2 additions & 1 deletion packages/ra-ui-materialui/src/button/DeleteWithUndoButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,14 +58,15 @@ const defaultIcon = <ActionDelete />;
export interface DeleteWithUndoButtonProps<
RecordType extends RaRecord = any,
MutationOptionsError = unknown
> extends ButtonProps<RecordType> {
> extends ButtonProps {
icon?: ReactElement;
onClick?: ReactEventHandler<any>;
mutationOptions?: UseMutationOptions<
RecordType,
MutationOptionsError,
DeleteParams<RecordType>
>;
record?: RecordType;
}

DeleteWithUndoButton.propTypes = {
Expand Down
14 changes: 9 additions & 5 deletions packages/ra-ui-materialui/src/button/EditButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import * as React from 'react';
import { ReactElement } from 'react';
import PropTypes from 'prop-types';
import ContentCreate from '@mui/icons-material/Create';
import { ButtonProps as MuiButtonProps } from '@mui/material/Button';
import { Link } from 'react-router-dom';
import {
RaRecord,
Expand All @@ -25,7 +24,9 @@ import { Button, ButtonProps } from './Button';
* <EditButton label="Edit comment" />
* );
*/
export const EditButton = (props: EditButtonProps) => {
export const EditButton = <RecordType extends RaRecord = any>(
props: EditButtonProps<RecordType>
) => {
const {
icon = defaultIcon,
label = 'ra.action.edit',
Expand Down Expand Up @@ -61,14 +62,17 @@ const defaultIcon = <ContentCreate />;
// useful to prevent click bubbling in a datagrid with rowClick
const stopPropagation = e => e.stopPropagation();

interface Props {
interface Props<RecordType extends RaRecord = any> {
icon?: ReactElement;
label?: string;
record?: RaRecord;
record?: RecordType;
scrollToTop?: boolean;
}

export type EditButtonProps = Props & ButtonProps & MuiButtonProps;
export type EditButtonProps<RecordType extends RaRecord = any> = Props<
RecordType
> &
ButtonProps;

EditButton.propTypes = {
icon: PropTypes.element,
Expand Down
5 changes: 0 additions & 5 deletions packages/ra-ui-materialui/src/button/SaveButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import ContentSave from '@mui/icons-material/Save';
import { useFormContext, useFormState } from 'react-hook-form';
import {
CreateParams,
MutationMode,
RaRecord,
TransformData,
UpdateParams,
Expand Down Expand Up @@ -160,10 +159,6 @@ interface Props<
transform?: TransformData;
saving?: boolean;
variant?: string;
// May be injected by Toolbar - sanitized in Button
record?: RaRecord;
resource?: string;
mutationMode?: MutationMode;
}

export type SaveButtonProps<RecordType extends RaRecord = any> = Props<
Expand Down
14 changes: 10 additions & 4 deletions packages/ra-ui-materialui/src/button/ShowButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,13 @@ import { Button, ButtonProps } from './Button';
* <ShowButton label="Show comment" record={record} />
* );
*/
const ShowButton = (props: ShowButtonProps) => {
const ShowButton = <RecordType extends RaRecord = any>(
props: ShowButtonProps<RecordType>
) => {
const {
icon = defaultIcon,
label = 'ra.action.show',
record: recordProp,
scrollToTop = true,
...rest
} = props;
Expand Down Expand Up @@ -58,14 +61,17 @@ const defaultIcon = <ImageEye />;
// useful to prevent click bubbling in a datagrid with rowClick
const stopPropagation = e => e.stopPropagation();

interface Props {
interface Props<RecordType extends RaRecord = any> {
icon?: ReactElement;
label?: string;
record?: RaRecord;
record?: RecordType;
scrollToTop?: boolean;
}

export type ShowButtonProps = Props & ButtonProps;
export type ShowButtonProps<RecordType extends RaRecord = any> = Props<
RecordType
> &
ButtonProps;
Comment on lines -68 to +76
Copy link
Contributor

Choose a reason for hiding this comment

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

EditButton also has MuiButtonProps allowed. Should we add them here as well? (although I agree this is beyond the scope of the problem solved by this PR)

Copy link
Member Author

Choose a reason for hiding this comment

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

ButtonProps already extends MuiButtonProps


ShowButton.propTypes = {
icon: PropTypes.element,
Expand Down
13 changes: 3 additions & 10 deletions packages/ra-ui-materialui/src/form/Toolbar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import {
Theme,
} from '@mui/material';
import clsx from 'clsx';
import { RaRecord } from 'ra-core';

import { SaveButton, DeleteButton } from '../button';

Expand Down Expand Up @@ -55,11 +54,7 @@ import { SaveButton, DeleteButton } from '../button';
* @prop {ReactElement[]} children Customize the buttons you want to display in the <Toolbar>.
*
*/
export const Toolbar = <
RecordType extends Partial<RaRecord> = Partial<RaRecord>
>(
props: ToolbarProps<RecordType>
) => {
export const Toolbar = (props: ToolbarProps) => {
const { children, className, resource, ...rest } = props;

const isXs = useMediaQuery<Theme>(theme => theme.breakpoints.down('sm'));
Expand Down Expand Up @@ -88,18 +83,16 @@ export const Toolbar = <
);
};

export interface ToolbarProps<RecordType extends Partial<RaRecord> = any>
extends Omit<MuiToolbarProps, 'classes'> {
export interface ToolbarProps extends Omit<MuiToolbarProps, 'classes'> {
children?: ReactNode;
className?: string;
record?: RecordType;
resource?: string;
}

Toolbar.propTypes = {
children: PropTypes.node,
className: PropTypes.string,
record: PropTypes.any,

resource: PropTypes.string,
};

Expand Down