deprecate / metal-clay-components

10 stars 14 forks source link

Power to customize ClayCheckbox and ClayRadio #182

Closed matuzalemsteles closed 6 years ago

matuzalemsteles commented 6 years ago

Currently ClayCheckbox and ClayRadio can not customize your label, there are cases like Card that we can make a Card as a checkbox.

I think we can make ClayCheckbox with a ClayCheckboxBase base class so we can have more customization power and make the ClayCheckbox class available to use as the current one.

- ClayCheckbox.js
- ClayCheckboxBase.js

In this thought we keep Checkbox markup control centralized in one place ClayCheckboxBase.

What do you guys think about that?

carloslancha commented 6 years ago

I think what you need can be done adding a parameter called, don't know, content and render it instead <span class="{$spanLabelClasses}">{$label}</span>

What do you think?

matuzalemsteles commented 6 years ago

Hey @carloslancha, I did not want to change the existing markup of the Card, currently, when a card is a checkbox its markup looks like this:

<div class="custom-control custom-checkbox">
    <label>
        <input class="custom-control-input" type="checkbox">
        <span class="custom-control-indicator"></span>
        <div class="card...">...</div>
    </label>
</div>
carloslancha commented 6 years ago

I know @matuzalemsteles basically there we're changing the span with the label text to a custom div, that's why I said about adding a content param for those cases we wan't to customize more the label.

matuzalemsteles commented 6 years ago

Sorry @carloslancha, had not understood well 😅. This approach is also simpler, but we will give the freedom that others can customize it, is this what we want? What do you think about this?

carloslancha commented 6 years ago

In this case I feel creating a ClayCheckboxBase or something like that is a little overkill for this and I'd go for a simpler approach even if that means more flexibility to the user. Both are good solutions, but I don't know...

@jbalsas any thoughts about this? 😛

jbalsas commented 6 years ago

It is of course convenient to allow any kind of markup injection, and the more we do the more it looks like we might end up there, but I don't like the idea...

I'm not sure how everything is currently setup, but maybe we could do something like this:

We currently have:

{template .input}
        ...
    <label>
        {let $inputAttributes kind="attributes"}
            ...
        {/let}

        <input {$inputAttributes}></input>

        <span class="custom-control-indicator"></span>

        {let $spanLabelClasses kind="text"}
            custom-control-description
            {if $hideLabel}
                {sp}sr-only
            {/if}
        {/let}

        <span class="{$spanLabelClasses}">{$label}</span>
    </label>
{/template}

We could do:

{template .input}
         ...
    <label>
        {let $inputAttributes kind="attributes"}
            ...
        {/let}

        <input {$inputAttributes}></input>

        <span class="custom-control-indicator"></span>

        {delcall ClayCheckbox.label variant="$labelVariant" /}
    </label>
{/template}

{deltemplate ClayCheckbox.label}
...
{/deltemplate}

This way, in ClayCard we could do something like:

{template .render}
    ...
    {call ClayCheckbox.render}
        {param labelVariant="card" /}
    {/call}
    ...
{/template}

{deltemplate ClayCheckbox.card}
    <div class="card...">
        ...
    </div>
{/deltemplate}
carloslancha commented 6 years ago

Sounds great @jbalsas I'm on it!

carloslancha commented 6 years ago

Proof of concept: https://github.com/metal/metal-clay-components/pull/186 - Rejected

carloslancha commented 6 years ago

Resolved in https://github.com/metal/metal-clay-components/pull/188