Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NcTextField v-model does not work when used in a spawnDialog component #6280

Closed
juliusknorr opened this issue Nov 28, 2024 · 7 comments
Closed
Labels
bug Something isn't working

Comments

@juliusknorr
Copy link
Contributor

Starting with 8.20 the richdocuments cypress tests for the save as dialog started to fail. When checking out the failure locally I've noticed that the v-model of the NcTextField used there is no longer working properly:

https://github.com/nextcloud/richdocuments/blob/dependabot/npm_and_yarn/nextcloud/vue-8.21.0/src/components/Modal/SaveAs.vue#L11-L15

In the console I'm getting the following error, however I lack deeper understanding in how vue js works in order to further debug this.

It seems to originate from #6172

Screenshot 2024-11-28 at 16 15 14

The only thing special about that usage compared to the examples in the docs is that the component is initialized as part of a spawnDialog method of @nextcloud/dialogs

@ShGKme Since you introduced that referenced pull request, do you have any idea what might go wrong here?

@juliusknorr juliusknorr added the bug Something isn't working label Nov 28, 2024
@juliusknorr
Copy link
Contributor Author

Interestingly just using spawnDialog it is not reproducable, wondering if this is another case of conflicting vue instances with viewer 🤷

@ShGKme
Copy link
Contributor

ShGKme commented Nov 28, 2024

the v-model of the NcTextField used

Before 8.20.0 NcTextField had no v-model interface at all. The use in SaveAs component is not valid. It tried to bind a variable with an "impossible" type (InputEvent on set and string on get). It technically wasn't possible to bind a "normal" property to v-model and there was no v-model interface documented in the docs.

It worked with a custom computed that handled it to avoid issue with using unexisting v-model interface. But it should have used :value.sync as it was the only available interface to have a 2-way bind value.

Since 8.20.0 v-model is available and can be used as expected - the setter of the computed receives the set value itself.

Replace this.selectedPath = event.target.value with this.selectedPath = event

@ShGKme
Copy link
Contributor

ShGKme commented Nov 28, 2024

But the error on the screenshot seems different from that

@ShGKme
Copy link
Contributor

ShGKme commented Nov 28, 2024

But the error on the screenshot seems different from that

Haven't tested yet, but if the component the parent are from different Vue copies, this error will appear.

@juliusknorr
Copy link
Contributor Author

I haven't been able to come up with a simple reproducer yet. I would expect that the the Vue instance that is used in spawnDialog is somewhat isolated from viewer though.

However i tried a similar approach as https://github.com/nextcloud/text/pull/5547/files now and it seems to do the trick.

@juliusknorr
Copy link
Contributor Author

However i tried a similar approach as nextcloud/text#5547 (files) now and it seems to do the trick.

Ok it does not, with that I'm getting Vue.js : $attrs is readonly, $listeners is readonly errors as then the viewer and richdocuments vue instances do not match any longer.

For now pushed that workaround to nextcloud/richdocuments#4259 but with those warnings it is likely just waiting for other issues to arise.

I'll give it a try to split the Viewer component from the one that actually does other component imports to be closer to text

@juliusknorr
Copy link
Contributor Author

Came up with nextcloud/richdocuments#4278 now.

@ShGKme I would consider this into another side effect of the viewer API reusing the vue instance of it, so does not seem like a but. Will close. Thanks for the first hints and the good explanation on v-model

@juliusknorr juliusknorr closed this as not planned Won't fix, can't repro, duplicate, stale Nov 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants