webcat12345 / ngx-ui-switch

Angular UI Switch component
https://webcat12345.github.io/ngx-ui-switch/demo/
MIT License
110 stars 68 forks source link

disabled attribute questions #353

Open AlexanderLea opened 5 years ago

AlexanderLea commented 5 years ago

Hi,

We're using v8.0.1, and have had an issue today where we are setting

[disabled]="componentDisabled"

If we don't set componentDisabled in my component, and don't set a default value, the ngx-ui-switch appears to be defaulting to disabled=true.

The code inside ngx-ui-switch seems to be

@Input() set disabled(v: boolean) {
   this._disabled = v !== false;
 };

which suggests that anything other than explicit false will set disabled=true?

Is this the desired behaviour? - Am I missing something?

Alex

cmckni3 commented 5 years ago

interesting artifact from the original codebase. All of the explicit checks against false should be changed.

AlexanderLea commented 5 years ago

You accepting PRs?

cmckni3 commented 5 years ago

Yes

cmckni3 commented 4 years ago

I believe reverse is broken as well based on #382.

PRs welcome

cmckni3 commented 4 years ago

The code inside ngx-ui-switch seems to be

@Input() set disabled(v: boolean) { this._disabled = v !== false; }; which suggests that anything other than explicit false will set disabled=true?

Yes, that's correct but only when disabled is set at all.

      <!-- disabled=true (undefined value passed in) -->
      <ui-switch disabled></ui-switch>
      <!-- explicit disabled=true -->
      <ui-switch checked [disabled]="true"></ui-switch>

      <!-- disabled=false -->
      <ui-switch [disabled]="false"></ui-switch>

      <!-- componentDisabled = undefined => disabled=true -->
      <!-- componentDisabled = null => disabled=true -->
      <ui-switch [disabled]="componentDisabled"></ui-switch>

I am in favor of changing the check.

cmckni3 commented 4 years ago

It's a bit confusing. Maybe use the value that is passed into disabled(v: boolean) as truthy/falsy?

What do you think @AlexanderLea?

Chewieez commented 1 year ago

Is there a reason that disabling the ui-switch does not add the disabled attribute to the underlying html button element? Not adding it means that if you use the (click)="someMethod()" event with the toggle, it will still fire when clicked, even when disabled, and css cursor: not-allowed.

Changing to use the (change)=someMethod()" does not have that same behavior, but I'm curious if there is a reason the button itself is not being disabled.

cmckni3 commented 1 year ago

Is there a reason that disabling the ui-switch does not add the disabled attribute to the underlying html button element? Not adding it means that if you use the (click)="someMethod()" event with the toggle, it will still fire when clicked, even when disabled, and css cursor: not-allowed.

Changing to use the (change)=someMethod()" does not have that same behavior, but I'm curious if there is a reason the button itself is not being disabled.

Probably just overlooked when initially implemented. Never looked into adding it.

Chewieez commented 1 year ago

Probably just overlooked when initially implemented. Never looked into adding it.

Gotcha. I will look into creating a PR for adding it.

cmckni3 commented 1 year ago

Probably just overlooked when initially implemented. Never looked into adding it.

Gotcha. I will look into creating a PR for adding it.

Sounds good. Thanks!

cmckni3 commented 10 months ago

Found the issue here. Angular can pass the input as string in certain cases. working a fix

Related to #382