os-js / osjs-dialogs

OS.js Dialogs Module
https://manual.os-js.org/v3/
Other
4 stars 7 forks source link

Issue with Cancel button on Save Dialog #6

Closed josephjeno closed 5 years ago

josephjeno commented 5 years ago

I am using the below code for creating and acting on a Save Dialog in OSjs.

    core.make('osjs/dialog', 'file', {type: 'save', title: "Save As"},  {parent: win, attributes: {modal: true}}, (status, file, event) => {
      if (status === "ok") {
        if (!file.path.endsWith(".wf"))
          file.path += ".wf";
        basic.emit('save-file', file);
      }
    });

And the resulting Dialog:

image

At this point, I would expect clicking the Cancel button to trigger a callback with status = 'cancel' and file = null. However the Cancel button callback doesn't trigger until Filename is populated with some text.

There also appears to be an error handler that triggers if the text equals an existing filename, even if the Cancel button was pressed and not the OK button, example below:

image

Pressing No returns to the Save Dialog, and pressing Yes on the Confirm window closes the Save Dialog and sends the status = 'cancel' callback.

I believe expected behavior is to not display the Confirm dialog when Cancel is pressed and to simply close the Save Dialog while sending the 'cancel' callback, regardless of whether any text is input in the Filename field.

Thanks Anders, you're the man.

andersevenrud commented 5 years ago

I've pushed a new release that should fix the following conditions:

  1. Pressing Cancel on a save dialog with empty field produces no closure
  2. Pressing Cancel will check for existence instead of closure

Run npm update @osjs/dialogs and npm run build to update.

Let me know if this takes care of this issue and close it :)

andersevenrud commented 5 years ago

In hindsight this was kind of a silly mistake. The Cancel button was never actually accounted for in the callback function, so this should fix any side-effects relating to pushing that button.

I really need to get started on adding tests here as I've done in all other libraries related to this project :blush:

andersevenrud commented 5 years ago

Just doing a ping since it's been almost a week since you opened this issue :innocent:

josephjeno commented 5 years ago

Ah my bad, yes it's been working perfecto! Thanks Anders!