10
\$\begingroup\$

In AngularJS, I currently have a directive and 2 services which permit me to bind the window resize event, and broadcast it to my application.

I would like to know if you could see anything that could be improved in my code, in term of optimization, naming or possible cross browser issues.

I have:

  • A directive which binds resize event. It will broadcast the event 300ms after the last resize (to avoid flood).
  • A factory to get window's dimensions
  • A service to detect browser's name (chrome, safari, etc...). In this case, it is used to target Safari which doesn't calculate window's dimensions the same way.

Here is a plunkr to illustrate the code.

Detect Browser Service

/*
 * Detects on which browser the user is navigating
 *
 * Usage:
 * var browser = detectBrowser();
 *
 */
commonApp.service('detectBrowser', ['$window', 
    function( $window ) {

        // http://stackoverflow.com/questions/22947535/how-to-detect-browser-using-angular
        return function() {
            var userAgent = $window.navigator.userAgent,
                browsers  = { 
                    chrome  : /chrome/i,
                    safari  : /safari/i,
                    firefox : /firefox/i,
                    ie      : /internet explorer/i
                };

            for ( var key in browsers ) {
                if ( browsers[key].test(userAgent) ) {
                    return key;
                }
            }

            return 'unknown';
        }
    }]);

Window Dimensions Factory

/*
 * Get window height and width
 *
 * Usage:
 * windowDimensions.height();
 * windowDimensions.width();
 *
 */
commonApp.factory('windowDimensions', ['$window', 'detectBrowser', 
    function( $window, detectBrowser ) {
        var browser = detectBrowser();

        return {
            height: function() {
                return ( browser === 'safari' ) 
                          ? document.documentElement.clientHeight 
                          : window.innerHeight || document.documentElement.clientHeight || document.body.clientHeight
            },

            width : function() {
                return ( browser === 'safari' ) 
                          ? document.documentElement.clientWidth 
                          : window.innerWidth || document.documentElement.clientWidth || document.body.clientWidth
            }
        }
    }]);

Window Resize Event Directive

/*
 * Watch window resizing event to set new window dimensions,
 * and broadcast the event to the app
 *
 * Usage:
 * <html watch-window-resize>...</html>
 *
 * Bind the resize event:
   $scope.$on('watchWindowResize::resize', function() {
       // Do something
   });
 *
 */
app.directive('watchWindowResize', ['$window', '$timeout', 'windowDimensions', 
    function( $window, $timeout, windowDimensions ) {

        return {
            link: function( $scope ) {
                // Get window's dimensions
                $scope.getDimensions = function() {

                    // Namespacing events with name of directive + event to avoid collisions
                    // http://stackoverflow.com/questions/23272169/what-is-the-best-way-to-bind-to-a-global-event-in-a-angularjs-directive
                    $scope.$broadcast('watchWindowResize::resize', {
                        height: windowDimensions.height(),
                        width : windowDimensions.width()
                    });
                }

                // On window resize...
                angular.element($window).on('resize', function( e ) {

                    // Reset timeout
                    $timeout.cancel($scope.resizing);

                    // Add a timeout to not call the resizing function every pixel
                    $scope.resizing = $timeout( function() {

                        $scope.getDimensions();
                    }, 300);
                });
            }
        }
    }]);
\$\endgroup\$
3
\$\begingroup\$

Fun question,

I am not sure why you think you need a special treatment for Safari, I could not find any reference to that on the webs. So if you only need the browser code for that part, then I would drop it and simply go for

/*
 * Get window height and width
 *
 * Usage:
 * windowDimensions.height();
 * windowDimensions.width();
 *
 */
app.factory('windowDimensions', ['$window',
  function($window) {
    return {
      height: function() {
        return window.innerHeight || document.documentElement.clientHeight || document.body.clientHeight;
      },
      width: function() {
        return window.innerWidth || document.documentElement.clientWidth || document.body.clientWidth;
      }
    };
  }
]);

From a naming perspective I would suggest these changes:

  • getDimensions -> broadcastDimensions (since that is really what you are doing there)
  • $scope.resizing -> $scope.resizingTimer (to reduce potential confusion, resizing is a verb so one would expect resizing to do something instead of merely being a timer)
  • 300 -> var delayTime = 300; //Broadcast 300ms after the last resize to avoid flood

Other than that, all is well besides a few missing semicolons. You can test my fork on Safari if you wish, it worked for me: http://plnkr.co/edit/SY8pnX3ae4YtWErno5f0?p=preview

\$\endgroup\$
4
  • \$\begingroup\$ "Fun question" ? :D By the way thanks for your answer, naming is really not my highest quality as english is not my mother language and I have some troubles to find the right vocabulary. Concerning Safari, I had problems on a previous project with media queries and real window's width. I just don't remember if it was in safari mobile devices (I could not test it right now), because you are right for latest Safari browser, it doesn't change anything. I updated the plunkr with your code and some media queries (also console log window's width) plnkr.co/edit/ephqGM1OiR8zsxApSrkR?p=preview \$\endgroup\$
    – Niflhel
    Aug 8 '14 at 17:21
  • \$\begingroup\$ Also, why putting delayTime inside a variable as I am using it only once? My goal is not to permit to change this delay inside the directive via an option. Is there a reason behind? \$\endgroup\$
    – Niflhel
    Aug 8 '14 at 17:24
  • \$\begingroup\$ I tested this on my iPhone 5, it worked for me. As for delayTime, you could also just put 300 /*ms delay*/ or something \$\endgroup\$
    – konijn
    Aug 8 '14 at 18:22
  • \$\begingroup\$ I will check this when I have access to more Safari devices and I come back to you. I am quite sure that I did this for a good purpose ^^ \$\endgroup\$
    – Niflhel
    Aug 8 '14 at 18:30

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.