[madbob]

Simplified ConfirmationModal to use NcDialog.

Bonus: made translatable the title of "Delete tag" modal.

Fixes #10101

[kesselb]

Grazie @madbob for the PR, nice cleanup! 🙌

Il Claudio had a look and says:

• The unsubscribe link no longer opens. NcDialogButton has no href/rel/target props, so confirmUrl gets dropped. For lists without one-click unsubscribe, the dialog
  in ThreadEnvelope.vue just closes. Open the link in confirm() with window.open(url, '_blank', 'noopener,noreferrer'), or keep a real link button.
• The dialog closes too early. callback returns undefined, so NcDialog closes right away and the disabled/loading state for delete and unsubscribe never shows.
  Return false from the callback and let the parent close the dialog.
• Wrong close event. Switch @close to @closing or @update:open. close isn't an NcDialog event; it only works because the listener falls through to NcModal.
• Import path. Import from @nextcloud/vue/components/NcDialog, like the rest of the repo.
• Separate commit for the l10n fix. Put the "Delete tag" title fix in its own commit so it can be backported.
• Commit subjects. Use refactor(ui): use NcDialog for ConfirmationModal and fix(l10n): translate delete tag dialog title.

Thanks again!

[madbob]

The unsubscribe link no longer opens.

Not correct. href, rel and target are handled as fallthrough attributes and go to the NcButton just wrapped by NcDialogButton. Given those props, NcButton becomes an anchor.
I've empirically tried hard-coding a href prop to the ConfirmationModal in DeleteTagModal, and it works as expected.

The dialog closes too early.

We already had this conversation. I'm dumb. Fixed now.

Wrong close event.

Fixed.

Import path.

Ok, I've worked too much on Contacts (which is organized in exactly the opposite way).

Separate commit for the l10n fix.

Ok.

Commit subjects.

Ok.

[github-actions]

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

[kesselb]

Ok, I've worked too much on Contacts (which is organized in exactly the opposite way).

Thanks for the hint. Would you say it makes sense to align the eslint rules? cc @hamza221

[kesselb]

Hey, good call about our earlier discussion (#13357).

I'd say we should do the same change here. The callers have to pass in a function to handle the confirmation, so the dialog can await it and show the loading state until it's done. Also, the Cancel button's callback can go: @closing already emits cancel, so it currently fires twice.

props: {
	// ...
	confirm: {
		type: Function,
		required: true,
	},
},

emits: ['cancel'],

computed: {
	buttons() {
		return [
			{
				label: t('mail', 'Cancel'),
				variant: 'tertiary',
				disabled: this.disabled,
			},
			{
				label: this.confirmText,
				variant: 'primary',
				disabled: this.disabled,
				callback: async () => {
					await this.confirm()
					return false
				},
				href: this.confirmUrl,
				target: this.confirmUrl ? '_blank' : undefined,
			},
		]
	},
},

Callers:

• DeleteTagModal.vue: :confirm="deleteTag"
• ThreadEnvelope.vue (one-click): :confirm="unsubscribeViaOneClick"
• ThreadEnvelope.vue (link): :confirm="() => { showListUnsubscribeConfirmation = false }"
• ThreadEnvelope.vue (mailto): :confirm="unsubscribeViaMailto"

[hamza221]

Ok, I've worked too much on Contacts (which is organized in exactly the opposite way).

Thanks for the hint. Would you say it makes sense to align the eslint rules? cc @hamza221

I honestly prefer The way it's done in contacts side, unless there are some real perf implications

[madbob]

Ok, I've done some cleanup around on messy events.

Also, the Cancel button's callback can go: @closing already emits cancel, so it currently fires twice.

I'm not sure about this...

In the current setup (no explicit closing event emitted, relying on closing from NcDialog) I tried with

<DeleteTagModal @cancel="closeDeleteModal" />

but the closeDeleteModal() function is never called and the state flag this.deleteTagModal in TagModal stays dirty.

I cannot find any cancel in NcDialog nor NcModal.