Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

md-autocomplete freezes UI when removed while loading items. #8358

Closed
mgilson opened this issue May 6, 2016 · 5 comments
Closed

md-autocomplete freezes UI when removed while loading items. #8358

mgilson opened this issue May 6, 2016 · 5 comments

Comments

@mgilson
Copy link
Contributor

mgilson commented May 6, 2016

There are a number of other issues which seem to be related, but they're all closed:
#7634
#3287
#3334
#3371 <--- closest that I can figure...

Actual behavior:

  • What is the issue?*
  • It seems that if the autocomplete gets removed from the UI while it is still loading items, the associated .md-scroll-mask persists and eats mouse/keyboard events making the UI seem unresponsive.
  • What is the expected behavior?*
  • The .md-scroll-mask should go away when the autocomplete does...

CodePen or Steps to reproduce the issue: *

  1. Open the pen.
  2. Start typing (slowly) in the autocomplete. I usually use "alaska". I usually get to about "al" before the autocomplete disappears.
  3. click the switch to bring the autocomplete back.
  4. refocus the autocomplete and keep adding to your state (slowly).
  5. Continue with steps 3 and 4 until the switch can no longer be clicked due to the presence of the .md-scroll-mask

Angular Versions: *

  • Angular Version:*
  • 1.4.8
  • Angular Material Version:*
  • 1.0.8

Additional Information:

  • Browser Type:*
  • Chrome
  • Browser Version:*
  • 49.0.2623.112
  • OS:*
  • OS-X 10.11.2
  • Stack Traces:*
  • None in the console.

Shortcut to create a new CodePen Demo.
Note: * indicates required information. Without this information, your issue may be auto-closed.

Do not modify the titles or questions. Simply add your responses to the ends of the questions.
Add more lines if needed.

@RolandHeath
Copy link

RolandHeath commented May 6, 2016

I tried updating your codepen to 1.1.0-rc3, the issue is definitely still around.
Also from my testing, you don't need to type anything into the autocomplete.
It will occur if you just

  1. Click to focus the autocomplete, wait for it to disappear
  2. Click the switch to bring it back
  3. Repeat

After a closer look with the dev tools, it seems that in the autocomplete controller ctrl.hidden is set to true before the $destroy event occurs, which means that the bug is in autocomplete, not in $mdutil.

I'm fairly confident I've narrowed down the bug to one of the following two places.
The function that sets ctrl.hidden, shouldHide() line 547-552
The call to $mdUtil.enableScrolling() in the autocomplete cleanup function line 182-184

The other possibility is that shouldHide isn't being called in all the situations that it should be, so that ctrl.hidden isn't up to date.

@mgilson
Copy link
Contributor Author

mgilson commented May 6, 2016

It seems like ctrl.hidden is being used as a proxy for whether or not the scroll has been disabled. Apparently that isn't the case. I'm not 100% sure that I know the code well enough to speculate, but it seems like it would be more robust to wrap $mdUtil.disableScrollAround and $mdUtil.enableScrolling:

var isScrollDisabled = false;

function disableScroll() {
  // Abort if isScrollDisabled?
  isScrollDisabled = true;
  $mdUtil.disableScrollAround();
}

function enableScroll() {
  // Check if isScrollDisabled?
  isScrollDisabled = false;
  $mdUtil.enableScrolling();
}

Then you would just replace the respective mdUtil scroll functions with these replacements that track the state...

@mgilson
Copy link
Contributor Author

mgilson commented May 6, 2016

I've dug around on this a little more today after getting angular material to build and I think I have a cursory understanding of the problem. It looks like $mdUtil.disableScrollAround() is getting called after cleanUp. This is likely because it is registered in a $mdUtil.nextTick (line 249-252)

One solution is to have a flag and to not call that method if the scope has been destroyed...

Note that I think we might also be able to consider this is bug in $mdUtil.nextTick. nextTick accepts 3 arguments (callback, digest and scope). There is no documentation on the scope param, but based on it's usage in processQueue (line 609), it looks like the purpose of the scope parameter is to prevent callbacks on destroyed scopes from executing. Unfortunately, this doesn't seem to work as intended. Here's why:

  • For each "tick", $mdUtil.nextTick can be called multiple times (registering multiple functions).
  • These functions can be registered from anywhere, so they aren't necessarily called with the same scope argument.
  • When the first function for that tick is registered, it's processQueue is the one that will execute.
  • The registered processQueue picks up at most 1 scope from the closure (so all other scopes which were registered during this "tick" will be dropped and only the function will be called).

@zsedem
Copy link

zsedem commented May 12, 2016

Hi, I think this is related:
We use the autocomplete menu to change to a user page when a user is selected from the list.

function selectedItemChange(user) {
     $location.url('/users/' + user.id);
}

When the page changes the div with md-scroll-mask is still in the html.
I implemented a workaround in our directive, like this:

$scope.$on('$destroy', $mdUtil.enableScrolling);

@mgilson
Copy link
Contributor Author

mgilson commented May 12, 2016

Yeah, I'm guessing that it is related. It sounds like similar circumstances for when we saw this bug on our team. Our current workaround is to decorate $mdUtil to fix the broken nextTick like in the associated pull-request:

/**
 * Decorate the angular-material util service to fix the md-mask bug.
 * TODO(team): remove this when angular-material fixes this bug.
 * @param {!angular.$provide} $provide
 * @ngInject
 */
var duckPunch$mdUtil = function($provide) {
  /**
   * @param {!Object<string, function(...)>} $delegate
   * @param {!angular.Scope} $rootScope
   * @param {!angular.$timeout} $timeout
   * @return {!Object<string, function(...)>}
   * @ngInject
   */
  var decorate$mdUtil = function($delegate, $rootScope, $timeout) {
    $delegate.nextTick = function(callback, digest, scope) {
      //-- grab function reference for storing state details
      var nextTick = $delegate.nextTick;
      var timeout = nextTick.timeout;
      var queue = nextTick.queue || [];

      //-- add callback to the queue
      queue.push({scope: scope, callback: callback});

      //-- set default value for digest
      if (digest == null) digest = true;

      //-- store updated digest/queue values
      nextTick.digest = nextTick.digest || digest;
      nextTick.queue = queue;

      //-- either return existing timeout or create a new one
      return timeout || (nextTick.timeout = $timeout(processQueue, 0, false));

      /**
       * Grab a copy of the current queue
       * Clear the queue for future use
       * Process the existing queue
       * Trigger digest if necessary
       */
      function processQueue() {
        var queue = nextTick.queue;
        var digest = nextTick.digest;

        nextTick.queue = [];
        nextTick.timeout = null;
        nextTick.digest = false;

        queue.forEach(function(queueItem) {
          var skip = queueItem.scope && queueItem.scope.$$destroyed;
          if (!skip) {
            queueItem.callback();
          }
        });

        if (digest) $rootScope.$digest();
      }
    };
    return $delegate;
  };

  $provide.decorator('$mdUtil', decorate$mdUtil);
};

var app = angular.module('Application', [
  'ngMaterial',
  ...
]).config(duckPunch$mdUtil);

of course, normal injection rules apply (we use the closure-compiler, so the @ngInject annotations take care of it for us).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants