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

Commit

Permalink
fix(autocomplete): Fix autocomplete items with spaces.
Browse files Browse the repository at this point in the history
If an autocomplete's item had spaces at the end, it would cause the
autocomplete to show only the selected option when attempting to
clear the value.

This was caused because the `oninput` event that we fire was somehow
resetting the value in the scope (after we had cleared it). Fix by
resetting the scope back to an empty value after we fire the `oninput`
event.

Additionally, fix a few issues in the Virtual Repeat that was affecting
the autocomplete and some test descriptions:

- Add unwatcher in Virtual Repeat to fix #8178.
- Use cached values in `$mdUtil.waitTransitionEnd()` to speed up performance
  and fix sizing issue in Virtual Repeat affecting the autocomplete.
- Fix a few descriptions in the tests to be shorter/clearer and fix typos.

Fixes #7655. Fixes #8178.

Closes #8580
  • Loading branch information
topherfangio authored and ThomasBurleson committed May 30, 2016
1 parent 92e932c commit 2fa2e4d
Show file tree
Hide file tree
Showing 6 changed files with 122 additions and 33 deletions.
102 changes: 77 additions & 25 deletions src/components/autocomplete/autocomplete.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ describe('<md-autocomplete>', function() {
}

describe('basic functionality', function() {
it('should update selected item and search text', inject(function($timeout, $mdConstant, $material) {
it('updates selected item and search text', inject(function($timeout, $mdConstant, $material) {
var scope = createScope();
var template = '\
<md-autocomplete\
Expand Down Expand Up @@ -103,8 +103,7 @@ describe('<md-autocomplete>', function() {
element.remove();
}));


it('should allow you to set an input id without floating label', inject(function() {
it('allows you to set an input id without floating label', inject(function() {
var scope = createScope(null, {inputId: 'custom-input-id'});
var template = '\
<md-autocomplete\
Expand All @@ -124,7 +123,7 @@ describe('<md-autocomplete>', function() {
element.remove();
}));

it('should allow allow using ng-readonly', inject(function() {
it('allows using ng-readonly', inject(function() {
var scope = createScope(null, {inputId: 'custom-input-id'});
var template = '\
<md-autocomplete\
Expand Down Expand Up @@ -153,7 +152,7 @@ describe('<md-autocomplete>', function() {
element.remove();
}));

it('should allow allow using an empty readonly attribute', inject(function() {
it('allows using an empty readonly attribute', inject(function() {
var scope = createScope(null, {inputId: 'custom-input-id'});
var template = '\
<md-autocomplete\
Expand All @@ -174,7 +173,7 @@ describe('<md-autocomplete>', function() {
element.remove();
}));

it('should allow you to set an input id with floating label', inject(function() {
it('allows you to set an input id with floating label', inject(function() {
var scope = createScope(null, {inputId: 'custom-input-id'});
var template = '\
<md-autocomplete\
Expand All @@ -195,7 +194,7 @@ describe('<md-autocomplete>', function() {
element.remove();
}));

it('should forward the `md-select-on-focus` attribute to the input', inject(function() {
it('forwards the `md-select-on-focus` attribute to the input', inject(function() {
var scope = createScope(null, {inputId: 'custom-input-id'});
var template =
'<md-autocomplete ' +
Expand All @@ -218,7 +217,7 @@ describe('<md-autocomplete>', function() {
element.remove();
}));

it('should forward the tabindex to the input', inject(function() {
it('forwards the tabindex to the input', inject(function() {
var scope = createScope(null, {inputId: 'custom-input-id'});
var template =
'<md-autocomplete ' +
Expand All @@ -240,7 +239,7 @@ describe('<md-autocomplete>', function() {
element.remove();
}));

it('should always set the tabindex of the autcomplete to `-1`', inject(function() {
it('always sets the tabindex of the autcomplete to `-1`', inject(function() {
var scope = createScope(null, {inputId: 'custom-input-id'});
var template =
'<md-autocomplete ' +
Expand Down Expand Up @@ -291,7 +290,7 @@ describe('<md-autocomplete>', function() {
element.remove();
}));

it('should clear value when hitting escape', inject(function($mdConstant, $timeout) {
it('clears the value when hitting escape', inject(function($mdConstant, $timeout) {
var scope = createScope();
var template = '\
<md-autocomplete\
Expand Down Expand Up @@ -380,7 +379,7 @@ describe('<md-autocomplete>', function() {
});

describe('basic functionality with template', function() {
it('should update selected item and search text', inject(function($timeout, $material, $mdConstant) {
it('updates selected item and search text', inject(function($timeout, $material, $mdConstant) {
var scope = createScope();
var template = '\
<md-autocomplete\
Expand Down Expand Up @@ -423,7 +422,60 @@ describe('<md-autocomplete>', function() {
element.remove();
}));

it('should compile the template against the parent scope', inject(function($timeout, $material) {
it('properly clears values when the item ends in a space character', inject(function($timeout, $material, $mdConstant) {
var myItems = ['foo ', 'bar', 'baz'].map(function(item) {
return {display: item};
});
var scope = createScope(myItems);

var template = '\
<md-autocomplete\
md-selected-item="selectedItem"\
md-search-text="searchText"\
md-items="item in match(searchText)"\
md-item-text="item.display"\
placeholder="placeholder">\
<md-item-template>\
<span md-highlight-text="searchText">{{item.display}}</span>\
</md-item-template>\
</md-autocomplete>';
var element = compile(template, scope);
var ctrl = element.controller('mdAutocomplete');
var ul = element.find('ul');

expect(scope.searchText).toBe('');
expect(scope.selectedItem).toBe(null);

$material.flushInterimElement();

// Focus the input
ctrl.focus();

element.scope().searchText = 'fo';
waitForVirtualRepeat(element);

expect(scope.searchText).toBe('fo');
expect(scope.match(scope.searchText).length).toBe(1);
expect(ul.find('li').length).toBe(1);

ctrl.keydown(keydownEvent($mdConstant.KEY_CODE.DOWN_ARROW));
ctrl.keydown(keydownEvent($mdConstant.KEY_CODE.ENTER));

$timeout.flush();

expect(scope.searchText).toBe('foo ');
expect(scope.selectedItem).toBe(scope.match(scope.searchText)[0]);

ctrl.clear();
$timeout.flush();

expect(scope.searchText).toBe('');
expect(scope.selectedItem).toBe(null);

element.remove();
}));

it('compiles the template against the parent scope', inject(function($timeout, $material) {
var scope = createScope(null, {bang: 'boom'});
var template =
'<md-autocomplete' +
Expand Down Expand Up @@ -470,7 +522,7 @@ describe('<md-autocomplete>', function() {
element.remove();
}));

it('should remove the md-scroll-mask on cleanup', inject(function($mdUtil, $timeout, $material) {
it('removes the md-scroll-mask on cleanup', inject(function($mdUtil, $timeout, $material) {
spyOn($mdUtil, 'enableScrolling');

var scope = createScope();
Expand Down Expand Up @@ -513,7 +565,7 @@ describe('<md-autocomplete>', function() {
expect($mdUtil.enableScrolling).toHaveBeenCalled();
}));

it('should ensure the parent scope digests along with the current scope', inject(function($timeout, $material) {
it('ensures the parent scope digests along with the current scope', inject(function($timeout, $material) {
var scope = createScope(null, {bang: 'boom'});
var template =
'<md-autocomplete' +
Expand Down Expand Up @@ -695,7 +747,7 @@ describe('<md-autocomplete>', function() {
expect(ctrl2.hasNotFound).toBe(false);
}));

it('should even show the md-not-found template if we have lost focus', inject(function($timeout) {
it('shows the md-not-found template even if we have lost focus', inject(function($timeout) {
var scope = createScope();
var template =
'<md-autocomplete' +
Expand Down Expand Up @@ -807,7 +859,7 @@ describe('<md-autocomplete>', function() {

describe('Async matching', function() {

it('should probably stop the loading indicator when clearing', inject(function($timeout, $material) {
it('properly stops the loading indicator when clearing', inject(function($timeout, $material) {
var scope = createScope();
var template =
'<md-autocomplete ' +
Expand Down Expand Up @@ -837,7 +889,7 @@ describe('<md-autocomplete>', function() {
});

describe('API access', function() {
it('should clear the selected item', inject(function($timeout) {
it('clears the selected item', inject(function($timeout) {
var scope = createScope();
var template = '\
<md-autocomplete\
Expand Down Expand Up @@ -871,7 +923,7 @@ describe('<md-autocomplete>', function() {
element.remove();
}));

it('should notify selected item watchers', inject(function($timeout) {
it('notifies selected item watchers', inject(function($timeout) {
var scope = createScope();
scope.itemChanged = jasmine.createSpy('itemChanged');

Expand Down Expand Up @@ -919,7 +971,7 @@ describe('<md-autocomplete>', function() {

element.remove();
}));
it('should pass value to item watcher', inject(function($timeout) {
it('passes the value to the item watcher', inject(function($timeout) {
var scope = createScope();
var itemValue = null;
var template = '\
Expand Down Expand Up @@ -955,7 +1007,7 @@ describe('<md-autocomplete>', function() {
});

describe('md-select-on-match', function() {
it('should select matching item on exact match when `md-select-on-match` is toggled', inject(function($timeout) {
it('selects matching item on exact match when `md-select-on-match` is toggled', inject(function($timeout) {
var scope = createScope();
var template = '\
<md-autocomplete\
Expand Down Expand Up @@ -1004,7 +1056,7 @@ describe('<md-autocomplete>', function() {
element.remove();
}));

it('should select matching item using case insensitive', inject(function($timeout) {
it('selects matching item using case insensitive', inject(function($timeout) {
var scope = createScope(null, null, true);
var template =
'<md-autocomplete ' +
Expand Down Expand Up @@ -1063,7 +1115,7 @@ describe('<md-autocomplete>', function() {
element.remove();
});

it('should validate an empty `required` as true', function() {
it('validates an empty `required` as true', function() {
var scope = createScope();
var template = '\
<md-autocomplete\
Expand All @@ -1082,7 +1134,7 @@ describe('<md-autocomplete>', function() {
expect(ctrl.isRequired).toBe(true);
});

it('should correctly validate an interpolated `ng-required` value', function() {
it('correctly validates an interpolated `ng-required` value', function() {
var scope = createScope();
var template = '\
<md-autocomplete\
Expand Down Expand Up @@ -1111,7 +1163,7 @@ describe('<md-autocomplete>', function() {
expect(ctrl.isRequired).toBe(true);
});

it('should forward the md-no-asterisk attribute', function() {
it('forwards the md-no-asterisk attribute', function() {
var scope = createScope();
var template = '\
<md-autocomplete\
Expand All @@ -1133,7 +1185,7 @@ describe('<md-autocomplete>', function() {
});

describe('md-highlight-text', function() {
it('should update when content is modified', inject(function() {
it('updates when content is modified', inject(function() {
var template = '<div md-highlight-text="query">{{message}}</div>';
var scope = createScope(null, {message: 'some text', query: 'some'});
var element = compile(template, scope);
Expand Down
13 changes: 9 additions & 4 deletions src/components/autocomplete/js/autocompleteController.js
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@ function MdAutocompleteCtrl ($scope, $element, $mdUtil, $mdConstant, $mdTheming,
/**
* Handles input focus event, determines if the dropdown should show.
*/
function focus () {
function focus($event) {
hasFocus = true;
//-- if searchText is null, let's force it to be a string
if (!angular.isString($scope.searchText)) $scope.searchText = '';
Expand Down Expand Up @@ -639,9 +639,9 @@ function MdAutocompleteCtrl ($scope, $element, $mdUtil, $mdConstant, $mdTheming,
/**
* Clears the searchText value and selected item.
*/
function clearValue () {
function clearValue ($event) {
// Set the loading to true so we don't see flashes of content.
// The flashing will only occour when an async request is running.
// The flashing will only occur when an async request is running.
// So the loading process will stop when the results had been retrieved.
setLoading(true);

Expand All @@ -652,9 +652,14 @@ function MdAutocompleteCtrl ($scope, $element, $mdUtil, $mdConstant, $mdTheming,

// Per http://www.w3schools.com/jsref/event_oninput.asp
var eventObj = document.createEvent('CustomEvent');
eventObj.initCustomEvent('input', true, true, { value: $scope.searchText });
eventObj.initCustomEvent('input', true, true, { value: '' });
elements.input.dispatchEvent(eventObj);

// For some reason, firing the above event resets the value of $scope.searchText if
// $scope.searchText has a space character at the end, so we blank it one more time and then
// focus.
elements.input.blur();
$scope.searchText = '';
elements.input.focus();
}

Expand Down
6 changes: 3 additions & 3 deletions src/components/autocomplete/js/autocompleteDirective.js
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ function MdAutocomplete () {
ng-keydown="$mdAutocompleteCtrl.keydown($event)"\
ng-blur="$mdAutocompleteCtrl.blur()"\
' + (attr.mdNoAsterisk != null ? 'md-no-asterisk="' + attr.mdNoAsterisk + '"' : '') + '\
ng-focus="$mdAutocompleteCtrl.focus()"\
ng-focus="$mdAutocompleteCtrl.focus($event)"\
aria-owns="ul-{{$mdAutocompleteCtrl.id}}"\
' + (attr.mdSelectOnFocus != null ? 'md-select-on-focus=""' : '') + '\
aria-label="{{floatingLabel}}"\
Expand All @@ -270,7 +270,7 @@ function MdAutocomplete () {
ng-model="$mdAutocompleteCtrl.scope.searchText"\
ng-keydown="$mdAutocompleteCtrl.keydown($event)"\
ng-blur="$mdAutocompleteCtrl.blur()"\
ng-focus="$mdAutocompleteCtrl.focus()"\
ng-focus="$mdAutocompleteCtrl.focus($event)"\
placeholder="{{placeholder}}"\
aria-owns="ul-{{$mdAutocompleteCtrl.id}}"\
' + (attr.mdSelectOnFocus != null ? 'md-select-on-focus=""' : '') + '\
Expand All @@ -284,7 +284,7 @@ function MdAutocomplete () {
type="button"\
tabindex="-1"\
ng-if="$mdAutocompleteCtrl.scope.searchText && !$mdAutocompleteCtrl.isDisabled"\
ng-click="$mdAutocompleteCtrl.clear()">\
ng-click="$mdAutocompleteCtrl.clear($event)">\
<md-icon md-svg-icon="md-close"></md-icon>\
<span class="_md-visually-hidden">Clear</span>\
</button>\
Expand Down
9 changes: 8 additions & 1 deletion src/components/showHide/showHide.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,19 @@ function createDirective(name, targetValue) {
var unregister = $scope.$on('$md-resize-enable', function() {
unregister();

var cachedTransitionStyles = window.getComputedStyle($element[0]);

$scope.$watch($attr[name], function(value) {
if (!!value === targetValue) {
$mdUtil.nextTick(function() {
$scope.$broadcast('$md-resize');
});
$mdUtil.dom.animator.waitTransitionEnd($element).then(function() {

var opts = {
cachedTransitionStyles: cachedTransitionStyles
};

$mdUtil.dom.animator.waitTransitionEnd($element, opts).then(function() {
$scope.$broadcast('$md-resize');
});
}
Expand Down
4 changes: 4 additions & 0 deletions src/components/virtualRepeat/virtual-repeater.js
Original file line number Diff line number Diff line change
Expand Up @@ -612,6 +612,10 @@ VirtualRepeatController.prototype.repeatListExpression_ = function(scope) {
VirtualRepeatController.prototype.containerUpdated = function() {
// If itemSize is unknown, attempt to measure it.
if (!this.itemSize) {
// Make sure to clean up watchers if we can (see #8178)
if(this.unwatchItemSize_ && this.unwatchItemSize_ !== angular.noop){
this.unwatchItemSize_();
}
this.unwatchItemSize_ = this.$scope.$watchCollection(
this.repeatListExpression,
angular.bind(this, function(items) {
Expand Down
Loading

0 comments on commit 2fa2e4d

Please sign in to comment.