aurelia / dialog

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

Dialog VM hooks for keyboard (Enter/Escape) #344

Closed pape87 closed 6 years ago

pape87 commented 6 years ago

I'm submitting a feature request

Current behavior: When settings.keyboard is configured to respect keyboard events, upon pressing the key (Enter or Escape), only controller.ok()/controller.cancel() are called respectively. There is no opportunity to verify or pass a result.

Expected/desired behavior: Introduce two new life-cycle hooks to the dialog VM like this:

import { DialogController } from "aurelia-dialog";

export class DialogVM {
  constructor(private ctrl: DialogController) {
  }

  public ok() {
    // result of this function is passed to controller.ok()
    // if a rejected promise is returned or an error thrown do not close the dialog
  }

  public cancel() {
    // result of this function is passed to controller.cancel()
    // if a rejected promise is returned or an error thrown do not close the dialog
  }
}
StrahilKazlachev commented 6 years ago
  1. Yes, there is no way to pass custom data.
  2. I don't understand verify. To verify what? When calling .ok()/.cancel() if the VM implements .canDeactivate, that hook will be called with a DialogCloseResult. So you can see what intention it is being closed with, and prevent the closing if needed by returning false.
  3. The keyboard logic is all in the default Renderer implementation. And yes some Renderer options are mixed in the general ones, this has been since day one.
Alexander-Taran commented 6 years ago

Oh my!! and it is referenced in the docs http://aurelia.io/docs/plugins/dialog#lifecycle-hooks

never knew

StrahilKazlachev commented 6 years ago

Now that I think about it, both canDeactivate and deactivate hooks are called with DialogCloseResult. So you can manipulate the .output field - ~definitely~probably not intended and feels like a hack but it is doable, since we are not freezing the result object(don't think we'll do it).

Alexander-Taran commented 6 years ago

@pape87 do you think this satisfies you?

pape87 commented 6 years ago

Yes this solved my issue. I've added some info to the docs about the canDeactivate result parameter.

Thank you

Alexander-Taran commented 6 years ago

@pape87 take a look at this comment about interfaces available for dialog vm https://github.com/aurelia/framework/issues/874#issuecomment-374203146

Alexander-Taran commented 6 years ago

@pape87 thanks for your help with docs (-: