ui-sortable icon indicating copy to clipboard operation
ui-sortable copied to clipboard

Remove logging when ngModel is missing from the directive instance

Open sszdh opened this issue 9 years ago • 9 comments

Hi there, I think, in case of using ui-sortable without ng-repeat, ngModel in directive definition should be optional and no log should be thrown in console.

Case of ng-repeat-free:

<ul ui-sortable="uiSortableProperties">
<% @items.each do |item| -%>
    <li class="movable">
        <%= item.text %>
    </li>
<% end -%>
    <li>....</li>
<ul>

sszdh avatar Nov 04 '15 11:11 sszdh

Actually, in v0.14.x the ng-repeat will be required. If there is not need to bind with a model, then you could just do a $('...').sortable() in a simple directive. I will keep this issue open thought and we can revisit in case there is more people requesting something like this.

thgreasi avatar Nov 04 '15 11:11 thgreasi

Moreover, this directive reverts the changes to the DOM after the drop happens and relies on the model change and the ng-repeat to actually create the final result. That was required in v0.12.0 because of the extra HTML comments that angularjs in v1.2 started to add.

thgreasi avatar Nov 04 '15 11:11 thgreasi

@thgreasi, I understand, but it will be lead us to duplicate jquery/angular usage and duplicate configuration in single application.

It will be DRY & good practice to use one library for single usecase in all over the APP.

Thanks.

sszdh avatar Nov 04 '15 12:11 sszdh

Something like this should be enough, just remember to use $scope.apply().

thgreasi avatar Nov 04 '15 13:11 thgreasi

Sorry, but Bad solution. If you actually require ngModel, so you should throw exception not log arbitary element in console

sszdh avatar Nov 04 '15 22:11 sszdh

We actually started logging info messages in the console, because there were tons of issues opened, claiming for bugs, but most of these people just forgot to use the ng-model. On the other hand, there where people like you, that just wanted the sorting interaction without effecting anything inside angular. That was the reason that there is an if (ngModel) and there is a log.

If logging bother you, then I assume that you should already be using $logProvider.debugEnabled(true) to disable logging on your production build. Or even better disable the debug data altogether.

What is the use case that you are trying to achieve and this issue is blocking it?

thgreasi avatar Nov 04 '15 23:11 thgreasi

@thgreasi, I understand the reason of if/else. Just logging info does not make sense for me in first place. I'm just trying to help :)

Anyway we should say thanks for this great package +1

sszdh avatar Nov 05 '15 08:11 sszdh

@sszdh is there a specific use case that troubles you? Should we just rename the issue to something like "remove logging possible configuration errors"? Can it be closed?

thgreasi avatar Nov 05 '15 09:11 thgreasi

@thgreasi, no special trouble! but seeing something like this, specially second line ( element's specifications ) :

(!) ui.sortable: ngModel not provided! 
[div.ng-isolate-scope, context: div.ng-isolate-scope, jquery: "2.1.4", constructor: function, selector: "", toArray: function…]

in browser's console, makes no sense in production. I think renaming the issue would be good choice :) Cheers

sszdh avatar Nov 05 '15 10:11 sszdh