goetzrobin / spartan

Cutting-edge tools powering Angular full-stack development.
https://spartan.ng
MIT License
1.1k stars 121 forks source link

RFC: Radio Groups as card layouts #246

Open marcjulian opened 3 months ago

marcjulian commented 3 months ago

Which scope/s are relevant/related to the feature request?

radio-group

Information

Allow Radio Groups to be styled in any layout such as these cards.

CleanShot 2024-04-02 at 19 35 25 CleanShot 2024-04-02 at 19 36 11

label is part of BrnRadioComponent how to style the label for custom layouts? Should the label be extracted from BrnRadioComponent to be styled by the user and follow a similar API as shadcn?

Shadcn API

<RadioGroup defaultValue="option-one">
 <div>
            <RadioGroupItem
              value="card"
              id="card"
              className="peer sr-only"
              aria-label="Card"
            />
            <Label
              htmlFor="card"
              className="flex flex-col items-center justify-between rounded-md border-2 border-muted bg-transparent p-4 hover:bg-accent hover:text-accent-foreground peer-data-[state=checked]:border-primary [&:has([data-state=checked])]:border-primary"
            >
              <svg
                xmlns="http://www.w3.org/2000/svg"
                viewBox="0 0 24 24"
                fill="none"
                stroke="currentColor"
                strokeLinecap="round"
                strokeLinejoin="round"
                strokeWidth="2"
                className="mb-3 h-6 w-6"
              >
                <rect width="20" height="14" x="2" y="5" rx="2" />
                <path d="M2 10h20" />
              </svg>
              Card
            </Label>
          </div>
   ...
</RadioGroup>

Describe any alternatives/workarounds you're currently using

No response

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

goetzrobin commented 3 months ago

How about we make styles and classes for the label, indicator, input inside the brn-radio inputs that can be overwritten? That way we have more flexibility on how to style the actual button. The anatomy of the brn-radio is inspired by Material, and I think in the case you are describing the label would be marked with sr-only and the indicator/touchTarget is where you would style the Card

marcjulian commented 2 months ago

Input component design

API design when we would add class inputs for label, indicator and input for brn-radio could look like

<brn-radio-group hlm class="grid grid-cols-3 gap-4" [(ngModel)]="card">
  <brn-radio hlm value="card" labelClass="...">
    <svg>...</svg>
    Card
  </brn-radio>
   ...
</brn-radio-group>

What about refactoring the hlm directives to components to use them instead

<hlm-radio-group class="grid grid-cols-3 gap-4" [(ngModel)]="card">
  <hlm-radio value="card" labelClass="...">
    <svg>...</svg>
    Card
  </hlm-radio>
   ...
</hlm-radio-group>

Extract label component design

What if we extract the label from brn-radio:

// brn-radio.component.ts
<div style="display: flex; height: fit-content; width: fit-content" (click)="_onTouchTargetClick($event)">
  <ng-content select="[target],[indicator]" />
</div>
<input 
  #input 
  style="position: absolute; width: 1px; height: 1px; padding: 0; margin: -1px; overflow: hidden; clip: rect(0, 0, 0, 0); white-space: nowrap; border-width: 0;"
  type="radio" 
  [id]="inputId" 
  [checked]="checked" 
  [disabled]="disabled" 
  [attr.name]="name" 
  [attr.value]="value" 
  [required]="required" 
  [attr.aria-label]="ariaLabel" 
  [attr.aria-labelledby]="ariaLabelledby" 
  [attr.aria-describedby]="ariaDescribedby" 
  (change)="_onInputInteraction($event)" 
  (click)="_onInputClick($event)" 
/>
- <label style="display: flex; height: fit-content; width: fit-content" [for]="inputId">
  <ng-content></ng-content>
- </label>

The API could look like as follow when we pull out the label from brn-radio, but we also need to handle <label [for]="inputId > each time.

<brn-radio-group hlm class="grid grid-cols-3 gap-4" [(ngModel)]="version">
  <brn-radio hlm value="card">
    <label class="...">
      <svg>...</svg>
      Card
    </label>
  </brn-radio>
  ...
</brn-radio-group>

Or with hlm components

<hlm-radio-group class="grid grid-cols-3 gap-4" [(ngModel)]="version">
  <hlm-radio value="card">
    <label class="...">
      <svg>...</svg>
      Card
    </label>
  </hlm-radio>
  ...
</hlm-radio-group>
marcjulian commented 2 months ago

You think it would be better to move the card layout into the indicator selector, something like this:

<brn-radio-group hlm class="grid grid-cols-3 gap-4" [(ngModel)]="card">
  <brn-radio hlm value="card" labelClass="sr-only">
    <!-- projected into indicator: touch area -->
    <div indicator class="...">
      <svg>...</svg>
      Card
    </div>
    <!-- projected into label: apply sr-only class-->
    Card
  </brn-radio>
</brn-radio-group>
goetzrobin commented 2 months ago

Interesting. What are you thinking?

goetzrobin commented 2 months ago

After reviewing this I think my absolute favorite would be this. And inside hlm-radio we allow you access to the label classes and styles.

<hlm-radio-group class="grid grid-cols-3 gap-4" [(ngModel)]="version">
  <hlm-radio value="card">
      Card
  </hlm-radio>
  ...
</hlm-radio-group>

And then for a custom radio we could do something like this:

<brn-radio-group class="grid grid-cols-3 gap-4" [(ngModel)]="card">
  <brn-radio value="card" labelClass="sr-only">
    <!-- projected into indicator: touch area -->
    <div indicator class="...">
      <svg>...</svg>
      Card
    </div>
    <!-- projected into label: apply sr-only class-->
    Card
  </brn-radio>
</brn-radio-group>
marcjulian commented 2 months ago

This sounds good to me.

<hlm-radio-group class="grid grid-cols-3 gap-4" [(ngModel)]="version">
  <hlm-radio value="card">
      Card
  </hlm-radio>
  ...
</hlm-radio-group>

But the for the custom radio, I don't like that we need to repeat the label. What is there reason not to project the custom radio into the label and apply the needed styles?

goetzrobin commented 1 month ago

@marcjulian just getting back to this. Were you able to get this to work? Anything I can do to help

marcjulian commented 3 weeks ago

@goetzrobin I started with converting the hlm radiogroup directives to components. However, it is not working yet.

For example the hlm-radio wraps brn-radio, but brn-radio injects the BrnRadioGroupComponent which is undefined. Should I create a draft PR and we can discuss a solution there? Is it possible for HlmRadio to extend BrnRadio?

@Component({
    selector: 'hlm-radio',
    template: `
        <brn-radio [class]="_computedClass()" [value]="value()" [disabled]="disabled()">
            <ng-content></ng-content>
        </brn-radio>
    `,
    imports: [BrnRadioComponent],
    standalone: true,
    host: {
        class: 'block',
    },
})
export class HlmRadioComponent {
goetzrobin commented 3 weeks ago

@marcjulian that works for me. I was also going to take a crack at it, but if you already started we can combine efforts!