Feature/dialog improvements

[Denny09310]

Description

• Added new base classes (DialogData, DialogData<TResult>) to support asynchronous and strongly-typed dialog results.
• Implemented Alert, Prompt, and custom Component dialogs, including their respective option classes and dedicated Razor components.
• Updated DialogService to manage a generic dialog collection and expose new methods for alert, prompt, and custom dialogs.
• Refactored dialog resolution to a generic Resolve<TResult> pattern with centralized closing logic and improved backdrop handling.
• Introduced DialogOptions, DialogOpenOptions, the DialogSize enum, and the DialogResult type.
• Added a reusable DialogHeader component for standardized dialog headers.
• Improved overall extensibility and reusability of the dialog system.

Type of Change

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update
• Refactoring (no functional changes)

Testing Checklist

• Blazor Server
• Blazor WebAssembly
• Blazor Hybrid (MAUI)
• Keyboard navigation / accessibility
• Dark mode

Related Issues

Closes #145

[Denny09310]

I don't know if it's better to unify all dialog results into a single class 'DialogResult' with derived classes for shorthands (similar to PromptDialogResult) or return directly the value, because I think instead of reacting "if the Value is null" (in the prompt example) we can differentiate between a null value and a cancelled state. What do you think? 😁

[Denny09310]

Sorry for the push forces but I've to disable my tailwind vs extensions, or it will reorder the classes every time making the commit unreadable

[mathewtaylor]

Hi @Denny09310, thanks for the commit, this is a great addition. I have reviewed and have some comments on the changes.

Breaking Change

1. The Confirm() return type changed from Task to Task — This is a breaking API change. Every existing consumer of DialogService.Confirm() will fail to compile. Is it possible to add a backward-compatible overload that still returns Task?

Potential Issues

1. Alert dialog resolves with DialogResult.Cancel() instead of DialogResult.Ok() — In AlertDialog.razor, the acknowledge button calls DialogResult.Cancel(). An alert acknowledgment is a confirmation, not a cancellation. The Alert() method's doc says "completes when user clicks OK" but the result will have Cancelled = true.
2. PromptDialogOptions.MaxLength is never enforced — The option exists and is documented ("the rendered input element should enforce this value via the maxlength attribute") but PromptDialog.razor never passes MaxLength to the .
3. DialogOptions.ConfirmText defaults to default! (null) — The base class DialogOptions has ConfirmText = default! which is a null-forgiving operator on a string. If a subclass forgets to set it in its constructor, this will render null in the button. Lets change this to have a sensible default like "OK" or be required.
4. ComponentDialogData takes DialogService as constructor parameter — This creates a circular dependency smell (service creates data, data holds reference to service). The other dialog types don't do this, they let the Resolve call flow through the provider. Consider making ComponentDialog.razor inject DialogService like the other internal dialogs do, and have IDialogReference methods dispatch through it.
5. role="alertdialog" and aria-modal="true" were removed — The original BbDialogProvider had proper ARIA attributes. The new internal dialog components have no ARIA attributes at all. This is an accessibility regression.
6. Duplicate
   on DialogService — Lines 717-726 of the diff show two consecutive

   XML doc blocks on DialogService. The old one ("Service for showing programmatic confirm dialogs") should be removed.
7. Orphaned XML doc comment in DialogData.cs — Line 564 has an empty
   tag with no content (was meant to document the tcs field but it was removed, leaving a dangling doc comment).
8. DialogContainerClass duplicated across 3 internal dialogs — AlertDialog.razor, ConfirmDialog.razor, and PromptDialog.razor all define the same const string DialogContainerClass with identical Tailwind classes. This should be a shared constant or handled by the parent BbDialogProvider.
9. TryBackdropClose doesn't handle Escape key — DialogOpenOptions.PreventClose says it "Prevents closing via Escape key or backdrop click" but there's no Escape key handler anywhere. The backdrop click is handled, but keyboard dismissal is not implemented.

Missing

1. No focus trap / scroll lock — The dialog overlay doesn't trap focus or lock scroll, which the existing Primitives Dialog component handles. This is expected for a service-based dialog, but worth noting.
2. No API surface test updates — Given the significant public API additions (DialogResult, DialogData, DialogSize, IDialogReference, etc.), the snapshot tests should have been updated. - I will handle this part.

Let me know if you need a hand with any of these changes.

Mathew

[Denny09310]

The Confirm() return type changed from Task to Task — This is a breaking API change. Every existing consumer of DialogService.Confirm() will fail to compile. Is it possible to add a backward-compatible overload that still returns Task?

Yes, I'm working on it, also I will ensure this methods ends with Async as they return a task, if it's okay.

Alert dialog resolves with DialogResult.Cancel() instead of DialogResult.Ok() — In AlertDialog.razor, the acknowledge button calls DialogResult.Cancel(). An alert acknowledgment is a confirmation, not a cancellation. The Alert() method's doc says "completes when user clicks OK" but the result will have Cancelled = true.

Already fixed just need to push the commit

PromptDialogOptions.MaxLength is never enforced — The option exists and is documented ("the rendered input element should enforce this value via the maxlength attribute") but PromptDialog.razor never passes MaxLength to the .

For this I should modify even the Input component as I can't pass the native maxlength attribute, or in alternative validate it on the Submit method

In the end I've implemented a better error validation handling directly in the PromptDialog

ComponentDialogData takes DialogService as constructor parameter — This creates a circular dependency smell (service creates data, data holds reference to service). The other dialog types don't do this, they let the Resolve call flow through the provider. Consider making ComponentDialog.razor inject DialogService like the other internal dialogs do, and have IDialogReference methods dispatch through it.

Sorry, I understood the problem, but I don't understand the part ''making ComponentDialog.razor inject DialogService ... and have IDialogReference methods dispatch through it." should I create a separate implementation of DialogReference taking both the ComponentDialogData and DialogService and then pass that as CascadingValue?

role="alertdialog" and aria-modal="true" were removed — The original BbDialogProvider had proper ARIA attributes. The new internal dialog components have no ARIA attributes at all. This is an accessibility regression.

Yes, sorry I didn't even saw them when moving components around 😅

Duplicate on DialogService — Lines 717-726 of the diff show two consecutive XML doc blocks on DialogService. The old one ("Service for showing programmatic confirm dialogs") should be removed.

Yes thanks, merge conflicts issues

Orphaned XML doc comment in DialogData.cs — Line 564 has an empty tag with no content (was meant to document the tcs field but it was removed, leaving a dangling doc comment).

Same as before

DialogContainerClass duplicated across 3 internal dialogs — AlertDialog.razor, ConfirmDialog.razor, and PromptDialog.razor all define the same const string DialogContainerClass with identical Tailwind classes. This should be a shared constant or handled by the parent BbDialogProvider.

Yes, you're right. I'll move up to BbDialogProvider

TryBackdropClose doesn't handle Escape key — DialogOpenOptions.PreventClose says it "Prevents closing via Escape key or backdrop click" but there's no Escape key handler anywhere. The backdrop click is handled, but keyboard dismissal is not implemented.

I need to add this yet. If you could help me in this one, because I've no idea how to do it properly

No focus trap / scroll lock — The dialog overlay doesn't trap focus or lock scroll, which the existing Primitives Dialog component handles. This is expected for a service-based dialog, but worth noting.

There is a possibility to reuse existing components logic without duplicating it from BbDialogContent?

Thanks for reviewing the code!

[Denny09310]

Okay, let's keep it like this for now. I don't want to make any mess. Sorry for the email spamming (probably) 😅

[mathewtaylor]

Hi @Denny09310, looking into this and will get back shortly. I will see if I can push back to your branch, I might not be able to. If not I will need to create a separate PR I think.

[mathewtaylor]

Hi @Denny09310, I needed to create a new PR in the end, I was not able to push my changes back to your PR because its from a fork. You can check out everything here #207. I think you did a great job on this, thank for the contribution. Take a look, if you're happy, I will merge it into develop.

Cheers,
Mathew

[Denny09310]

Thanks a lot, sorry again for the mess 😅. I will close this