Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

[dropdown] Not unbinding keydown handler properly when closing dropdown by opening another one #6208

Closed
Agamnentzar opened this issue Aug 26, 2016 · 3 comments

Comments

@Agamnentzar
Copy link

Keydown handle for dropdowns is not being unbinded correctly when dropdown is closed by opening another dropdown.

http://plnkr.co/edit/wQ0j12hABSNz6pYEqRQL?p=preview

Steps to reproduce

  1. Open plunker, open dev console
  2. Click on first dropdown to open it
  3. Click on second dropdown to open it (without closing the first one first)
  4. Click on the background to close second dropdown
  5. Start pressing keyboard buttons
  6. dev console will show Uncaught TypeError: Cannot read property 'getDropdownElement' of null errors on each key press

Angular: 1.5.5
UIBS: 2.1.3
Bootstrap: 3.3.6

Probably introduced by 6bad759

@almothafar
Copy link

almothafar commented Sep 8, 2016

+1
Angular: 1.5.8
Bootstrap: 3.3.7

and without open another one, I just open the drop down then click anywhere, and start typing making this, if I open drop down, select something and type there is no issue.

simply the HTML like the following code :

<div class="form-group"
     ng-class="{'has-error': $ctrl.forms.newUserForm.mobile.$invalid && $ctrl.forms.newUserForm.mobile.$dirty}">
  <label for="mobile" class=" col-md-3">Mobile No.</label>

  <div class="col-md-7">
    <input name="mobile" required ng-model="$ctrl.user.mobile" id="mobile" type="text"
           class="form-control"
           placeholder="Mobile Number" aria-describedby="basic-addon9">
  </div>
</div>


<div class="form-group">
  <label class=" col-md-3">Permission</label>

  <div class="col-md-7">
    <div class="btn-group" uib-dropdown>
      <button type="button" class="btn btn-default" uib-dropdown-toggle>
        {{$ctrl.user.permission}} <label class="caret"></label>
      </button>
      <ul uib-dropdown-menu role="menu">
        <li ng-repeat="value in $ctrl.permissions">
          <a ng-click="$ctrl.user.permission=value"
             class="pointer-on-mouse">{{value}}</a>
        </li>
      </ul>
    </div>
  </div>
</div>

@leon3s
Copy link

leon3s commented Oct 4, 2016

Hello there !

I got the same error and that was my way to fix it.

The error pop on function keybindFilter

  this.keybindFilter = function(evt) {
    var dropdownElement = openScope.getDropdownElement();
    var toggleElement = openScope.getToggleElement();
    var dropdownElementTargeted = dropdownElement && dropdownElement[0].contains(evt.target);
    var toggleElementTargeted = toggleElement && toggleElement[0].contains(evt.target);
    if (evt.which === 27) {
      evt.stopPropagation();
      openScope.focusToggleElement();
      closeDropdown();
    } else if (openScope.isKeynavEnabled() && [38, 40].indexOf(evt.which) !== -1
         && openScope.isOpen && (dropdownElementTargeted || toggleElementTargeted)) {
      evt.preventDefault();
      evt.stopPropagation();
      openScope.focusDropdownEntry(evt.which);
    }
  };

When we close dropDown this is called

  this.close = function(dropdownScope, element) {
    if (openScope === dropdownScope) {
      openScope = null;
      $document.off('click', closeDropdown);
      $document.off('keydown', this.keybindFilter);
    }
  };

openScope is set to null before use it on this.keybindFilter WTF ???
I fixed it like that but it maybe not the best way

  this.keybindFilter = function(evt) {
    if (!openScope) { return; }
    var dropdownElement = openScope.getDropdownElement();
    var toggleElement = openScope.getToggleElement();
    var dropdownElementTargeted = dropdownElement && dropdownElement[0].contains(evt.target);
    var toggleElementTargeted = toggleElement && toggleElement[0].contains(evt.target);
    if (evt.which === 27) {
      evt.stopPropagation();
      openScope.focusToggleElement();
      closeDropdown();
    } else if (openScope.isKeynavEnabled() && [38, 40].indexOf(evt.which) !== -1
         && openScope.isOpen && (dropdownElementTargeted || toggleElementTargeted)) {
      evt.preventDefault();
      evt.stopPropagation();
      openScope.focusDropdownEntry(evt.which);
    }
  };

It's probably better set openScope to null at the end of keybindFilter ?
Please fix it <3

nmccready added a commit to realtymaps/bootstrap that referenced this issue Oct 7, 2016
@nmccready
Copy link
Contributor

#6278

@wesleycho wesleycho added this to the 2.2.0 milestone Oct 10, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants