Allow using v-show for NcModal

[raimund-schluessler]

This is an alternative approach to #3762 to fix #497. It is a bit more effort compared to #3762 because we need an additional prop and kind of duplicate the internal showModal state with the show prop to keep it non-breaking, but it feels more like vue to me.

For the next major version we could make the show prop required and get rid of the watcher and the internal showModal value, which would simplify it a lot.

[susnux]

I think we can get rid of the watcher and the internal state already, simply replace all usage of showModal with the show prop, add a v-show="show" on the mask and set the default to true.

Then it should work like before (default open when used with v-if) but also allow using v-show.

[raimund-schluessler]

I think we can get rid of the watcher and the internal state already, simply replace all usage of showModal with the show prop, add a v-show="show" on the mask and set the default to true.

Then it should work like before (default open when used with v-if) but also allow using v-show.

Let me try that.

[raimund-schluessler]

I think we can get rid of the watcher and the internal state already, simply replace all usage of showModal with the show prop, add a v-show="show" on the mask and set the default to true.

Then it should work like before (default open when used with v-if) but also allow using v-show.

Sorry, I think this doesn't work, because we have no way of updating the show prop internally without making it required. Then it would be breaking.
We want to set it to false in the close() method, otherwise it would be fine to be always true.

[susnux]

Sorry, I think this doesn't work, because we have no way of updating the show prop internally without making it required. Then it would be breaking.

I tried this, I do not see how this is breaking:
master...fix/ncmodal-show-prop

It works for v-if as before and additionally adds support to keep the modal in the dom by setting show to false.
Or am I missing something?

[raimund-schluessler]

Sorry, I think this doesn't work, because we have no way of updating the show prop internally without making it required. Then it would be breaking.

I tried this, I do not see how this is breaking: master...fix/ncmodal-show-prop

It works for v-if as before and additionally adds support to keep the modal in the dom by setting show to false. Or am I missing something?

Mind creating a PR? I assumed the show prop wouldn't be updated if there was no :show.sync="showModal" set on the component. Then the out-transition would not be triggered when using v-if. But even if that's the case, probably no one would notice, since it's anyway unmounted. 🤔

[raimund-schluessler]

Sorry, I think this doesn't work, because we have no way of updating the show prop internally without making it required. Then it would be breaking.

I tried this, I do not see how this is breaking: master...fix/ncmodal-show-prop
It works for v-if as before and additionally adds support to keep the modal in the dom by setting show to false. Or am I missing something?

Mind creating a PR? I assumed the show prop wouldn't be updated if there was no :show.sync="showModal" set on the component. Then the out-transition would not be triggered when using v-if. But even if that's the case, probably no one would notice, since it's anyway unmounted. 🤔

@susnux I created a PR #3769 with your changes in https://github.com/nextcloud/nextcloud-vue/tree/fix/ncmodal-show-prop and when using v-if without :show.sync="showModal", the transitions don't run anymore. I don't know how bad this is, but it could be considered a bug I guess.

[susnux]

the transitions don't run anymore. I don't know how bad this is, but it could be considered a bug I guess.

I noticed that, I am unsure about the badness of that too...
If it is too bad, I would stick with your changes and change the property to required with version 8.

[raimund-schluessler]

Superseded by #3769.