aurelia / dialog

A dialog plugin for Aurelia.
MIT License
106 stars 115 forks source link

Global callbacks / events every time a dialog opens / closes #374

Closed RomkeVdMeulen closed 5 years ago

RomkeVdMeulen commented 5 years ago

I'm submitting a feature request

Expected/desired behavior:

There's a couple of ways this could be handled. The DialogService could accept subscribers for each of the events and call them when appropriate. Or events could be published on the EventAggregator. Or perhaps there's another API I haven't thought of.

RomkeVdMeulen commented 5 years ago

@EisenbergEffect @StrahilKazlachev Any thoughts on this? If there are better ways to solve this I'd be happy to change the proposal.

bigopon commented 5 years ago

@RomkeVdMeulen beforeOpen sounds like activate and afterOpen sounds like attached to me. Is this correct?

RomkeVdMeulen commented 5 years ago

For individual dialogs, yes. But I want to implement behaviour for every dialog.

I could put those hooks in a base class and extend every dialog from that, but that isn't the most elegant way of implementing a cross-cutting concern.

StrahilKazlachev commented 5 years ago

So if you need to do something immediately before/after the dialog gets added/removed to/from the DOM, you can use a custom Renderer. You should be able to extend the default ones, override showDialog/hideDialog and register your Renderer. Here the Renderer is the piece that adds/removes the "dialog" from the DOM. showDialog/hideDialog are the closest points to the DOM manipulation that I can think of, excluding writing a dedicated Renderer.

bigopon commented 5 years ago

@RomkeVdMeulen I was expecting the concern you raised, thanks for putting it plainly. How would you, even with some pseudo code, solve your issue with events?

RomkeVdMeulen commented 5 years ago

@bigopon Something like:

dialogService.beforeOpen((dialog) => {
  dialog.lastFocused = document.querySelector(":focus");
});

dialogService.afterClose((dialog) => {
  dialog.lastFocused.focus();
});

@StrahilKazlachev Interesting. I'll look into that.

You know, come to think of it: shouldn't the behaviour of returning focus after the dialog closes be built into aurelia-dialog itself? It seems important to have the plugin produce accessible dialogs out of the box, without having to add some listeners yourself.

Perhaps I should open a separate bug for this?

StrahilKazlachev commented 5 years ago

The way all this is designed is that all DOM related work should go in a Renderer or some custom element in the "dialog" itself. Also I think that currently the Renderer is the weakest link in the whole concept.

RomkeVdMeulen commented 5 years ago

I'm going to assume we want to bake this behavior in somewhere in stead of leaving it to the user.

I'll open a separate bug for that.