Conversation
src/main.ts
Outdated
| function getDialog(validate: (password: string) => Promise<void>): { dialog: App, eventBus: Emitter<PasswordDialogEvents> } { | ||
| console.debug('Prompting password form') | ||
| const eventBus = mitt<PasswordDialogEvents>() | ||
| const dialog = spawnDialog(PasswordDialogVue, { eventBus, validate }, () => {}) |
There was a problem hiding this comment.
We don't need an event bus here to handle closing.
If we removed _nc_password_confirmation_dialog, we can just pass onClose handler here directly as we are passing props.
If we want to reuse the same dialog, we can implement the prompt as the dialog's exposed public method.
There was a problem hiding this comment.
I talked to Louis and he confirmed that we can drop the global dialog handle.
This simplifies the whole logic and would in theory open up the possibility to use the same code for both Vue versions (vue-demi?).
There was a problem hiding this comment.
This simplifies the whole logic and would in theory open up the possibility to use the same code for both Vue versions (vue-demi?).
No. vue-demi works well for pure vue libraries, but here we have many dependencies, and they are not compatible with Vue in a "demi" way. Every dependency must also be based on vue-demi, or not use anything changed in Vue 3
There was a problem hiding this comment.
To support both versions in practice, we can bundle Vue in this library. Then it doesn't matter, what Vue version a library user uses.
However, it has a drawback - it makes the library MUCH bigger in the bundle size. I'll check if we can make it not so big. But it will be several times bigger than the current size.
There was a problem hiding this comment.
We have a spawnDialog function now in the nextcloud-vue.
Once it's backported to nextcloud-vue@8, we can use it in this library. Then the only difference between Vue 2 and Vue 3 versions is dependency versions.
This is also why we used forward compatible features:
susnux
left a comment
There was a problem hiding this comment.
only docs need to be adjusted otherwise fine for migration
Signed-off-by: Richard Steinmetz <[email protected]>
|
I squashed without rebasing and applied Ferdinand's suggestion. Nothing else was changed. |
I tested this with twofactor_webauthn and it works fine.
Caveat
This is a breaking change and we need to maintain two versions for the foreseeable future. One for Vue 2 and one for Vue 3.
TODO
Clarify if we can drop the window global to store the dialog-> Yes, we can