angular / material

Material design for AngularJS
https://material.angularjs.org/
MIT License
16.55k stars 3.39k forks source link

ng-messages does not show message #6399

Closed amitport closed 8 years ago

amitport commented 8 years ago

reproduce:

1 - open http://codepen.io/topherfangio/pen/eJpoZN 2 - enter input>5 and focus out => error message appears 3 - clear the input and focus out => expected "required" message does not appear

Note that it seems that the message div is there but stuck on animation. Manually removing it's md-input-message-animation class in chrome devtools reveals the message.

0101adm commented 8 years ago

+1

edgaralves commented 8 years ago

+1

mpiasta-ca commented 8 years ago

I just came across this issue too. Thought I was doing something wrong in my project. So then I went to angular-material demo and replicated it: https://material.angularjs.org/latest/demo/input

Scroll to "Errors" section:

  1. In the "Description" field, enter a really long string like "This is a long string that exceeds the maximum"
  2. Now select-all and backspace/delete. Then blur from the field.
  3. The field is underlined in red because it has "required" error. But the required error message is not appearing.
FrancescoRizzi commented 8 years ago

But in the same demo block ("Errors"), the very next sample field ("Client Name", required) seems to work? eg: focus on it blur out of it required error appears (as expected) focus on it enter any value required error already disappear (as expected) you can even blur out and it's fine.

Could the problem be due to the char counter that appears in both the error scenarios (original code pen Test field, and Description field in the "Errors" demo)?

mpiasta-ca commented 8 years ago

@FrancescoRizzi, the issue appears when there are multiple ng-message error cases for an input field. The "Client Name" field only has a single ng-message error case. The issue happens when one ng-message is removed/hidden, and another ng-message block is supposed to be displayed; but in this case, the subsequent ng-message block is never displayed.

Zacharias3690 commented 8 years ago

+1, occurs for me when going from one invalid state to another. When going from invalid -> valid -> invalid, messages work as expected.

Zacharias3690 commented 8 years ago

I think I found the problem in input.js Line 642

function ngMessageAnimation($animateCss) {
  return {
    enter: function(element, done) {
      var messages = getMessagesElement(element);

      // If we have the md-auto-hide class, the md-input-invalid animation will fire, so we can skip
      if (messages.hasClass('md-auto-hide')) {
        done();
        return;
      }

      return showMessage(element, $animateCss);
    },

    leave: function(element, done) {
      return hideMessage(element, $animateCss);
    }
  }
}

When removing one class for another it looks like the md-input-invalid animation isn't called. Removing the if statement (ln 642-645) appears to resolve the issue.

topherfangio commented 8 years ago

Thanks for the comments everyone. We are working to add some tests to ensure that we properly fix this. Will keep everyone updated with our progress.

blackdynamo commented 8 years ago

+1

Here is a video actually demonstrating the issue: video

topherfangio commented 8 years ago

Quick update, we are still working on the proper test structure to make sure we fix this and other input issues and don't introduce regressions in the future. Unfortunately, animation tests are a bit tricky. We should have something soon.

0101adm commented 8 years ago

:+1:

ad0ran commented 8 years ago

+1

pochen6 commented 8 years ago

+1

ThomasBurleson commented 8 years ago

This issue is closed as part of our ‘Surge Focus on Material 2' efforts. For details, see our forum posting @ http://bit.ly/1UhZyWs.

bobwah commented 8 years ago

Getting really really sick of this kind of action

EladBezalel commented 8 years ago

@bobwah if you want to solve this bug you are more than welcome to suggest a PR.

devversion commented 8 years ago

@bobwah @EladBezalel The fix was quite often submitted. See for example: https://github.com/angular/material/pull/7583

topherfangio commented 8 years ago

@bobwah I am updating my branch which has some fixes/tests for this exact issue. Please follow #8864 for updates.

bobwah commented 8 years ago

Thanks @topherfangio I used the links above to patch the version of material we are using though I am sure at some point this will be updated to take a later version of main and i'll have to do the same again. I am sad what looks like a good fix you have made cannot be tested and released to main.

topherfangio commented 8 years ago

@bobwah I too wish it was easier to get into master, but if you look at the code, using Karma to do animation tests is very messy. Our only other option is to use Protractor for those, but while they are elegant, making that work on the CI seems to be pretty difficult and we're not sure if we want to do it.

Ultimately, I think we are going to try to get the fix in 1.1.0 even if we have to strip the tests out since this seems to clean up the code pretty well.

bobwah commented 8 years ago

I had to change ngMessageAnimation using the following version of material v1.1.0-rc.5-master-7f01138 instead of using the above code:

function ngMessageAnimation($animateCss) {
  return {
    enter: function(element, done) {
      var messages = getMessagesElement(element);
      var inputContainer = getInputElement(element);
      var input =  angular.element(inputContainer[0].querySelector('input'));

      if (messages.hasClass('md-auto-hide') && input.hasClass('ng-untouched')) {
        done();
        return;
      }

      return showMessage(element, $animateCss);
    },

    leave: function(element, done) {
      return hideMessage(element, $animateCss);
    }
  }
}
NateNjuguna commented 7 years ago

I've been bugged by this but turns out there is a simple logic missing in the if block to skip the message animation. If I am not wrong we should skip only if the md-auto-hide class exists on the ng-messages element and there is no md-input-invalid on the md-input-container. Otherwise the md-input-invaid animation won't handle our new enter event since no md-input-container class is present in the md-input-container. Therefore adding this logic (see code below) will rid ng-messages of the bug. PS: I tested this so I that I would know if it works. I'm also a newbie, so there might be a more efficient way to write this but anyone willing to help submit this can.

function ngMessageAnimation($animateCss) {
  return {
    enter: function(element, done) {
      var messages = getMessagesElement(element);

      // If we have the md-auto-hide class, the md-input-invalid animation will fire, so we can skip
      if (!getInputElement(element).hasClass('md-input-invalid') && messages.hasClass('md-auto-hide')) { // <= Here
        done();
        return;
      }

      return showMessage(element, $animateCss);
    },

    leave: function(element, done) {
      return hideMessage(element, $animateCss);
    }
  }
}

Update I checked and saw that this bug was fixed eons ago