HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Browser resize service

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
resizeservicebrowser

Problem

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

Solution

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

Code Snippets

/*
 * 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;
      }
    };
  }
]);

Context

StackExchange Code Review Q#59268, answer score: 3

Revisions (0)

No revisions yet.