feat(dialog): add more dialog options to BrnDialogDefaultOptions

[marcjulian]

• change closeDelay to 100 default
• use closeDelay input value
• use defaultOptions values for input signal defaults

PR Checklist

Please check if your PR fulfills the following requirements:

• The commit message follows our
  guidelines: https://github.com/goetzrobin/spartan/blob/main/CONTRIBUTING.md#-commit-message-guidelines
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

• Bugfix
• Feature
• Code style update (formatting, local variables)
• Refactoring (no functional changes, no api changes)
• Build related changes
• CI related changes
• Documentation content changes
• Other... Please describe:

Which package are you modifying?

• accordion
• alert
• alert-dialog
• aspect-ratio
• avatar
• badge
• button
• calendar
• card
• checkbox
• collapsible
• combobox
• command
• context-menu
• data-table
• date-picker
• dialog
• dropdown-menu
• hover-card
• icon
• input
• label
• menubar
• navigation-menu
• pagination
• popover
• progress
• radio-group
• scroll-area
• select
• separator
• sheet
• skeleton
• slider
• sonner
• spinner
• switch
• table
• tabs
• textarea
• toast
• toggle
• tooltip
• typography

What is the current behavior?

Closes #608

What is the new behavior?

Does this PR introduce a breaking change?

• Yes
• No

provideBrnDialogDefaultOptions needs to be added to HlmAlertDialogComponent

import {
	BRN_ALERT_DIALOG_DEFAULT_OPTIONS,
} from '@spartan-ng/brain/alert-dialog';
import { provideBrnDialogDefaultOptions } from '@spartan-ng/brain/dialog';

providers: [
		...,
		provideBrnDialogDefaultOptions({
			...BRN_ALERT_DIALOG_DEFAULT_OPTIONS,
		}),
	],

Other information

[elite-benni]

Maybe i have to check it again. But the reason why i didn't add more properties to the defaultOptions is because is felt misleading.

You would have to add it to the helm component as well, otherwise it won't work.
Atleast thats what I experienced on my tests.

I am a big fan of the inject defaultOptions pattern but it doesnt work that good for components.

Maybe I am wrong and had some other mistakes in my code while trying, but thats why i refactored back to the mutableSignals instead of the default options.

If it really works this way i would rather remove all mutable signals, than leaving 2 ways to set the values.

[marcjulian]

I thought this is common practices to provide default input, where the component inputs cannot be used.
We have provideBrnButtonConfig, provideHlmDatePickerConfig etc.

Not sure why it didn't work before, but looks like that the only option closeDelay wasn't even used from defaultOptions nor from the closeDelay input.

For me it seems to work, try it out with hm-alert-dialog and add state open to provideBrnDialogDefaultOptions and the alert dialog should be open when navigating to the alert-dialog page.
However, for this case setting the state via provideBrnDialogDefaultOptions causes issues for the outputs on initial page load.

I guess we will still need the mutableSignals for inputs which may change for other inputs like in the trigger directives.

[elite-benni]

I just nearly went crazy because it really works.
My problem was, that I wanted to add the provideconfig to BrnAlertDialogComponent but it is not reused automatically for HlmAlertDialogComponent.
This means that you have to also Change the HlmComponent to make this feature work again, which is maybe a bigger breaking change.

But maybe it was seen too serious by me.

As written in my first comment, i like the defaultOptions pattern a lot and would love this to be the default way to set the options.

You are right, not all of them can be removed!
I am pretty sure we can remove some of the mutableInputs, because they are just used by helm component to set it.

The following should be able to remove:

mutableCloseOnBackdropClick
mutableRole
_mutableHasBackdrop
mutableScrollStrategy

[elite-benni]

Thanks for this improvement.

LGTM!!!

I just wanted to point out, that it is a potential breaking change for users that use their own helm components, because they have to add the provideBrnDialogDefaultOptions, but I think it is ok.
We had some of these "breaking changes" while refactoring the inputs.

[marcjulian]

I updated the breaking change section