spartan icon indicating copy to clipboard operation
spartan copied to clipboard

Usage of <hlm-dialog-content> in dynamic content dialog results in endless loop

Open tvollstaedt opened this issue 1 year ago • 3 comments

Please provide the environment you discovered this bug in.

Created a StackBlitz that took the official example and wrapped the dialog content into <hlm-dialog-content>.

Which area/package is the issue in?

accordion

Description

The Dialog component allows the usage with dynamic content by providing a separate component with Dialog's usage. The example on spartan ui shows how to use it this way and makes use of other dialog-related components like <hlm-dialog-header>. However, when using <hlm-dialog-content> as one would presume (also to keep semantic meaning), the Dialog will render itself in an endless loop. Since the example does not make use of <hlm-dialog-content> I guess it's not compatible. This should be either stated with an JS error or within the docs. Neither is the case.

Please provide the exception or error you saw

The dialog renders itself in an endless loop fashion. No error shown or displayed (other then "stack limit exceeded").

Other information

No response

I would be willing to submit a PR to fix this issue

  • [ ] Yes
  • [X] No

tvollstaedt avatar Sep 16 '24 10:09 tvollstaedt

I would assume this is because that the component you pass through to the .open function is actually rendered inside the by virtue of the dialogContext, but if you use inside a component you're trying to render its just going to keep grabbing the component from the context and rendering it over and over.

seems like just some doco updates would be needed no?

LukeEdgecock-Bennett avatar Sep 20 '24 07:09 LukeEdgecock-Bennett

@tvollstaedt I think @LukeEdgecock-Bennett is correct here! We need to be clearer in the documentation here: The mimics the RadixUI API. It's actually a tag that wraps what will be rendered as the content of the dialog, hence you find the title and description inside of the content. When you use a dynamically rendered component that component "becomes" the wrapper.

Does that make sense?

goetzrobin avatar Sep 24 '24 20:09 goetzrobin

@tvollstaedt I think @LukeEdgecock-Bennett is correct here! We need to be clearer in the documentation here: The mimics the RadixUI API. It's actually a tag that wraps what will be rendered as the content of the dialog, hence you find the title and description inside of the content. When you use a dynamically rendered component that component "becomes" the wrapper.

Does that make sense?

Yeah it make sense I guess, just keep in mind that new dev's could fall into the same trap as the doc's don't explicitly state what you just described. I also think a docs update would be the best & simplest solution here.

tvollstaedt avatar Sep 24 '24 21:09 tvollstaedt