Jenesius / vue-modal

🖖The progressive and simple modal system for Vue.js v3
https://modal.jenesius.com
MIT License
145 stars 14 forks source link

promptModal produce promise cannot go next step when call openModal #109

Closed drdevelop closed 7 months ago

drdevelop commented 8 months ago

Describe the bug promptModal produce promise cannot go next step when call openModal, the code after promptModal cannot exec forever

Please complete the following information:

Jenesius commented 8 months ago

Hi, can you show me a short example of this problem?

drdevelop commented 8 months ago

@Jenesius This is my miniest reproducible demo:

ModalA.vue:

<template>
  hello A
</template>

ModalB.vue

<template>
  hello B
</template>

Entry.vue

<template>
  <div>
    <div @click="showModalA">show modal A</div>
    <div @click="showModalB">show modal B</div>
  </div>
</template>

<script lang="ts" setup>
import { openModal, promptModal } from 'jenesius-vue-modal';
import ModalA from './ModalA.vue';
import ModalB from './ModalB.vue';

async function showModalA() {
  await promptModal(ModalA);
  console.log('show modal A run end'); // did not print
}

async function showModalB() {
  await openModal(ModalB);
  console.log('show modal B run end'); // print
}
</script>
drdevelop commented 8 months ago

i fix it in my local by rewrite promptModal method。or i can submit a pr to fix it.

Jenesius commented 8 months ago

Sorry for the long answer. Thank you again for the open task and the described problem!

The main question here is what should promptModal return if the window was closed without returning a value. Many people assume that a given Promise should be fulfilled if the value was accurately passed from the modal window. Otherwise, an error should be returned (rejected).

So I can do two ways to solve this problem:

  1. promptModal returns undefined if the modal window was closed without a corresponding value.

    const value = await promptModal(ModalCode);
    if (!value) // do something
  2. promptModal reject the Promise:

    try {
    const value = await promptModal(ModalCode);
    // do something
    } catch(e) {
    // handle prompt modal without returned value
    }

    or

    promptModal(ModalCode)
    .then(value => doSomething)
    .catch(handleModalWithoutReturnedValue)

We need to think about which method is more correct. The advantage of the second method is that it will be compatible with the previous version, because his behavior won't really change.

Jenesius commented 8 months ago

The method 'promptModal' was developed similar to the 'prompt': https://developer.mozilla.org/en-US/docs/Web/API/Window/prompt

By default it return null, if nothing was entered. Should we do the same...

this update will definitely be in version 2.x, so we can change the behavior a little.

drdevelop commented 8 months ago

The method 'promptModal' was developed similar to the 'prompt': https://developer.mozilla.org/en-US/docs/Web/API/Window/prompt

By default it return null, if nothing was entered. Should we do the same...

this update will definitely be in version 2.x, so we can change the behavior a little.

yep, i think we can do the same as browser native behavior,it doesn't increase the understanding burden on the user.

drdevelop commented 8 months ago

But it might be a little late until version 2.x? Is it too risky to upgrade version 1.x to 2.x?

Jenesius commented 8 months ago

Updates 2.x.x will definitely be available, but later. In them, I will update the structure of modal containers, as well as the internal code of this library. Perhaps I should reconsider the view of the major version, and lean more towards the minor version, but we will decide on this in the end.

The main reason why I'm looking at version 2.x.x is that people should pay attention to the changes and not just mindlessly update the version. I analyzed some of the projects that use this library, and also opened my work projects, where the team also relies on jenesius-vue-modal and found a large amount of code like this:

const value = await promptModal(ModalValue);
return value * 2;

This is a short and slightly modified example. However, he demonstrates what I am guilty of. The documentation does not say a word about the fact that this function will return something if you just close the modal window. And this gave rise to a large amount of erroneous code. In this example, the following specification would be correct:

const value = await promptModal(ModalValue);
if (value === null) throw new Error('...') 
return value * 2;

While I was writing this message, I took another look at the specification. I will try to re-read it again and form a set of rules for promptModal.

drdevelop commented 8 months ago

Ok,i will pay attention to the changes for the version of fix this problem. Expect it!

Jenesius commented 7 months ago

I have updated the library version to 1.11.0.

I had to add the [ondestroy] method (https://modal.jenesius.com/guide/modal-object.html#ondestroy) so that the prompt event was uniquely executed after the modal window was closed. NowpromptModal returns null if the window was closed without using the required event Modal.EVENT_PROMPT