vue-toggle-btn icon indicating copy to clipboard operation
vue-toggle-btn copied to clipboard

use parent events and value on checkbox

Open davidmeirlevy opened this issue 5 years ago • 6 comments

  1. allows user to use "v-model" with all it's modifiers (because value binding and all related events are valid).
  2. reduce code, because now the component doesn't need to emit the change event manually.
  3. specify props types.
  4. add ability to add "name" attribute to the checkbox - needed for some forms.

davidmeirlevy avatar Jun 30 '19 07:06 davidmeirlevy

  1. I do not really see a use case for this. There is no point in passing to the toggle button a string it only is supposed to receive a boolean - isActive

  2. Working with Input value instead of isActive is interesting, Could you elaborate on what value the $listeners provides, what does it do?

3 is perfectly clear and I agree on it being a better addition.

  1. Can you elaborate on this, why should I need a name attribute - for accessibility?

JonathanDn avatar Jul 21 '19 06:07 JonathanDn

  1. I changed the "isActive" to "value" because when you use "value" prop name - you can also pass it as v-model, so users can use this component using "v-model".

  2. you don't need to manage the input events and triggers at all. The user can register to all of the input native events, such as focus, change, input, etc.. Also, when users are using "v-model", they expecting the "input" or "change" events (depends if they are using "lazy" modifier or not).

  3. thanks.

  4. the "name" is used for native html forms. There are several cases that it's recommended to add name attribute to inputs.

davidmeirlevy avatar Jul 21 '19 07:07 davidmeirlevy

Alright so to wrap up:

  • isActive is changed to value prop (please update the readme documentation according)

  • because of that change consumers of the component can now use v-model - (Can you think of where in the documentation this information would be useful, maybe as a code example? if so feel free to add it to the readme)

  • The docs are addressing the custom events - please adjust them to the relevant 'input' and 'change' events so it would be clear for consumers of the component.

  • same about 'name' prop now exposed as part of the API - needs to be documented.

JonathanDn avatar Jul 21 '19 08:07 JonathanDn

Readme file has changed. please review.

davidmeirlevy avatar Jul 21 '19 10:07 davidmeirlevy

readme updated. please review again.

davidmeirlevy avatar Jul 23 '19 07:07 davidmeirlevy

@JonathanDn ?

davidmeirlevy avatar Oct 31 '19 12:10 davidmeirlevy