Proposal: extract core checkbox and radio behavior into separate @Directives

[shlomiassaf]

• Do you want to request a feature or report a bug? feature
• What is the current behavior?
  A User can use md-checkbox / md-radio-X components to create chebox's and radio's.
• What is the expected behavior?
  User can apply checkbox / radio behaviour to any element as well as use the currently available
  md-checkbox / md-radio-X components.
• What is the motivation / use case for changing the behavior?
  By implement checkbox & radio group/button as directive's users can apply them on other elements.
  The current implementation of md-checkbox & md-radio-X will use the directives in their implementation so we are not duplicating code.
  Having this, user's can easily create a Toggle button or a group of toggle buttons as well as radio group made of buttons (as in bootstrap).

This will also enable #112 today and reduce the code for it.

• Other information
  This is generally how the directive should look like

import {
    Directive,
    Provider,
    forwardRef,
    Input,
    Output,
    EventEmitter
} from 'angular2/core';

import {
    NG_VALUE_ACCESSOR,
    ControlValueAccessor
} from 'angular2/src/common/forms/directives/control_value_accessor';

let nextId = 0;

const CHECKBOX_CONTROL_VALUE_ACCESSOR = new Provider(
    NG_VALUE_ACCESSOR, {
        useExisting: forwardRef(() => AsCheckbox),
        multi: true
    });


@Directive({
    selector: '[asCheckbox]',
    host: {
        'role': 'checkbox',
        '[id]': 'id',
        '[class.md-checkbox-checked]': 'checked',
        '[class.md-checkbox-disabled]': 'disabled',
        '[attr.tabindex]': 'disabled ? null : tabindex',
        '[attr.aria-checked]': 'ariaChecked',
        '[attr.aria-disabled]': 'disabled',
        '(click)': 'onInteractionEvent($event)',
        '(keydown.space)': 'onKeyDown($event)',
        '(keyup.space)': 'onInteractionEvent($event)',
        '(blur)': 'onTouched()'
    },
    providers: [CHECKBOX_CONTROL_VALUE_ACCESSOR]
})
export class AsCheckbox implements ControlValueAccessor {
    @Input() id: string = `md-checkbox-${++nextId}`;
    @Input() disabled: boolean = false;
    @Input() tabindex: number = 0;

    @Input() get checked(): boolean {
         return this._checked;
     }

     set checked(checked: boolean) {
         if (checked === this._checked) return;
         this._checked = checked;
         this.change.emit(this._checked);
     }

    get ariaChecked(): 'true' | 'false' {
        return this.checked ? 'true' : 'false';
    }

    @Output() change: EventEmitter<boolean> = new EventEmitter();

    private _checked: boolean = false;
    private _changeSubscription: {unsubscribe: () => any} = null;

    toggle() {
        this.checked = !this.checked;
    }

    onInteractionEvent(event: Event) {
        if (this.disabled) {
            event.preventDefault();
            event.stopPropagation();
            return;
        }
        this.toggle();
    }

    onKeyDown(evt: Event) {
        evt.preventDefault();
    }

    writeValue(value: any) {
        this.checked = !!value;
    }

    registerOnChange(fn: any) {
        if (this._changeSubscription) {
            this._changeSubscription.unsubscribe();
        }
        this._changeSubscription = <{unsubscribe: () => any}>this.change.subscribe(fn);
    }

    registerOnTouched(fn: any) {
        this.onTouched = fn;
    }

    onTouched(): any {}

}

From here, creating a toggle button is as simple as

<span class="md-button-wrapper" asCheckbox>Toggle me</span>

The code above was writting on the fly so it's just for the idea.
I'v implemented it in some components I work with not related to material, I can post a PR if you like the idea.

[shlomiassaf]

Notes:

• In my project I also use inheritance since a lot of the code for radio group and checkbox is similar.
• This is also true for implementing ControlValueAccessor, same code. maybe mixins can help here or not if it makes the code less readable.
• Indeterminate state left out, should be added on md-checkbox

[jelbourn]

This is an interesting thought on composable behavior. It's really more of a md-checkable than "checkbox" (you can also break down the behavior into, say, md-disableable and md-touchable).

I generally like this idea of extracting individual behaviors as building blocks for components. We've definitely talked about it before, and had been leaning on mixins as being the approach. Mixins make sense because, while all of these "checkable" components share some behavior, there is special behavior for each one, meaning that you'd end up building a custom component for each anyway. The result is easier to test and reuse.

Aside from toggle-buttons and slide-toggle, do you have any other use cases in mind for such a md-checkable directive? We already plan on both of these in the near future (#112, #90).

[shlomiassaf]

@jelbourn In my project I do use mixins, this is because I have a Checkable class that is good for both checkbox and radio-group and ControlValueAccessor mixin as well..

The reason I did not emphasise it was the goal's of this project, it is intended to be an example of how to do Angular 2. Mixins, while wonderful and powerful are a bit more difficult to understand as opposed to classical inheritance... One have to connect the dots, tough its not that bad, totally up to you.

As for more ideas, I have mentioned that checkbox is also used for radio, which in turn enables Exclusive selection in mdToggleButtonGroup with the help of RadioDispatcher

md-disableable and md-touchable are doable but they need to be both mixin classes and directives so in the code the md-checkbox will not use it in the template but mix it into itself. (avoid boilerplate)

This is quite a robust composition system :)

With your permission, I would like to offer my contribution with a small PR that will introduce a refactor for md-chackable directive and Checkable mixin class.
Once review iteration is done I will start introducing more composable units until we have full composition for radio and checkbox.

Is this an option?

Thanks, Shlomi

[jelbourn]

@shlomiassaf I'd rather not introduce a new checkable directive at this point; it's not clear to me yet that it would be very valuable to users. As far as refactoring the existing code into a mixin, @devversion may have already started on refactoring checkbox code out to be used in the new slide-toggle. I'm not sure what his status on that is at the moment.

[shlomiassaf]

Sure thing, let me know if you need my help

[jelbourn]

Now that TypeScript 2.2 supports mixins we will be exploring breaking out functionality into reusable pieces for the cdk. See #3944 for the first explorations into this space.

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}