Famous / famous-angular

Bring structure to your Famo.us apps with the power of AngularJS. Famo.us/Angular integrates seamlessly with existing Angular and Famo.us apps.
https://famo.us/angular
Mozilla Public License 2.0
1.92k stars 275 forks source link

Disabled button inside fa-surface still raise ngClick event #261

Closed jbuiss0n closed 9 years ago

jbuiss0n commented 9 years ago

Hello,

I faced a strange issue, when a <button> is inside an <fa-surafce>, it always raise the ngClick event, even if the button is disabled usin ng-disabled.

Here is the guilty code:

  <fa-modifier fa-size="[undefined, 40]" fa-origin="[.5, .5]" fa-align="[.5, 1]" fa-translate="[0, -30, 5]">
    <fa-modifier fa-size="[width - 2 * 20, 40]" fa-translate="[20, -50, 10]">
      <fa-surface class="capture">
        <button class="btn bg-grey-darken"
                ng-disabled="!isValid()"
                ng-click="goNextStep()" active-link>
          Click me
        </button>
      </fa-surface>
    </fa-modifier>

When isValid() returns false, button is clearly visible as disabled, but if you click on it, it still call the goNextStep() method.

jmeiss commented 9 years ago

Thanks the report @jbuiss0n.

@zackbrown it seems to be related to the fix I did. About ng-disabled but obviously, I didn't fixed the whole bug.

Thanks for your help :)

jbuiss0n commented 9 years ago

As i continue my investigation, i find this piece of code in famous-angular:

  window.addEventListener('touchend', function(event) {
      var currTime = _now();
      for (var i = 0; i < event.changedTouches.length; i++) {
          var touch = event.changedTouches[i];
          var startTime = potentialClicks[touch.identifier];
          if (startTime && currTime - startTime < clickThreshold) {
              var clickEvt = new window.CustomEvent('click', {
                  'bubbles': true,
                  'detail': touch
              });
              recentlyDispatched[currTime] = event;
              event.target.dispatchEvent(clickEvt);
          }
          delete potentialClicks[touch.identifier];
      }
  });

I don't know if event.target.dispatchEvent(clickEvt); should handle the disabled check, or not, the probleme may come from here ?

jbuiss0n commented 9 years ago

Last discovery from my investigation, if there is an ng-click attribute set for the fa-surface, the button inside will not raise it's own ng-click event (if the button is disbaled obviously). But if you remove the fa-surface ng-click event, the button will raise it's own event, event if it is disabled.

jordanpapaleo commented 9 years ago

Hi @jbuiss0n -

Is this still a valid issue? I just created a codepen for this and I am not able to reproduce the described behavior. Maybe its already been fixed. Can you please update the pen to a state where it is failing so I can better investigate your issue?

Thanks,

Jordan

jbuiss0n commented 9 years ago

Hello,

After some tests, it seems the bug was not really one... I think it's a misuse from us, we know only use fa-click on fa-surface (no button, or else) and everything looks good.

(I cannot see your codepen, it's blank for me).

Thanks for your help