xpepermint / angular-ui-switch

On/off switch button for AngularJS. DEPRECATED!
278 stars 127 forks source link

ng-click seems to cause problems #28

Open iJungleboy opened 8 years ago

iJungleboy commented 8 years ago

I tried adding an event on ng-click but I always end up gettin angular-js errors. anybody else too?

craiggoldstone commented 8 years ago

Same. Frustrating!

ilyes-garifullin commented 8 years ago

+

barnesm999 commented 8 years ago

Not sure of your particular use case, but could you just ng-change to listen for changes in the buttons state? The switch itself should already be listening for the ng-click and changing the state automatically so ng-change should be able to listen for the change and then execute something as a result. That is how I use it.

If you want to use ng-click to make sure the switch should actually be clicked before changing the state, maybe consider putting a disable listener on the element instead.

If you definitely need an ng-click, maybe you could wrap the switch in a span and put ng-click on that span. But be careful with this as you want to make sure this click propagates down to the actual switch.

Zemke commented 7 years ago

An easy and quick workaround is ending your ngClick expression with ; true ||.

Why does that work? The switch directive itself applies the ngClick directive on it:

' ng-click="' + attrs.disabled + ' ? ' + attrs.ngModel + ' : ' + attrs.ngModel + '=!' + attrs.ngModel + (attrs.ngChange ? '; ' + attrs.ngChange + '()"' : '"')

which translates to this in a readable format:

if (attrs.disabled) {
    attrs.ngModel; // noop
} else {
    attrs.ngModel =! attrs.ngModel;

    if (attrs.ngChange) {
        attrs.ngChange();
    }
}

As you see, a simple solution would be to disable the switch. But you don’t want that. It alters other behavior of the switch. You only want to have an ngClick without further consequences and before the model updates.

When you attach ngClick yourself, it will simply be prepended to it, thus causing a syntax error. For example:

<switch ng-model="$ctrl.thisIsMyModel"
        ng-click="$ctrl.thisIsMyNgClick()">
</switch>

results in this ngClick:

$ctrl.thisIsMyNgClick() undefined ? $ctrl.thisIsMyModel : $ctrl.thisIsMyModel=!$ctrl.thisIsMyModel
                      ^ Your ngClick ends here and
                        there's the syntax error

If you’re wondering what that undefined is about, compare this resulting ngClick with how the switch directive assembles the ngClick (look at the beginning of my message). You’ll see that undefined is the ngChange attribute which simply wasn’t set.

Now, you’re probably already getting the idea. If you append ; true || to your ngClick definition like this

<switch ng-model="$ctrl.thisIsMyModel"
        ng-click="$ctrl.thisIsMyNgClick(); true ||">
</switch>

the switch directive will be assembled like this

$ctrl.thisIsMyNgClick(); true || undefined ? $ctrl.thisIsMyModel : $ctrl.thisIsMyModel=!$ctrl.thisIsMyModel

which translates to

$ctrl.thisIsMyNgClick();

if (true || attrs.disabled) {
    attrs.ngModel; // noop
} else {
    attrs.ngModel =! attrs.ngModel;

    if (attrs.ngChange) {
        attrs.ngChange();
    }
}

Please, notice two shortcomings. Firstly, you’d need to update the model yourself (i.e. in $ctrl.thisIsMyNgClick()). Secondly, this is rather hacky and I think we’re talking about a bug in this directive. I don’t see why this directive would make use of replace option and kind of override ngClick functionality. A more future-proof approach is to make it a component and correctly implementing NgModelController.

Zemke commented 7 years ago

That ; true || does still throw an error, because it’s an incorrectly terminated expression. One way I figure is to do some verbose counter-intuitive “state-setting”. But it is valid and not as crude.

<switch ng-model="$ctrl.thisIsMyModel"
        ng-click="$ctrl.disabled = true;"
        ng-change="$ctrl.thisIsMyNgClick()"
        disabled="$ctrl.disabled">
</switch>

It makes sense given the explanation I’ve given before.

ebdellikhaled commented 6 years ago

; will solve the error <switch ng-model="thisIsMyModel" ng-click="thisIsMyNgClick();"> </switch>

Marcevalmon commented 5 years ago

@ebdellikhaled image