Shuffling and hiding views specified by parameters In my Angular app, users click on different icons. When a user clicks on respective icon, I can show the icons gallery, as well shuffle the view.

Apart from this shuffling functionality, I need to hide whatever other gallery is already open.

For that, I've written the following function, but, I think it could use some refactoring.

$scope.galleryMenu = function ( GalleryType ) {

            var hideGallery = (function (type) {

                return {

                    galleryVideoShow : function () {

                        //closing others
                        $scope.contractorShow = false;
                        $scope.galleryPhotoShow = false;
                    },

                    contractorShow : function () {
                        //closing others
                        $scope.galleryPhotoShow = false;
                        $scope.galleryVideoShow = false;

                    },

                    galleryPhotoShow : function () {

                       //closing others
                        $scope.contractorShow = false;
                        $scope.galleryVideoShow = false;

                    }

                }

            })();

            $scope.$watch('viewProjectInfo', function ( newValue, oldValue ) {


                if(!newValue) return;

                $scope.photos = $scope.viewProjectInfo.ImagePaths;

                $scope[GalleryType] = !$scope[GalleryType];
                $scope.shownGallery =  GalleryType;

                hideGallery[GalleryType](); //manually close other open gallery

                $('html, body').animate({scrollTop:$(document).height()}, 'slow');



            });



        };
A few things that could be improved:



You have quite a lot of empty white lines, and extraneous space:


return {

    galleryVideoShow : function () {

        //closing others
        $scope.contractorShow = false;
        $scope.galleryPhotoShow = false;
    },

    contractorShow : function () {

    // ...



This block, for example, without extraneous whitespace, would look like:

return {
    galleryVideoShow: function(){
        $scope.contractorShow,
        $scope.galleryPhotoShow = false;
    },
    contractorShow: function(){

    //...





        var hideGallery = (function (type) {

            return {

                galleryVideoShow : function () {

                    //closing others
                    $scope.contractorShow = false;
                    $scope.galleryPhotoShow = false;
                },

                contractorShow : function () {
                    //closing others
                    $scope.galleryPhotoShow = false;
                    $scope.galleryVideoShow = false;

                },

                galleryPhotoShow : function () {

                   //closing others
                    $scope.contractorShow = false;
                    $scope.galleryVideoShow = false;

                }

            }

        })();



There's no reason to assign hideGallery like that.

It'd be better as a plain object.

var hideGallery = {
        galleryVideoShow: function(){
            //closing others
            $scope.contractorShow,
            $scope.galleryPhotoShow = false;
        },
        contractorShow: function(){
            $scope.galleryPhotoShow,
            $scope.galleryVideoShow = false;
        },
        galleryPhotoShow: function(){
            $scope.contractorShow,
            $scope.galleryVideoShow = false;
        }
}


Which would be better called something like GALLERY_EFFECTS, and kept as a constant.

Also note that for each function in hideGallery, the comment //closing others was present, it's only part of what the functions do, and would be better off at the root of hideGallery in a more expanded version.




       $scope.$watch('viewProjectInfo', function ( newValue, oldValue ) {


            if(!newValue) return;

            $scope.photos = $scope.viewProjectInfo.ImagePaths;

            $scope[GalleryType] = !$scope[GalleryType];
            $scope.shownGallery =  GalleryType;

            hideGallery[GalleryType](); //manually close other open gallery

            $('html, body').animate({scrollTop:$(document).height()}, 'slow');



        });



    };




Why are there so many empty lines at the bottom? (six, I counted) They just look poorly formatted, or like missing code.
You should wrap your conditionals in brackets, because untold circumstances can happen.
You should spread the .animate call over a multiple lines, as you're building an object inside the function call.




   $scope.$watch('viewProjectInfo', function(newValue, oldValue){
       if (!newValue){ return };
       $scope.photos = $scope.viewProjectInfo.ImagePaths;

       $scope[GalleryType] = !$scope[GalleryType];
       $scope.shownGallery = GalleryType;

       hideGallery[GalleryType]();

       $('html, body').animate({
           scrollTop: $(document).height()
       }, 'slow');
   });