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

Simplify Buttons Props #7750

Merged
merged 1 commit into from
May 24, 2022
Merged

Simplify Buttons Props #7750

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
27 changes: 16 additions & 11 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 @@ -19,12 +19,6 @@ import {
} from 'ra-core';
import { Path } from 'react-router';

export type LocationDescriptor = Partial<Path> & {
redirect?: boolean;
state?: any;
replace?: boolean;
};

/**
* A generic Button with side icon. Only the icon is displayed on small screens.
*
Expand All @@ -38,7 +32,9 @@ export type LocationDescriptor = Partial<Path> & {
* </Button>
*
*/
export const Button = (props: ButtonProps) => {
export const Button = <RecordType extends RaRecord = RaRecord>(
Copy link
Member

Choose a reason for hiding this comment

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

should default to any

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually, I failt to see why there is a record prop. This should be handled by the specialized buttons only imo

props: ButtonProps<RecordType>
) => {
const {
alignIcon = 'left',
children,
Expand Down Expand Up @@ -102,7 +98,7 @@ export const Button = (props: ButtonProps) => {
);
};

interface Props {
interface Props<RecordType extends RaRecord = RaRecord> {
Copy link
Member

Choose a reason for hiding this comment

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

Same, I think this is a breaking change

alignIcon?: 'left' | 'right';
children?: ReactElement;
className?: string;
Expand All @@ -114,13 +110,16 @@ interface Props {
size?: 'small' | 'medium' | 'large';
redirect?: RedirectionSideEffect;
variant?: string;
// May be injected by Toolbar
record?: RaRecord;
// May be provided manually
record?: RecordType;
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, I fail to see what record does here. It's not even used in the code. This somehow seems to forbid usage of the generic Button outside of a record context, e.g. in a List, a dialog, a sidebar, or a custom page. I wonder why, as Button is purely a UI component.

resource?: string;
mutationMode?: MutationMode;
}

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

export const sanitizeButtonRestProps = ({
// The next props are injected by Toolbar
Expand Down Expand Up @@ -186,3 +185,9 @@ const getLinkParams = (locationDescriptor?: LocationDescriptor | string) => {
state,
};
};

export type LocationDescriptor = Partial<Path> & {
redirect?: boolean;
state?: any;
replace?: boolean;
};
9 changes: 1 addition & 8 deletions 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 @@ -4,7 +4,6 @@ import PropTypes from 'prop-types';
import { UseMutationOptions } from 'react-query';
import {
RaRecord,
RedirectionSideEffect,
MutationMode,
DeleteParams,
useRecordContext,
Expand Down Expand Up @@ -82,18 +81,12 @@ export const DeleteButton = <RecordType extends RaRecord = any>(
export interface DeleteButtonProps<
RecordType extends RaRecord = any,
MutationOptionsError = unknown
> extends Omit<ButtonProps, 'record'>,
> extends ButtonProps<RecordType>,
SaveContextValue {
className?: string;
confirmTitle?: string;
confirmContent?: string;
icon?: ReactElement;
label?: string;
mutationMode?: MutationMode;
record?: RecordType;
redirect?: RedirectionSideEffect;
resource?: string;
// May be injected by Toolbar
mutationOptions?: UseMutationOptions<
RecordType,
MutationOptionsError,
Expand Down
8 changes: 1 addition & 7 deletions 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 @@ -10,7 +10,6 @@ import {
MutationMode,
RaRecord,
DeleteParams,
RedirectionSideEffect,
useDeleteWithConfirmController,
useRecordContext,
useResourceContext,
Expand Down Expand Up @@ -97,17 +96,12 @@ const defaultIcon = <ActionDelete />;
export interface DeleteWithConfirmButtonProps<
RecordType extends RaRecord = any,
MutationOptionsError = unknown
> extends Omit<ButtonProps, 'record'> {
className?: string;
> extends ButtonProps<RecordType> {
confirmTitle?: string;
confirmContent?: React.ReactNode;
icon?: ReactElement;
label?: string;
mutationMode?: MutationMode;
onClick?: ReactEventHandler<any>;
record?: RecordType;
redirect?: RedirectionSideEffect;
resource?: string;
// May be injected by Toolbar - sanitized in Button
translateOptions?: object;
mutationOptions?: UseMutationOptions<
Expand Down
9 changes: 1 addition & 8 deletions 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 @@ -8,7 +8,6 @@ import clsx from 'clsx';
import { UseMutationOptions } from 'react-query';
import {
RaRecord,
RedirectionSideEffect,
useDeleteWithUndoController,
DeleteParams,
useRecordContext,
Expand Down Expand Up @@ -59,15 +58,9 @@ const defaultIcon = <ActionDelete />;
export interface DeleteWithUndoButtonProps<
RecordType extends RaRecord = any,
MutationOptionsError = unknown
> extends Omit<ButtonProps, 'record'> {
className?: string;
> extends ButtonProps<RecordType> {
icon?: ReactElement;
label?: string;
onClick?: ReactEventHandler<any>;
record?: RecordType;
redirect?: RedirectionSideEffect;
resource?: string;
// May be injected by Toolbar - sanitized in Button
mutationOptions?: UseMutationOptions<
RecordType,
MutationOptionsError,
Expand Down