axel-zarate/js-custom-select

Not showing text on bind collection after set ngModel value

Opened this issue ยท 13 comments

This tool is very helpful, but I got one issue with this. I set ng-model value first and after few second I set collection because I get first RecordInfo and after that info I get collection and in this situation default text displayText showing but it should show from collection.

I fix this issue by put this code in your library

// watch for collection
// this will update when collection change from outside
scope.$watchCollection(values, function (collection) {
    getMatches('');
});

Please fix this issue from your side or merge this code.

@kishanmundha Thank you for this solution. On my team, we also have this problem and I came here just for the same reason as the date of our first release is nearing. Maybe that using the 'cs-depends-on' attribute on a dummy variable (or the model?) will also do the trick.
@axel-zarate Any chance for an integration and a new release? :-) Many thanks.

Thanks for your feedback. Placing a watch on the data source would work well when using an in-memory array. Unfortunately, it's very impractical for remote data sources, as it would be evaluating them on every digest cycle. So I cannot include that solution as part of the code.

Until there's a better solution, I would suggest that you first store what will be your ng-model to a temporary variable (not the scope), then fetch the items and set the ng-model value to the scope only after you have set the collection, by means of a call back or a promise.

As @Amenel mentioned, you could also work mix it with the use of a dummy cs-depends-on to force ng-model reevaluation. It could be as simple as a Boolean value that you toggle on and off:

$scope.toggle = !$scope.toggle;

Or a numeric value that you increment by 1.

Sorry for this poor solution, but I can't think of one that covers both sync and async scenarios. Let me know if you come up with something.

At first sight, when I implemented it, it looked like cs-depends-on would work. But it didn't. The reason is that the execution path never goes into the if (newVal !== oldVal) block, even though the cs-depends-on attribute is defined in the controller and correctly referred to in the view. I can see in Firebug that each time the watch is triggered, both new and old values are undefined.

That's because I have encapsulated this widget in a larger one with additional action buttons, and with an isolated scope. This was solved by a quick and dirty solution (watching the depends-on value in the parent and sync'ing it in the larger widget's scope). Then it appeared that instead of "just" making sure the input reflects the current ng-model value, well, custom-select clears the input (I guess that's a consequence of childScope.select(undefined)). In our case, the model is somehow modified and since we have buttons that should be disabled on pristine forms, I reverted what I thought would be a solution!

I have not found a definitive solution yet.

Anyway, my advice would be to make the toggle an integer instead of a boolean.

@axel-zarate I've been looking for a solution to this problem for days now and got a lot of thinking done. Here's the untidy train of thoughts that led me to a solution. The solution I've come up with is at the bottom of this patch of text written over the course of the several days.

You wrote:

Placing a watch on the data source would work well when using an in-memory array. Unfortunately, it's very impractical for remote data sources, as it would be evaluating them on every digest cycle. So I cannot include that solution as part of the code.

The fact is that what I am doing (and I think that's what others are also doing) is that I set the data source for custom-select as an in-memory array that is fetched (only once!) via a remote source. Once the $resource promise is resolved, the bound data source never changes. In that scenario (which I think is the only real-world scenario with remote fetching of the list contents), won't the argument of reevaluating the data source on every digest cycle be moot?

You also wrote:

Until there's a better solution, I would suggest that you first store what will be your ng-model to a temporary variable (not the scope), then fetch the items and set the ng-model value to the scope only after you have set the collection, by means of a call back or a promise.

Here is an example from our code, where $scope.profils is the data source for custom-select.

    $scope.profils = profilRest.query({}, function(result) {
        $timeout(function() {
            $log.log("Setting idProfil to 3");
            $scope.dataModel.idProfil = 3;
        }, 0);
    });

The profiles list gets a promise, which is resolved when the server call returns. From then on, we never touch the list again. We still have the view showing the value in displayText and updating the view only when the list is clicked.

I've done as you suggested with the code above. There are circumstances I can't characterize when setting the model after the collection has loaded does not change anything either. For instance, when the view is loaded into a modal container, the text is never changed from the default display text, no matter how high the value of the timeout delay is.

The conclusion of all the testing is that setting the model is of no use regarding the matter at hand if custom-select does not know that its datasource has finally been fed, i.e. if the select (as label) for value in collection expression is not reevaluated. Apparently, it is when there's a click. We also need a way to have it be reevaluated at client code's will.

Reading the code, I see this in the NgOptions break down section:

    values = match[4],
    valuesFn = $parse(values),

Does that account for promises? Also, valuesFn is only used in getMatches, which is called... on a click (of course!) and when the search term changes. If none of these is called and the datasource was empty or not resolved yet at the moment this code is executed, it won't matter whether there is a promise into play.

Here is the solution.

What fed my thoughts is that we also use angular-ui-grid in the project. In case gridOptions.data is set to a string, that string is seen as the name of a variable to watch. I don't know (yet) of a UI component more likely to handle a huge volume of data than grids. Yet angular-ui-grid offers a watch. I saw some similarity there.

The solution is pretty simple: you add these lines in the code (I chose right after the if (dependsOn) block:

    var api = {
        refreshView : function() {
            getMatches('');
        }
    };
    if (angular.isFunction(options.onRegisterApi)) {
        options.onRegisterApi(api);
    }

Any client code facing this issue will then add an onRegisterApi property to their options, set it to a function and later on call refreshView as needed. This means that the options get an additional entry in the readme file. Sorry for the extra work :-) But, you control what is API and what's not, and the onus will be on clients to update the view in cases like the one this issue discussion is about.

I believe this is the cleanest and least disruptive solution, taking into account all that's been written in this repository about this problem. Names need to be changed though: I'm not fond of 'onRegisterApi' at all. A simple 'api' could be better. 'refreshView' could be 'updateView', etc. I hope you can integrate this and make another of your blinking fast releases. Many thanks.

This is work for me :

From Controller :
$rootScope.$broadcast('ModelUpdate');

In custom-select directive :
scope.$on('ModelUpdate', function () { getMatches(''); });

@dalbir Your solution appears simpler at first sight and I didn't look past my own solution once I found that it was working. I've never been in favor of broadcasting as that opens the door for many controllers being involved when they shouldn't be, in strict and purist terms. For instance, multiple instances of custom-select on a page will react to one broadcast, even when they are displaying static values. I'd rather have one strictly targeted getMatches call: that way, one can control what calls what. Anyway, congrats on coming up with the simplest solution so far.

@Amenel, sorry, I understand where you put the refreshView code, but I did not understand when it will be fired..

@rhesusfabio You have to fire it when the controller of your view has everything needed, ie it has the model value and it knows that the remote data has arrived.

Considering that you've put refreshView already in the customSelect code, you need the following steps.

In your options, you need something like

$scope.customSelectOptions.onRegisterApi = function(widget) {
    $scope.customSelectWidget = widget;
};

Then, in your $resource success function, supposing you've already got the model value set, you need to include $scope.customSelectWidget.refreshView().

It'll work, provided that the remote data is available and the model value has been set. The difficulty for you is in assessing when, in your own application, both conditions are satisfied.

Holy sh.....
Thank you so much @Amenel.

Hi everyone,

I read many suggestion. But as per my view a library should completely independent from any other controller or custom code. I have implemented this code in many pages. So as per your suggestion I need to change in every page. I can't do this because I can do this by change only in one place and I believe to change minimum code.

My latest changes code in this library

// only use if collection is local
if (!options.async) {
    // watch for collection
    // this will update when collection change from outside
    scope.$watchCollection(values, function (collection) {
        getMatches('');
    });
}

But as per my view a library should completely independent from any other controller or custom code.

Sure, but I've taken refreshView and onRegisterApi straight from angular-ui-grid.

Due to our offering the user to edit values of the displayed item (which may lead to a discrepancy between the label displayed in custom-select and the label stored in the database) and to delete , I've come to update the code I proposed in an earlier comment. In our project, we now have this:

var api = {
    refreshView : function() {
        getMatches('');
        setDisplayText();
    }
};
if (angular.isFunction(options.onRegisterApi)) {
    options.onRegisterApi(api);
}

The difference is that I've added a call to setDisplayText.

When the user edits the object that is displayed, or deletes it, the data source is reloaded and I call refreshView on the custom-select widget. With this code, edits are immediately reflected on the widget and deletions lead to the default display text being shown again.

I apologize for the waiting on this issue. Took a long vacation. I appreciate all your contributions and ideas. But honestly I don't know which approach I should take on this issue. On one side, I could try and make the plugin "smarter" and guess as much as it can so that it can make assumptions on when to eagerly refresh the collection, but I'm sure it won't cover all cases. On the other side, I could provide an API object as suggested by @Amenel and let user decide when and how to refresh (and probably expose some other functionality).

Maybe I'd lean more towards the API solution, so that it the plugin is more flexible (but slightly more complex for the user).