"Best practice" is fuzzy ground here. If it's readable and it works, then you're 90% there, IMO, and it's probably fine.
That said, the "angular way" is to keep DOM manipulation out of the controller, and to use dependency injection to make sure everything is testable. Obviously the way you illustrated above would be hard to test, and puts some DOM manipulation in the controller.
I guess what I would do to get the DOM manipulation out of the controller is use a directive:
A simple directive to tie your dialog open call to a click on an element:
app.directive('openDialog', function(){
return {
restrict: 'A',
link: function(scope, elem, attr, ctrl) {
var dialogId = '#' + attr.openDialog;
elem.bind('click', function(e) {
$(dialogId).dialog('open');
});
}
};
});
And in mark up it would be used like so:
<button open-dialog="modal-to-open">Open Dialog</button>
Now, this is obviously very basic. You could get pretty advanced with this if you wanted to, adding additional attributes for different options in the dialog.
You could go even further and add a Service that opened the dialog for you, so you could inject it into your controller or even your directive, and get the call out of there that way. For example:
app.factory('dialogService', [function() {
return {
open: function(elementId) {
$(elementId).dialog('open');
}
};
}]);
And here it is in use. It seems silly because it's essentially the same thing. But that's mostly because it's a very simplistic example. But it at least leverages DI and is testable.
app.controller('MyCtrl', function($scope, dialogService) {
$scope.open = function () {
dialogService.open('#modal-to-open');
};
});
Anyhow. I hope all of that helps you decide what path you want to take. There are a thousand ways to do this. The "right" way is whatever works, allows you to do whatever you need to do (testing or anything else), and is easy to maintain.
