patternjavascriptMinor
Browser resize service
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:
Here is a plunkr to illustrate the code.
Detect Browser Service
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
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
From a naming perspective I would suggest these changes:
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
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,resizingis a verb so one would expectresizingto 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.