feat: extend alert interface with div props

[mnlfischer]

Description

• extend alert interface with 'div' component props

[linear]

PLA-1202 Extend Alert component in Hailstorm with props

[rodelta]

I think I've seen another component that was doing this, and it was pretty hard to use. The autocomplete had a huge list of options, and it was hard to find the ones that are "intended props to control this" and which were "optional prop drilling for parent element". I'm not sure what's the intended case here, either.

What do you think about:

1. Explicitly typing only the things we want to modify, like classnames or some listener/callbacks. We would inject tailwind classes for all styling.

2. Having an explicit prop to pass props to the parent div. This would keep the usage clean

interface DemoCompProps {
   title: string; 
   // ...
   topLevelProps: React.ComponentPropsWithoutRef<"div">;
} 

below we would deestructure topLevelProps instead of whatever falls into the ... props } thing

[mnlfischer]

Thank you for your feedback. In general, I completely agree with you that we only pass on props that we really want to make available and make it easy to access.

1. we always proceed in this way until we reach a point where we need the native properties. I think this approach is good because we start with the smallest and then build up from there. When we need the native properties, I think it's more solid to pass the ones defined by React completely, which will create the same behavior as the native element and complement it. For example, the data-attribute properties are well defined, which we either have to define completely ourselves anyway or create utility functions, which increases the complexity and we continue to increase it with each additional similar property.
   I agree that it's not beneficial for autocompletion, however you can always jump into the interface definition or look it up in the storybook.

2. the problem with child elements is unfortunately not solved nicely and we try to split the component so small that the passing of the properties only reaches the component that has been implemented and no child elements are affected. Many components have a very granular division and here we could consider extending the component with a title component to make it clear that the properties only apply to the explicit component.
   I consider the distribution of properties to certain child elements to be more complex, as in this example it is relatively simple, but there are components that have significantly more child elements if they have not been defined granularly enough. The problem is then rather the definition of the component that should be solved.

Idea
Extract the title into Alert.title component and create a real compound component but this is also a really small component and maybe not worth it regarding the effort vs. solving the issue that can be solved by a click into the interface definition. Also the icon could be valid to extract but the design is fixed by putting it to the left and this would be open it a bit too much.

[rodelta]

Point 1 was not about duplicating props, but expose them in a controlled manner. For example, picking callbacks from DOMAttributes for callbacks, extending only AriaAttributes, etc.
But the main point is pretty fair, the type definition should be enough to see what's custom and what's extended from React.

If I understand correctly, your problem with 2 is that in some components we need to inject things to several children, so we should create subcomponents instead of named props. I think this makes sense as a better alternative, but does not discount naming the injected props to make them more ergonomic.

We could have something like injectedProps, which we only allow one of, so if you need to expose children there, you need to create a subcomponent. The extra complexity is minimal, and it's a bit cleaner/more explicit, but I get that that's going away from the spirit of exposing a "modified html element" you can interact with normally. This seems to be a matter of opinion, though.

I think I raised this point because when moving away from abusix-ui, I've seen a bunch of custom props, misused ...props and stuff like that. At some point, if you expose too much and the consumers are not careful, the component library loses its point, and it makes it hard to migrate.

I'm still unsure about going deeper into this pattern, but I'll approve to not block the thing, specially considering the pattern already exists in the codebase. Thanks for the analysis here!

[mnlfischer]

I will note that for a deeper investigation if we can find a better solution or define a general way of how we want to create component interfaces. Your feedback is valuable and this topic is not easy and until now I did not find any solution without a drawback. Lets have a look into public libs and get a feeling about if we are on a good way! Thanks @rodelta!

[abusix-bot]

🎉 This PR is included in version 5.4.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀