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

fix(#271) if option is set the refresh function is getting the request. #1845

Merged
merged 11 commits into from
Jan 25, 2017
Merged

fix(#271) if option is set the refresh function is getting the request. #1845

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
84420c3
fix(#271) if option is set only on activate the refresh function is g…
Jefiozie Nov 3, 2016
d137ded
Instead of new var used the open variable as it is set on activate.
Jefiozie Nov 3, 2016
57f612a
Changed so that attr is removed and watch is checkin select.open
Jefiozie Nov 7, 2016
91e48dd
Reverted: removed broadcast event
Jefiozie Nov 7, 2016
2a8509f
changed watch of search as the initial value should be ignored on load
Jefiozie Nov 7, 2016
a52f0b2
reverted the watch function for the desired use cases.
Jefiozie Nov 8, 2016
d7a5df2
Added test for initial refresh call
Jefiozie Nov 8, 2016
98c179a
removed some whitespace
Jefiozie Nov 8, 2016
7531d29
Revert "Added test for initial refresh call"
Jefiozie Nov 9, 2016
119b072
removed empty line
Jefiozie Nov 9, 2016
914f2d8
Merge branch 'master' into 625_refreshOnActive
Jefiozie Jan 16, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions src/uiSelectChoicesDirective.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,8 @@ uis.directive('uiSelectChoices',


$select.parseRepeatAttr(attrs.repeat, groupByExp, groupFilterExp); //Result ready at $select.parserResult

$select.disableChoiceExpression = attrs.uiDisableChoice;
$select.onHighlightCallback = attrs.onHighlight;

$select.dropdownPosition = attrs.position ? attrs.position.toLowerCase() : uiSelectConfig.dropdownPosition;

scope.$on('$destroy', function() {
Expand All @@ -68,7 +66,7 @@ uis.directive('uiSelectChoices',
scope.$watch('$select.search', function(newValue) {
if(newValue && !$select.open && $select.multiple) $select.activate(false, true);
$select.activeIndex = $select.tagging.isActivated ? -1 : 0;
if (!attrs.minimumInputLength || $select.search.length >= attrs.minimumInputLength) {
if ((!attrs.minimumInputLength || $select.search.length >= attrs.minimumInputLength) && $select.open) {
Copy link
Contributor

Choose a reason for hiding this comment

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

How come you changed this? I didn't think it would be needed? Does reverting this fix tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed this as this will prevent the initial request of the refresh function on initializing.
Yes reverting will fix the tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

So we can revert this then. 😁

From my comment a few days ago:

the user can check manually in fetchFromServer(...) if select is open

Copy link
Contributor Author

@Jefiozie Jefiozie Nov 7, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Based on your comment yes we can revert. But still on initializing the refresh will be exectued. And this is not the behavior as it should be i think and also the problem of #271 ?

Copy link
Contributor

Choose a reason for hiding this comment

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

As I said before I want the user to check if it's open not us, users should already be able to do this. (I'm writing on mobile so my replies are short sorry!).

I also don't want to add breaking change as some users might like to load on initialise.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, if that is the case then we can revert almost everything. Only thing we need to add is when activating/focus the control we need to execute the refreshfunction.

The users can already check if it is open by adding (as you already mentioned) a second param $select.open

Copy link
Contributor

Choose a reason for hiding this comment

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

Yup 😁 if I could add functionality while reducing lines of code I would! 😛 Hopefully we both learned something more about ui-select though 😂

$select.refresh(attrs.refresh);
} else {
$select.items = [];
Expand All @@ -80,10 +78,11 @@ uis.directive('uiSelectChoices',
var refreshDelay = scope.$eval(attrs.refreshDelay);
$select.refreshDelay = refreshDelay !== undefined ? refreshDelay : uiSelectConfig.refreshDelay;
});

scope.$watch('$select.open', function(open) {
if (open) {
tElement.attr('role', 'listbox');
$select.refresh(attrs.refresh);
} else {
tElement.removeAttr('role');
}
Expand Down
5 changes: 0 additions & 5 deletions src/uiSelectController.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,11 +115,8 @@ uis.controller('uiSelectCtrl',
if(!avoidReset) _resetSearchInput();

$scope.$broadcast('uis:activate');

ctrl.open = true;

ctrl.activeIndex = ctrl.activeIndex >= ctrl.items.length ? 0 : ctrl.activeIndex;

// ensure that the index is set to zero for tagging variants
// that where first option is auto-selected
if ( ctrl.activeIndex === -1 && ctrl.taggingLabel !== false ) {
Expand Down Expand Up @@ -257,8 +254,6 @@ uis.controller('uiSelectCtrl',
if (ctrl.dropdownPosition === 'auto' || ctrl.dropdownPosition === 'up'){
$scope.calculateDropdownPos();
}

$scope.$broadcast('uis:refresh');
};

// See https://github.com/angular/angular.js/blob/v1.2.15/src/ng/directive/ngRepeat.js#L259
Expand Down
16 changes: 15 additions & 1 deletion test/select.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,8 @@ describe('ui-select tests', function() {
if (attrs.refresh !== undefined) { choicesAttrsHtml += ' refresh="' + attrs.refresh + '"'; }
if (attrs.refreshDelay !== undefined) { choicesAttrsHtml += ' refresh-delay="' + attrs.refreshDelay + '"'; }
if (attrs.backspaceReset !== undefined) { attrsHtml += ' backspace-reset="' + attrs.backspaceReset + '"';}
if (attrs.refreshOnActive !== undefined) { choicesAttrsHtml += ' refresh-on-active="' + attrs.refreshOnActive + '"'; }

}

return compileTemplate(
Expand Down Expand Up @@ -3119,5 +3121,17 @@ describe('ui-select tests', function() {
expect(el.scope().$select.spinnerClass).toBe('randomclass');
});
});


describe('With refresh on active', function(){
it('should not refresh untill is activated', function(){
scope.fetchFromServer = function(){};
var el = createUiSelect({refresh:"fetchFromServer($select.search)",refreshOnActive:true,refreshDelay:0});
spyOn(scope, 'fetchFromServer');
$timeout.flush();
expect(scope.fetchFromServer.calls.any()).toEqual(false);
el.scope().$select.activate();
$timeout.flush();
expect(scope.fetchFromServer.calls.any()).toEqual(true);
});
});
});