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

[dialog] Add imperative methods to display them directly #24759

Open
CarbonPool opened this issue Feb 3, 2021 · 7 comments
Open

[dialog] Add imperative methods to display them directly #24759

CarbonPool opened this issue Feb 3, 2021 · 7 comments
Labels
component: dialog This is the name of the generic UI component, not the React module! new feature New feature or request waiting for 👍 Waiting for upvotes

Comments

@CarbonPool
Copy link

CarbonPool commented Feb 3, 2021

Refer to the call and implementation of the ant-design dialog box. This solution brings more flexibility and concise code.

Will there be such a solution?

import { Dialog } from "@material-ui/core";

Dialog.show({
  type: "info",
  title:  "Message",
  content: "Hello",
  onOk: () => { ... }
});

Benchmark

@CarbonPool CarbonPool added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Feb 3, 2021
@eps1lon
Copy link
Member

eps1lon commented Feb 3, 2021

Could you expand a bit on how you would use it and why the existing API cannot be used for that use case?

Offering an imperative API for state updates can be quite problematic since we or React is no longer in control of the semantics of such an update. So these APIs impose quite the maintenance burden and it's not clear why it is needed considering this is a React library.

@eps1lon eps1lon added component: dialog This is the name of the generic UI component, not the React module! new feature New feature or request and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Feb 3, 2021
@CarbonPool
Copy link
Author

CarbonPool commented Feb 3, 2021

Could you expand a bit on how you would use it and why the existing API cannot be used for that use case?

Offering an imperative API for state updates can be quite problematic since we or React is no longer in control of the semantics of such an update. So these APIs impose quite the maintenance burden and it's not clear why it is needed considering this is a React library.

Example: https://ant.design/components/modal/

In many scenarios, the notification dialog needs to be called repeatedly to avoid introducing too many <Diglog>{children}</Dialog> in the component render, but according to different states, the confirmation button flexibly controls the callback function. This idea comes from most systems API.

@eps1lon
Copy link
Member

eps1lon commented Feb 3, 2021

In many scenarios, the notification dialog needs to be called repeatedly to avoid introducing too many {children} in the component render

Could you describe these scenarios in detail? I'm not following why this API is required to display a single dialog instead of multiple ones.

@Sandeep06Dev
Copy link
Contributor

Could you expand a bit on how you would use it and why the existing API cannot be used for that use case?
Offering an imperative API for state updates can be quite problematic since we or React is no longer in control of the semantics of such an update. So these APIs impose quite the maintenance burden and it's not clear why it is needed considering this is a React library.

Example: https://ant.design/components/modal/

In many scenarios, the notification dialog needs to be called repeatedly to avoid introducing too many <Diglog>{children}</Dialog> in the component render, but according to different states, the confirmation button flexibly controls the callback function. This idea comes from most systems API.

To achieve scenario you can be use library such as toastify or notistack.
Even notistack is build with material UI ..
https://iamhosseindhv.com/notistack (Simply use custom snackback)

@oliviertassinari oliviertassinari changed the title Dialog adds static methods, such as <Dialog.show>? [Dialog] adds static methods, such as dialog.show() Feb 3, 2021
@oliviertassinari oliviertassinari changed the title [Dialog] adds static methods, such as dialog.show() [Dialog] Add imperative methods to display them directly Feb 3, 2021
@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 3, 2021

Thanks for opening this issue. I thought that we already had an open issue for this pain point. The equivalent issue for the Snackbar is #18098. I have updated the issue's description with the benchmark I have been doing on this for a year or two (I didn't find much).

@koistya
Copy link
Contributor

koistya commented Sep 17, 2022

@CarbonPool most likely you want imperative handler(s) to be set on the instance of the dialog component rather than relying on static methods, for example:

import * as React from "react";
import { Button, Container } from "@mui/material";
import { ConfirmDialog, DialogElement } from "./ConfirmDialog.js";

export function Example(): JSX.Element {
  const dialogRef = React.useRef<DialogElement>(null);

  return (
    <Container>
      <Button onClick={() => dialogRef.current?.open()}>Show Dialog</Button>
      
      <ConfirmDialog ref={dialogRef} onConfirm={...} />
    </Container>
  );
}

@oliviertassinari BTW, it might be a good idea to add .open() method to the Dialog's ref out of the box which should be a small tweak without any breaking changes.

kriasoft/react-starter-kit#2004


Alternatively, with <DialogProvider> and useDialog() hook, it may look like this:

import * as React from "react";
import { Button, Container } from "@mui/material";
import { useDialog } from "./core/dialog.js";
import { ConfirmDialog } from "./dialogs/ConfirmDialog.js"; // Custom dialog based on Material UI <Dialog />

export function Example(): JSX.Element {
  const dialog = useDialog(
    // factory method returning an instance of Material UI dialog
    () => <ConfirmDialog onConfirm={...} />, // `open`, and `onClose` props  will be added by the hook
    [] // React hook dependencies
  );

  return (
    <Container>
      <Button onClick={dialog.show}>Show Dialog</Button>
    </Container>
  );
}

By default, the DialogProvider can mount a single instance of the target dialog when used on multiple screens, and if a new instance of the dialog is needed, it can be done by passing key={...} with a unique value to the dialog props.


Alternatively, the useDialog() hook can operate without a need to add <DialogProvider> upstream like this:

import * as React from "react";
import { Button, Container } from "@mui/material";
import { useDialog } from "./core/dialog.js";
import { ConfirmDialog } from "./dialogs/ConfirmDialog.js"; // Custom dialog based on Material UI <Dialog />

export function Example(): JSX.Element {
  const dialog = useDialog(
    // factory method returning an instance of Material UI dialog
    () => <ConfirmDialog onConfirm={...} />, // `open`, and `onClose` props  will be added by the hook
    [] // React hook dependencies
  );

  return (
    <Container>
      <Button onClick={dialog.show}>Show Dialog</Button>
      {dialog.component}
    </Container>
  );
}

@oliviertassinari oliviertassinari added the waiting for 👍 Waiting for upvotes label Feb 1, 2024
@oliviertassinari oliviertassinari changed the title [Dialog] Add imperative methods to display them directly [dialog] Add imperative methods to display them directly May 30, 2024
@Janpot
Copy link
Member

Janpot commented May 31, 2024

The Toolpad team is exploring imperative APIs for interacting with dialogs. If this is something that interests you, I invite you to read and comment on the RFC.

Toolpad Core is new set of abstractions we're creating to help you build internal and B2B applications faster

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: dialog This is the name of the generic UI component, not the React module! new feature New feature or request waiting for 👍 Waiting for upvotes
Projects
None yet
Development

No branches or pull requests

6 participants