spartan
spartan copied to clipboard
Examples of how to wrap Brain components
Which scope/s are relevant/related to the feature request?
Don't know / other
Information
Background
One of the things that really excites me about this project is the potential to use the brain library as internal implementation details of our own, custom UI library.
In order to do this, we'd create wrappers around the brain components (akin to helm) that provide the styles and configuration.
I notice that the helm examples require the consumer of the components to be aware of both helm and brain (by adding both directives). This composable pattern is great for flexibility, but isn't something we'd want to expose to the consumers. The reason being is that, to get a specific type of menu, the consumer has to add lots of directives, classes, styles, which should be refactored into a higher-level component that handles this consistently.
So, we'd like all of the brain stuff to be fully encapsulated behind our own component abstractions.
The ability to refactor into wrapper components is an important principle for any component library that will be used to support a large application.
Prior art
The React Radix project has some really great concepts that facilitate this kind of approach. Namely, the asChild feature, which allows component developers to control the rendered elements.
Angular Solutions
Create custom components and use brain directives internally
We create our own set of components and use brain as an internal implementation detail.
E.g. Our Menu component would add the BrnMenu directive using directive composition.
This is great in theory as the use of brain is hidden behind our component API.
Unfortunately, angular has long removed the replace:true feature of AngularJS so we have to deal with the host element wrapping our custom markup in a way that often interferes with brain's (or other third-party library's) requirement to have a direct parent/child relationship, or for the brain directives to be added to the element
E.g.
<app-menu> <!-- this has `BrnMenu` in hostDirectives -->
<app-menu-item title="Item 1" /> <!-- this has `BrnMenuItem` in hostDirectives -->
</app-menu>
Becomes
<app-menu>
<app-menu-item title="Item 1">
<button /> <!-- This is now a child of the element that the brnMenuItem directive is added to -->
</app-menu-item>
</app-menu>
- If we leave the
brnMenuItemdirective on the host element, it won't apply the a11y logic to the right element (the button) - If we moved the
brnMenuItemdirective to the button instead, it would no longer be a direct child of brnMenu and wouldn't work.
Directive Composition
If we're prepared to not completely encapsulate everything behind our component API, we could make use of custom directive wrappers like:
<app-menu>
<button *appMenuItem title="">
</app-menu>
Where both Menu and MenuItem are custom directives that use hostBindings to compose the brain directives.
This is the same method brain uses to wrap Cdk. Where brain re-exports the cdk inputs/outputs using the directive composition API.
Unfortunately, our custom wrappers won't allow us to export those inputs/outputs again, because they don't belong to the brain components (they belong to cdk).
This appears to be a concious decision by Angular, so, unless this changes, I'm not sure there's much we can do here.
Some variant of the asChild feature of Radix
Have the brain directives support an asChild option that, when configured, makes the directive apply its logic to a child element instead of the host.
I would imagine this would add significant complexity to brain, and the React solutions to this benefit from things like prop spreading.
Closing
Really just looking for some discussion on how we see brain being used by component library developers, instead of just how you might use it to build a business requirement directly.
Describe any alternatives/workarounds you're currently using
No response
I would be willing to submit a PR to fix this issue
- [ ] Yes
- [X] No
The Radix asChild feature documentation for reference: https://www.radix-ui.com/primitives/docs/guides/composition
This is great feedback!
So the way I thought asChild was being used for Radix was as a replacement for directives as
React does not have anything like them. But maybe (by accident or not) this solution offers other benefits that directives alone cannot provide.
I am not completely sure I understand what you mean by this:
Unfortunately, our custom wrappers won't allow us to export those inputs/outputs again, because they don't belong to the brain components (they belong to cdk).
Are you saying that passing them through twice is not supported?
I understand what you are saying about the wrapper causing the menu logic to break
If you could share the repo / or create a stack blitz with the complete source code showcasing the issue that would be awesome.
So the way I thought asChild was being used for Radix was as a replacement for directives as React does not have anything like them. But maybe (by accident or not) this solution offers other benefits that directives alone cannot provide.
I notice that most brain components are using ContentChildren with descendants:true so this might only be an issue with trying to wrap Menu (which composes CdkMenu), as CdkMenu relies on this direct parent/child relationship.
This video explains how a Design System UI Component Library author might wrap/refactor Radix components into their own component API (which is what I'm trying to achieve in angular):
https://www.youtube.com/watch?v=VM6YRrUsnUY
Always feels like working against the framework when using Angular.
Are you saying that passing them through twice is not supported?
Yeah exactly that. When you use outputs or inputs, it makes those props from the hostDirective available in the host template, but they don't actually belong to the component, so any subsequent wrappers can't re-export. (The error message states that the input can't be forwarded because it doesn't exist on BrnMenu, or something similar)
Not sure if we can handle this by explicitly defining inputs/outputs in Brain.
I'll knock up a repro when I get some time!
Not sure if we can handle this by explicitly defining inputs/outputs in Brain.
That is what I am thinking. It should be possible. And you might be right about the CdkMenu.
Great video and I think that should be the goal ultimately for helm. Another thin layer that hides the more barebones APIs that make it so flexible and allow very easy and intuitive use inside your own applications.
P.S. This GitHub ticket talks about the reasoning why Angular decided not to allow re-exports of exported hostDirective input/outputs:
https://github.com/angular/angular/issues/48105
Very confusing. But I think adopting the style here is what I'd say makes the most sense to me: https://github.com/angular/angular/issues/48105#issuecomment-1321055121
Thoughts?
The problem there is that brain exports the output as brnCtxMenuTriggerFor
hostDirectives: [{ directive: CdkContextMenuTrigger, inputs: ['cdkContextMenuTriggerFor: brnCtxMenuTriggerFor'] }],
Which isn't appropriate for my wrapper component as it's leaking brain out past my API. If we changed implementations later, all of our features would break.
This is what I'm trying to achieve for my own component API (using brain internally)
<button app-button appearance="secondary" [appMenuTriggerFor]='menu'>Open menu</button>
<ng-template #menu>
<app-menu>
<app-menu-item title='Alpha' (triggered)='checked = !checked' />
<app-menu-item title='Bravo' />
<app-menu-divider />
<app-menu-item title='Charlie' />
</app-menu>
</ng-template>
Let me change it to this:
@Directive({
selector: '[brnCtxMenuTriggerFor]',
standalone: true,
hostDirectives: [CdkContextMenuTrigger],
})
export class BrnContextMenuTriggerDirective {
private readonly _cdkTrigger = inject(CdkContextMenuTrigger, { host: true });
private readonly _align = signal<BrnCtxMenuAlign>(undefined);
@Input()
set align(value: BrnCtxMenuAlign) {
this._align.set(value);
}
@Input()
set brnCtxMenuTriggerFor(value: TemplateRef<unknown> | null) {
this._cdkTrigger.menuTemplateRef = value;
}
constructor() {
effect(() => {
const align = this._align();
if (!align) return;
this._cdkTrigger.menuPosition = [
{
originX: align,
originY: 'bottom',
overlayX: align,
overlayY: 'top',
},
{
originX: align,
originY: 'top',
overlayX: align,
overlayY: 'bottom',
},
];
});
}
}
That should allow you to rename it
Hey @peterhodges! This was released in the newest alpha. Is this working for you?
Hey @peterhodges! I am curious if this is still an issue or if your experience with spartan/ui has improved. Let me know if we should close this issue or keep it open and give it some new attention!
Since we are now integrating hlm directly with brain primitives the source code of the hlm components/directives is a good example of how you can go about implementing your own custom wrappers. Here is the hlm dropdown stories that show an API that would allow you to create your self-styled dropdown in a similar way: https://github.com/goetzrobin/spartan/blob/main/libs/ui/menu/dropdown-menu.stories.ts
Let me change it to this:
@Directive({ selector: '[brnCtxMenuTriggerFor]', standalone: true, hostDirectives: [CdkContextMenuTrigger], }) export class BrnContextMenuTriggerDirective { private readonly _cdkTrigger = inject(CdkContextMenuTrigger, { host: true }); private readonly _align = signal<BrnCtxMenuAlign>(undefined); @Input() set align(value: BrnCtxMenuAlign) { this._align.set(value); }
@Input() set brnCtxMenuTriggerFor(value: TemplateRef
| null) { this._cdkTrigger.menuTemplateRef = value; } constructor() { effect(() => { const align = this._align(); if (!align) return; this._cdkTrigger.menuPosition = [ { originX: align, originY: 'bottom', overlayX: align, overlayY: 'top', }, { originX: align, originY: 'top', overlayX: align, overlayY: 'bottom', }, ]; }); } } That should allow you to rename it
@goetzrobin
Out of curiosity was this approach dropped? I need something similar myself. I’d prefer to avoid exposing brn inputs in our public facing components.
Right now, all the Storybook examples and docs seem to show direct usage of the Brain directives, which doesn’t align with how we want to abstract them internally.
Hey @OmerGronich can you give me an example of what you're looking for. are you running into the same issue?
Hey @OmerGronich can you give me an example of what you're looking for. are you running into the same issue?
I want consumers of my UI lib to use my own, library branded trigger (appMenuTriggerFor in this example) without ever needing to know about the underlying brain package. Ideally they could write:
<button
app-button
appearance="secondary"
<!-- 👇Currently impossible -->
[appMenuTriggerFor]="menu"
>
Open menu
</button>
<ng-template #menu>
<app-menu>
<app-menu-item title="Alpha" (triggered)="checked = !checked" />
<app-menu-item title="Bravo" />
<app-menu-divider />
<app-menu-item title="Charlie" />
</app-menu>
</ng-template>
But BrnContextMenuTriggerDirective has already aliased the relevant CDK inputs, so Angular won’t let me create a second alias under a different name. As a result, the internal brain directive leaks into my public API