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

Commit

Permalink
Bug fix - Search input isn't blocked (#1822)
Browse files Browse the repository at this point in the history
* fixed a bug: User input keys to search input isn't blocked when search-enabled is set to false and all items are selected in multiple select mode.

* Revert "fixed a bug: User input keys to search input isn't blocked when search-enabled is set to false and all items are selected in multiple select mode."

This reverts commit ffcf66c.

* fixed a bug: User input keys to search input isn't blocked when search-enabled is set to false and all items are selected in multiple select mode.
  • Loading branch information
nabilnaffar authored and aaronroberson committed Oct 19, 2016
1 parent 10a60fa commit 0d81493
Showing 1 changed file with 4 additions and 1 deletion.
5 changes: 4 additions & 1 deletion src/uiSelectController.js
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,7 @@ uis.controller('uiSelectCtrl',
ctrl.select = function(item, skipFocusser, $event) {
if (item === undefined || !_isItemDisabled(item)) {

if ( ! ctrl.items && ! ctrl.search && ! ctrl.tagging.isActivated) return;
if ( (!ctrl.items || !ctrl.items.length) && ! ctrl.search && ! ctrl.tagging.isActivated) return;

if (!item || !_isItemDisabled(item)) {
// if click is made on existing item, prevent from tagging, ctrl.search does not matter
Expand Down Expand Up @@ -645,6 +645,9 @@ uis.controller('uiSelectCtrl',
});
}
}
}else{
e.preventDefault();

This comment has been minimized.

Copy link
@schandra-rpx

schandra-rpx Oct 20, 2016

preventDefault and stopPropagation block user to type characters. for any async operations this is a blocker.

This comment has been minimized.

Copy link
@chadfurman

chadfurman Oct 22, 2016

Agreed. This prevents me from hitting the "backspace" key when the search returns an empty list

This comment has been minimized.

Copy link
@merarischroeder

merarischroeder Oct 22, 2016

Broken for me too. This else block shouldn't be here IMO

e.stopPropagation();
}

});
Expand Down

6 comments on commit 0d81493

@narrowtux
Copy link

@narrowtux narrowtux commented on 0d81493 Oct 21, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You just broke our production app with this "fix". At least bump your minor version instead of pushing it in a patch.

The search-enabled property doesn't seem to have a default value and as such this was probably null or false for us.

What happened was that npm installed the 0.19.5 version (because it matches to ^0.19.4). Because I developed the box with 0.19.4 I didn't know of the bug and as such it broke my working thing.

@pasha-r
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guys this "fix" breaks completely different scenario:
search-enabled is true
and it's not a multiple select
And you are typing something that empties ctrl.items.length

Everything is blocked, user can't even correct input - backspace is blocked as well

@bioub
Copy link

@bioub bioub commented on 0d81493 Oct 21, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as @narrowtux and @pasha-r this breaks the ability to write or delete anything

@narrowtux
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly, this just removes the ability to type anything (even switching tabs in chrome), which isn't the way I would solve disabling an input. Wouldn't it be better to set disabled="disabled" on the <input>?

@chadfurman
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also a problem with 0.19.4 -- either that, or the version number in the comment atop the file didn't change: https://github.com/angular-ui/ui-select/blob/master/dist/select.js#L925

@chadfurman
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not in 0.19.4 - the file comment is wrong. Easy fix is to downgrade

Please sign in to comment.