fix(autocomplete): Fix messages not appearing. #9909

Merged
merged 1 commit into from Nov 15, 2016

Projects

None yet

3 participants

@topherfangio
Contributor

In certain scenarios, the ng-messages of an autocomplete would fail to animate properly leaving them in a state where they were in the DOM, but not visible to the user.

This was ultimately caused by the animations not properly retrieving the messages element to animate, but was also tangentially caused by the animations not properly calling the done callback upon failure.

There are three associated fixes:

  1. Log a warning if the messages element or it's children cannot
    be found.
  2. Ensure the done callback always fires even on failures.
  3. Ensure the getMessagesElement() method correctly returns when
    the element passed IS the messages element itself.

Fixes #9468.

+ // that should NOT be exposed to the public but that should be tested.
+ //
+ // As an example, see input.js which exposes some animation-related methods.
+ window._mdMocksIncluded = true;
@topherfangio
topherfangio Oct 24, 2016 Contributor

I am open to alternate ways of doing this; if you have a better suggestion, I'd love to hear it!

@ThomasBurleson ThomasBurleson added this to the 1.1.2 milestone Oct 27, 2016
src/components/input/input.js
+ inputModule
+
+ // Register a service for each animation so that we can easily inject them into unit tests
+ .service('mdInputInvalidAnimation', mdInputInvalidMessagesAnimation)
@ThomasBurleson
ThomasBurleson Oct 27, 2016 Contributor

Let's just use this special internal accessor:

$$mdInput = { 
  // special accessor to internals... useful for testing
  messages {
    show        : showInputMessages
    hide        : hideInputMessages,
    getElement  : getMessagesElement,    
    animation : {      
      invalid   : mdInputInvalidMessagesAnimations,
      message   : ngMessageAnimation
      messages  : ngMessagesAnimations,
     }    
  }
}
@ThomasBurleson
Contributor

@topherfangio - can you fix this plz so we can merge it.

@topherfangio topherfangio fix(autocomplete): Fix messages not appearing.
In certain scenarios, the `ng-messages` of an autocomplete would
fail to animate properly leaving them in a state where they were
in the DOM, but not visible to the user.

This was ultimately caused by the animations not properly retrieving
the messages element to animate, but was also tangentially caused by
the animations not properly calling the `done` callback upon failure.

There are three associated fixes:

 1. Log a warning if the messages element or it's children cannot be
    found.
 2. Ensure the `done` callback always fires even on failures.
 3. Ensure the `getMessagesElement()` method correctly returns when
    the element passed IS the messages element itself.

Fixes #9468.
289793b
@topherfangio
Contributor

@ThomasBurleson Updated. I can use this for the test methods (and have done so), but for the animations, I need the injector to retrieve my dependencies.

Will this work?

@ThomasBurleson
Contributor

@topherfangio - lgtm

@kara kara added needs: merge and removed needs: presubmit labels Nov 15, 2016
@kara kara merged commit ce5f7c2 into angular:master Nov 15, 2016

2 checks passed

cla/google All necessary CLAs are signed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment