RFC: Alert Dialog about spartan HOT 4 CLOSED

Hard one to start with, due to dialog considerations :) I like the directive approach to dialog-title, description etc. I'd honestly take it further and make the entire thing a structural directive as this would allow us to pass context to the entire component & extend it, remove the internal button from it's content projection & make the current *dialogContent directive unnecessary.

Single component example:

<button [brnDialogTriggerFor]="myAlert" />

<div #myAlert *brnDialog="data; let ctx=context;" />
    <!-- dialog content -->
</div>

This would also allow for a constructor based approach a la angular cdk

@Component({
  // dialog
})
class DialogComponent {
  private readonly ctx = inject(BrnDialogContext);
}

@Component({
   template: `
   <button (click)="openDialog()" />Open</button>
   `
})
class ParentComponent {
   private readonly dialog = inject(BrnDialog);



   openDialog() {
      this.dialog.open(DialogComponent, {
         // options,
        data
      })
   }
}

We could separate data & the dialog ref into separate injection tokens/passed context props or have them on the same object as I think most use both when using injection

interface BrnDialogRef<Output = unknown> {
   close(output?: Output): void;
   afterClosed(): Observable<Output | undefined>
   // other relevant fns
}


type BrnDialogContext<Input = unknown, Output = unknown> {
   readonly $implicit: Input;
   readonly ref: BrnDialogRef<Output>;
   readonly data: Input // getter to $implicit for easier typing within .ts files
}

Then the alert can either be done as a simple alert component, composed with the dialog component itself or as syntax sugar for the dialog directive through extension. Functionally it would do the same.

I'm a bit unsure on the separation of Cancel & Action buttons, maybe an angular material like approach with a simple [brn-dialog-close]="outputData" would suffice?

The overlay i don't mind either way, since tw would allow for any reasonable customization through background color, opacity.. class props

from spartan.

We could do that too for sure. Using the wrapper component could allow people to add classes to that component which are used to style the backdrop, and "hides" the manual wiring up of the trigger by using a reference to the directive exposed by "exportAs". I think that would make it more beginner friendly, but they're also not mutually exclusive.

If you have a wrapper it does the wiring up for you if you don't you'll need to do it yourself?

from spartan.

Yeah I just looked at the radix UI code to see if any special aria roles are assigned by those close and action buttons and it does not seem so. I think it could be a convenience thing to add the directives, but with exposing the context it's absolutely not needed. Very open to drop them tbh

from spartan.

Fair :) a wrapper it is.

Not really firm on this one either, personally id always use close, but we can always add more descriptive selectors to that directive

from spartan.