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

MdDialog communication (i.e: passing data to MdDialog) #2181

Closed
shlomiassaf opened this issue Dec 12, 2016 · 17 comments · Fixed by #2266
Closed

MdDialog communication (i.e: passing data to MdDialog) #2181

shlomiassaf opened this issue Dec 12, 2016 · 17 comments · Fixed by #2266
Assignees
Labels
feature This issue represents a new feature or feature request rather than a bug or bug fix

Comments

@shlomiassaf
Copy link
Contributor

Bug, feature request, or proposal: Proposal

What is the expected behavior?

Pass data to a component opened by the dialog service without depending on the dialog instance.

What is the current behavior?

You can pass data to a component by accessing it's instance and setting a predefined property.

What is the use-case or motivation for changing an existing behavior?

There are several ways to communicate with a dialog component:

  1. Custom Dependency injection - Set Providers as part of the options
  2. Fixed Dependency injection - Set a single data item as part of the options, injected via a token.
  3. MdDialogRef as a carrier - Set a single data item as part of the options, component consumed via MdDialogRef.data.
  4. Direct assignment - Current behavior

The current behavior allow (4) and a limited (1)

(1) Only predefined providers.

It does not fit the angular pattern.

Angular uses DI a lot, this is part of the flow.

  • Automatic errors if DI mismatch
  • Allows proper use of hooks
  • Less dependant

Since the dialog service abstract the component instantiation there is no control over the providers.
However, this process does allow changing providers by supplying an injector...

So all in all I think it should be possible to get some sort of control over the DI.

Workaround:

My current workaround is to create a service wrapping MdDialog.
This services uses a wrapper component as the Component to create.
The wrapper component then create the original component inside it with the supplied providers.

@crisbeto
Copy link
Member

If I'm understanding it correctly, you'd want the dialog module to be something like:

constructor(data: SomeUserSuppliedData) {
  console.log(data);
}

Instead of having to do this:

constructor(dialogRef: mdDialogRef) {
  console.log(dialogRef.componentInstance.someUserSuppliedData);
}

@shlomiassaf
Copy link
Contributor Author

shlomiassaf commented Dec 12, 2016

Yep, something like this:

constructor(dialogRef: mdDialogRef,  /*public?*/ data: SomeUserSuppliedData) {
}

This does not apply constraints, it can be strict or @Optional and the developer doesn't need to check if the value has been assigned or not... if not the DI will throw.

Its much more angular than

dialogRef.componentInstance.someUserSuppliedData

In my modal library it's working quite well.

@fxck
Copy link
Contributor

fxck commented Dec 12, 2016

I still think that the best way(or at least a way that should be possible) to go about modals is to use inputs/outputs.

<md-modal [open]="isModalOpen" (backdropClick)="isModalOpen = false">
 <whatever-cmp 
   [data]="someData" 
   (onSave)="doWhatever($event);isModalOpen = false">
  </whatever-cmp>
</md-modal>

the way portal/overlay works, this can easily sit in your template, not created until [open] is true, and you can use the component inside as you would any other angular2 component. It's as angular2 as it gets.

I have yet to see a case where this wouldn't be sufficient.

@shlomiassaf
Copy link
Contributor Author

@fxck That's preference, its not best vs worst... just what people prefer.

The question you should ask is, can I get from A->B and from B->A?

Can you get from component based modal (your proposal) to a service base modal? I think no.
But you can build a component based modal based on a service...

So provide a service, if someone needs a declarative way later, no problem.

@crisbeto About the providers, it's not much work anyway, you already need to create a child injector since you inject the MdDialogRef so it a quick fix.

@jelbourn jelbourn added the feature This issue represents a new feature or feature request rather than a bug or bug fix label Dec 13, 2016
@crisbeto crisbeto self-assigned this Dec 18, 2016
crisbeto added a commit to crisbeto/material2 that referenced this issue Dec 18, 2016
Adds the ability to pass in data to a dialog via the `data` property. While this was previously possible through the `dialogRef.componentInstance.someUserSuppliedProperty`, it wasn't the most intuitive. The new approach looks like this:

```
dialog.open(SomeDialog, {
  data: {
    hello: 'world'
  }
});

class SometDialog {
  constructor(data: MdDialogData) {
    console.log(data['hello']);
  }
}
```

Fixes angular#2181.
@abidev
Copy link

abidev commented Jan 3, 2017

@shlomiassaf Can you please share your Workaround in a bin or plunker ?

@xtianus79
Copy link

xtianus79 commented Jan 6, 2017

haha @shlomiassaf

I pretty much came up with a similiar scenario... can you please share your workaround?

  1. the dialog modal popup should be able to be called from any component including in and of itself.
    (Component A calls component A and accesses data from Component A)
  2. the dialog modal popup should be able to be called from another component while accessing data from a predeterminate component. (Component A calls component B and access Data from Component B)
  3. the dialog modal popup should be able to be called from another component while accessing data from itself. (Component A calls component B and access Data from Component A)
  4. the dialog modal popup should be able to be called from another component while accessing data from a predeterminate component and or itself. (Component A calls component B and access Data from Component A and or Component B)
  5. the api should be precise and intuitive to use.

@shlomiassaf
Copy link
Contributor Author

shlomiassaf commented Jan 7, 2017

Hi guys,

You need to remember that the Dialog service is still young :)
Material 1 provided a fluent AP and it can be added here but I'm not sure on the MdDialog service, maybe another service that use the dialog service for it...

@abidev @xtianus79 I can't find that piece of code so i'll try to write down what to do:

Create a service with an open method like in MdDialog, inject MdDialog to this service.

open<T>(component: ComponentType<T>, config?: MdDialogConfig): MdDialogRef<T>;

Extend MdDialogConfig with an interface that accepts a providers array
Replace the new type with the original type in the open method you created.

Create a component that accepts a component and a list of providers as parameters and render it the component with the list of providers
If you don't know how to, this is the probably hardest part, handling ViewContainerRef, ComponentFactoryResolver and their APIs to init a new component.
Once the component init's you get the list of providers and the component to render and render it...

If you think it's complicated no worries once the NgComponentOutlet PR gets into angular you can use it to do the above with little effort (just use injector instead of providers).

@patrykbialek
Copy link

Dear all, look here...

https://medium.com/@tarik.nzl/making-use-of-dialogs-in-material-2-mddialog-7533d27df41#.pf8d9o1ji

Less code in controller(component) - only to call 'dialog' service...

crisbeto added a commit to crisbeto/material2 that referenced this issue Jan 12, 2017
Adds the ability to pass in data to a dialog via the `data` property. While this was previously possible through the `dialogRef.componentInstance.someUserSuppliedProperty`, it wasn't the most intuitive. The new approach looks like this:

```
dialog.open(SomeDialog, {
  data: {
    hello: 'world'
  }
});

class SometDialog {
  constructor(data: MdDialogData) {
    console.log(data['hello']);
  }
}
```

Fixes angular#2181.
@xtianus79
Copy link

@patrykbialek I don't get how that answers what we are specifically referring to... Calling components and or modules isn't the issue... it is enacting upon code/functions at a specific point in time while the modal/dialog is awake. i.e. passing data to the open dialog modal.

crisbeto added a commit to crisbeto/material2 that referenced this issue Jan 21, 2017
Adds the ability to pass in data to a dialog via the `data` property. While this was previously possible through the `dialogRef.componentInstance.someUserSuppliedProperty`, it wasn't the most intuitive. The new approach looks like this:

```
dialog.open(SomeDialog, {
  data: {
    hello: 'world'
  }
});

class SometDialog {
  constructor(data: MdDialogData) {
    console.log(data['hello']);
  }
}
```

Fixes angular#2181.
crisbeto added a commit to crisbeto/material2 that referenced this issue Jan 31, 2017
Adds the ability to pass in data to a dialog via the `data` property. While this was previously possible through the `dialogRef.componentInstance.someUserSuppliedProperty`, it wasn't the most intuitive. The new approach looks like this:

```
dialog.open(SomeDialog, {
  data: {
    hello: 'world'
  }
});

class SometDialog {
  constructor(data: MdDialogData) {
    console.log(data['hello']);
  }
}
```

Fixes angular#2181.
@vigie
Copy link

vigie commented Feb 4, 2017

Nice fix @crisbeto, any idea when we might see this land in a release? #2266

@vigie
Copy link

vigie commented Feb 4, 2017

This is where you lose me @shlomiassaf:

Create a component that accepts a component and a list of providers as parameters

Exactly how do you pass those parameters to the wrapper component? As input properties after it is instantiated (but wouldn't this be too late)? Or do you create a custom injector and then use ViewContainerRef to create the component? If so, what ViewContainerRef do you use?

@vigie
Copy link

vigie commented Feb 4, 2017

I have yet to see a case where this wouldn't be sufficient.

@fxck My use case is that I want certain dialogs to be shown in response to the user clicking certain context menu items. These menus can be triggered by many different elements in many different routes from potentially different modules, so there is no obvious ancestor template to put them in and I obviously don't want to duplicate markup in different templates. Can you think of a nice way to handle such a situation without using the dynamic approach suggested above?

@fxck
Copy link
Contributor

fxck commented Feb 4, 2017

depends on how many modals are you talking about, but what would I probably do is

  1. use ngrx/store or any other global state managment, you can easily store the dialog state from any component in your app
  2. create a "smart" component and put it inside your root component, this component will inject store and select the slice with your dialog state
  3. put a "dumb" dialog component(feat(dialog): add md-dialog component #2855) inside it that will be open/closed depending on the state passed down from the smart component

@vigie
Copy link

vigie commented Feb 6, 2017

@fxck Thanks for linking me to the md-dialogPR. Lets say I have two dozen different modals that can be invoked throughout the application, for the approach you are advocating above to work, would I need 24 md-dialog DOM entries in my root component, each hosting the specific dialog markup inside?

@fxck
Copy link
Contributor

fxck commented Feb 6, 2017

@vigie it's literally the same as with using standard .open() method, so the same approaches you'd use to reduce boilerplate in your services you'd use in templates. The only difference is how you can pass in data to it and receive events from.

@fxck
Copy link
Contributor

fxck commented Feb 6, 2017

Also it doesn't necessarily have to be inside the root component, you can create one or more container components. It doesn't matter if you put md-dialog in a nested component, it will always append it to the body(without leaving the element in those nested nodes).

tinayuangao pushed a commit that referenced this issue Feb 6, 2017
* feat(dialog): add a config option for passing in data

Adds the ability to pass in data to a dialog via the `data` property. While this was previously possible through the `dialogRef.componentInstance.someUserSuppliedProperty`, it wasn't the most intuitive. The new approach looks like this:

```
dialog.open(SomeDialog, {
  data: {
    hello: 'world'
  }
});

class SometDialog {
  constructor(data: MdDialogData) {
    console.log(data['hello']);
  }
}
```

Fixes #2181.

* Switch to using an OpaqueToken.

* chore: fix aot errors
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature This issue represents a new feature or feature request rather than a bug or bug fix
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants