feat(tagsInput): Add ng-disabled support

[oteichmann]

Add an option for the user to disable the tags input component. Set ng-disabled to true will prevent any changes to the tags defined within the model. The field can not be focused any more. Anchors to remove tags will be hidden as well as the "add new" placeholder.

This pull is based on the existing one #295 but without any conflicts.

Closes #102.

[coveralls]

Coverage increased (+0.0%) to 99.72% when pulling 6a0d1c9 on oteichmann:ng-disabled into 315f3a2 on mbenford:master.

[coveralls]

Coverage increased (+0.0%) to 99.72% when pulling 5038411 on oteichmann:ng-disabled into 315f3a2 on mbenford:master.

[coveralls]

Coverage increased (+0.0%) to 99.72% when pulling b1054c0 on oteichmann:ng-disabled into 315f3a2 on mbenford:master.

[coveralls]

Coverage increased (+0.0%) to 99.72% when pulling 50d6ad0 on oteichmann:ng-disabled into 315f3a2 on mbenford:master.

[cezarpretto]

how about this functionality. I need this and I think its a good functionality.

[oteichmann]

Hey @cezarpretto,

I am sure @mbenford is working hard to get all the issues fixed asap. In the meantime you could use my fork of the bower release repo. The following commit contains the latest changes https://github.com/oteichmann/ngTagsInput-bower.git#7eb07b5e12be542c1cb89ce7773fc260d80de1a9

[cezarpretto]

Thanks @oteichmann,

It will be very helpful.

[jukkasi]

Maybe by default hide placeholder text when ng-disabled="true"? When visible but grayed out it only confuses user.

Also I am able to focus input by clicking the input, after this blue borders appear, however I'm unsure if this is because our custom styles.

[oteichmann]

Hi @jukkasi,

thanks for your comment. In fact i first followed the approach you suggest. But then I recognized that other bootstrap fields also do not hide the placeholder. Additionally it caused some styling issues if there is no tag left. That's why I decided to keep it.

Regarding the focus issue. Which browser did you test with? I am pretty sure this had already been fixed with my latest commit.

Regards,
Oliver

[mbenford]

@oteichmann Could you take a look at #295? That PR and this one seem to solve the same problem.

[oteichmann]

Hi @mbenford,

I did already. As stated in my first comment this pull is based on #295. But there had been some merge conflicts in the original one. And some functionalities like disabling the input did not work as expected. I hope this pull here can be merged without any problems.
Another reason to create a second version was that I needed a quick solution for my project that can be integrated via bower dependency management... ;-)

Regards,
Oliver

[mbenford]

I skimmed over the comments and missed yours. My bad. :(

I wonder if you could work with @dlukez and merge both PRs into one?

[oteichmann]

Of course we could... But I don't know if it is worth the effort. I just compared the two branches. If you do yourself you will recognize that the one of @dlukes is far behind the current master version of your component. My branch already contains all of the approaches / concepts from @dlukes as well as some more improvements.

[mbenford]

I just don't want to take anyone for granted. @dlukez's PR is around for a while, and yours seems to contain more improvements. I'd be very happy if you guys could sort it out. :)

[dlukez]

@mbenford take this one! Mine is old :)

[jukkasi]

@oteichmann

For Internet Explorer (11) everything works like I think it should regarding the focus issue, clicking disabled ng-tags-input does nothing visually.

In Firefox 32 clicking disabled ng-tags-input I get gray "dotted" borders around input.

For Chrome behavior is same than in FF but I get blue borders (our custom style I guess)

It is still possible that your default styles are OK and something is wrong with ours. Another problem is that in IE 11 disabled placeholder text is much more visible than in other browsers.

[mbenford]

I think it would be better to target the tag-input element directly, like this:

tags-input[disabled] {
...
}

That way you wouldn't need to set any class to any element.

[oteichmann]

Hey @mbenford, everything done. Please review.

[mbenford]

Thanks! Now, I'm not sure what has happened, but your PR's history is messed up. There are commits from the master branch among your own commits.

It seems that you have merged the master branch into your own, instead of rebasing it on top of master.

[oteichmann]

Hmm than maybe I understood something wrong. I just followed your contribution guidelines and executed "git rebase master -i" after updating the master from upstream...

Maybe the problem is the following!?

Don’t Rebase Public History
As we’ve discussed with git commit --amend and git reset, you should never rebase commits that have been pushed to a public repository. The rebase would replace the old commits with new ones, and it would look like that part of your project history abruptly vanished.

https://www.atlassian.com/git/tutorials/rewriting-history/git-rebase

[mbenford]

Rebasing commits that already have been pushed is only an issue if other people have based their work on those commits. That's not the case for most forks, so there's no harm.

I see a merge commit in your PR's history, so a merge was effectively done. The "correct" history should contain only one commit containing your changes, like this one. I suggest that you extract your changes and apply them to a clean branch.

[oteichmann]

Hi @mbenford,
it took me some time but I managed to fix the history of the branch / pull. Please review.

[coveralls]

Coverage remained the same at 100.0% when pulling 69016ba on oteichmann:ng-disabled into 5ecd0d4 on mbenford:master.

[mbenford]

You still need to squash your commits into a single one, with the feat(tagsInput)... message.

[coveralls]

Coverage remained the same at 100.0% when pulling a443603 on oteichmann:ng-disabled into 5ecd0d4 on mbenford:master.

[oteichmann]

@mbenford Done, please review.

[coveralls]

Coverage remained the same at 100.0% when pulling c040d24 on oteichmann:ng-disabled into 5ecd0d4 on mbenford:master.

[devshane]

Thanks for this feature @oteichmann and thanks for the directive @mbenford, this blows away the other solutions.

Just a note: the API documentation at http://mbenford.github.io/ngTagsInput/documentation/api doesn't reflect this PR, you can only find it by searching the repo.

[Dan-Nolan]

Great feature, thanks! Still the case it's not in the API though.