Currently, I'm working on a project where I needed to run some code every time the page is hidden.
I searched high and low and found this question: Is there a way to detect if a browser window is not currently active?. The accepted answer did an amazing job, but it only sets a class, doesn't run a function.
So, with that need, I took off from that code and made an API around it:
;(function(window){
//Name of the property used by the browser
var hidden = 'hidden';
//Internal status, to know if the page is visible or not
//This will hold the current page visibility status, in case someone runs .fire()
var status;
var handlers = {
visible: [],
hidden: [],
change: []
};
var fire = function(event){
var fired_status = event === 'change' ? status : event;
//Prevents external changes from causing off-by-n bugs
var list = handlers[event];
var length = list.length;
for(var i = 0; i < length; i++)
{
if(list[i])
{
list[i].call(window, fired_status);
}
}
};
//=========================================================================================
// Core code, taken from https://stackoverflow.com/q/1060008/
// Changed to my own taste and needs
//Only runs when the status changes, so, we trigger the change here too
var onchange = function(event){
var map = {
focus: 'visible',
focusin: 'visible',
pageshow: 'visible',
blur: 'hidden',
focusout: 'hidden',
pagehide:'hidden'
};
event = event || window.event;
//We need to keep the internal status updated
status = map[event.type] || (this[hidden] ? 'hidden' : 'visible');
fire(status);
fire('change');
};
if (hidden in document)
{
document.addEventListener('visibilitychange', onchange);
}
else if ((hidden = 'mozHidden') in document)
{
document.addEventListener('mozvisibilitychange', onchange);
}
else if ((hidden = 'webkitHidden') in document)
{
document.addEventListener('webkitvisibilitychange', onchange);
}
else if ((hidden = 'msHidden') in document)
{
document.addEventListener('msvisibilitychange', onchange);
}
else if ('onfocusin' in document)
{
document.onfocusin = document.onfocusout = onchange;
}
else
{
window.onpageshow = window.onpagehide = window.onfocus = window.onblur = onchange;
}
status = document[hidden] === undefined ? null : (document[hidden] ? 'hidden' : 'visible');
//=========================================================================================
if(status !== null)
{
var fn = function(){
if(status === 'hidden')
{
fire('hidden');
}
window.removeEventListener('load', fn);
};
window.addEventListener('load', fn);
}
window.PageVisibility = {
//jQuery - style handling
visible: function(handler){
if(handler instanceof window.Function)
{
handlers['visible'][handlers['visible'].length] = handler;
}
else
{
fire('visible');
}
},
//plain style (eg: obj.onevent = handler)
set onvisible(handler){
handlers['visible'] = handler instanceof window.Function ? [handler] : [];
},
get onvisible(){
return handlers['visible'][0];
},
hidden: function(handler){
if(handler instanceof window.Function)
{
handlers['hidden'][handlers['hidden'].length] = handler;
}
else
{
fire('hidden');
}
},
set onhidden(handler){
handlers['hidden'] = handler instanceof window.Function ? [handler] : [];
},
get onhidden(){
return handlers['hidden'][0];
},
visibilitychange: function(handler){
if(handler instanceof window.Function)
{
handlers['change'][handlers['change'].length] = handler;
}
else
{
fire('change');
}
},
set onvisibilitychange(handler){
handlers['change'] = handler instanceof window.Function ? [handler] : [];
},
get onvisibilitychange(){
return handlers['change'][0];
},
//Modern style
addEventListener: function(event, handler){
if(!(event in handlers) || !(handler instanceof window.Function))
{
return false;
}
handlers[event][handlers[event].length] = handler;
return true;
},
removeEventListener: function(event, handler){
if(!(event in handlers) || !(handler instanceof window.Function))
{
return false;
}
if(handler)
{
for(var i = 0, l = handlers[event].length; i < l; i++)
{
if(handlers[event][i] === handler)
{
delete handlers[event][i];
}
}
}
else
{
handlers[event] = [];
}
return true;
},
//Should return null only when it is impossible to check if the page is hidden
isVisible: function(){
return status === null ? null : status === 'visible';
},
fire: function(){
if(status !== null)
{
fire(status);
}
},
//May be useful for someone
hiddenPropertyName: hidden
};
})(Function('return this')());
The code is quite long but most of it is just whitespace.
The code allows to use jQuery-style event handling, direct-attribution style (or old style) and the "modern" addEventListener
style.
I know the code isn't compatible with IE8, but I only need to support IE9 and up. Any other browser that works is perfectly fine.
My major area of concert is all the repeated code on the event handling. I couldn't think of a way to keep it easy and clean.
Performance might be an issue as well.
In terms of readability, clarity and performance, what else can I improve?
2 Answers 2
Currently, there is a small bug on removeEventListener
.
The method is prepared to remove all event listeners. It does so by having no value on the 2nd parameter.
To remove a single event listener, just pass the function.
An example:
PageVisibility.removeEventListener('change', function(){}); //works, removes the event listener
PageVisibility.removeEventListener('change'); //doesn't work, returns `false`
There are 2 ways to fix this:
Change the
if
inside the method:if(!(event in handlers) || (handler && !(handler instanceof window.Function))) { return false; }
Rewrite the code to remove multiple event listeners:
[...] removeEventListener: function(event, handler){ if(!(event in handlers) || !(handler instanceof window.Function)) { return false; } for(var i = 0, l = handlers[event].length; i < l; i++) { if(handlers[event][i] === handler) { delete handlers[event][i]; } } return true; }, removeEventListeners: function(event){ if(!(event in handlers)) { return false; } handlers[event] = []; return true; }, [...]
I've decided to stick with the first option since it is easier to use.
Some may disagree and prefer the second option due to separation of tasks and avoid surprises if they forget the 2nd parameter.
I would suggest refactoring, at least, two "moments":
- check for 'hidden' property support among browsers;
- window.PageVisibility
object
The goal is to eliminate redundancy and duplicate logic.
var hiddenMap = {'hidden': '', 'mozHidden': 'moz', 'webkitHidden': 'webkit', 'msHidden': 'ms'},
foundSupport = false, // indicates whether '~hidden' was determined in current browser (depending on browser's type)
prop;
// check for '~hidden' property existence among major browsers
for (prop in hiddenMap) {
if ((hidden = prop) in document) {
document.addEventListener(hiddenMap[prop] + 'visibilitychange', onchange);
foundSupport = true;
break;
}
}
if (!foundSupport) {
if ('onfocusin' in document) {
document.onfocusin = document.onfocusout = onchange;
} else {
window.onpageshow = window.onpagehide = window.onfocus = window.onblur = onchange;
}
}
/**
* Sets/Fires passed 'PageVisibility' handler
* @param handler expects function object
* @param eventType event type
* @param setter whether the method id delegated to set passed handler into list of handlers
* return void
**/
var setVisibilityHandler = function(handler, eventType, setter) {
if (handler && type) {
if (setter) { // setter action
handlers[eventType] = handler instanceof window.Function ? [handler] : [];
} else {
if (handler instanceof window.Function) {
handlers[eventType][handlers[eventType].length] = handler;
} else {
fire(eventType);
}
}
}
};
window.PageVisibility = {
//jQuery - style handling
visible: function (handler) {
setVisibilityHandler(handler, 'visible');
},
//plain style (eg: obj.onevent = handler)
set onvisible(handler) {
setVisibilityHandler(handler, 'visible', true);
},
get onvisible() {
return handlers['visible'][0];
},
hidden: function (handler) {
setVisibilityHandler(handler, 'hidden');
},
set onhidden(handler) {
setVisibilityHandler(handler, 'hidden', true);
},
get onhidden() {
return handlers['hidden'][0];
},
visibilitychange: function (handler) {
setVisibilityHandler(handler, 'change');
},
set onvisibilitychange(handler) {
setVisibilityHandler(handler, 'change', true);
},
get onvisibilitychange() {
return handlers['change'][0];
},
//Modern style
addEventListener: function (event, handler) {
if (!(event in handlers) || !(handler instanceof window.Function)) {
return false;
}
handlers[event][handlers[event].length] = handler;
return true;
},
removeEventListener: function (event, handler) {
if (!(event in handlers) || !(handler instanceof window.Function)) {
return false;
}
if (handler) {
var len = handlers[event].length;
while (len--) {
if (handlers[event][len] === handler) {
delete handlers[event][len];
}
}
} else {
handlers[event] = [];
}
return true;
},
//Should return null only when it is impossible to check if the page is hidden
isVisible: function () {
return status === null ? null : status === 'visible';
},
fire: function () {
if (status !== null) {
fire(status);
}
},
//May be useful for someone
hiddenPropertyName: hidden
};
-
\$\begingroup\$ I'm really sorry, but I'm very confused with your review. Why does
checkHandler
does everything but checking the handler? And why is it a named function, on the global scope? Why it doesn't return anything? And why thatfoundSupport
variable? I'm not saying your review is bad. I'm saying that it is confusing and needs some clarification. I really don't know your logic and why you wrote it the way you did. \$\endgroup\$Ismael Miguel– Ismael Miguel2016年03月28日 22:21:33 +00:00Commented Mar 28, 2016 at 22:21 -
\$\begingroup\$ 1)
Why does checkHandler does everything but checking the handler?
- Doesn'tcheckHandler
check passed handler with thishandler instanceof window.Function
orhandler instanceof window.Function
? don't you think? 2)And why is it a named function, on the global scope?
- Ok. I had replaced it with variable; 3)Why it doesn't return anything?
- Functions don't always have to return something, it isn't strictly necessary. It's an entirely subjective decision. 4) why that foundSupport variable? - It's a flag, indicating whether~hidden
property was determined among major browsers \$\endgroup\$RomanPerekhrest– RomanPerekhrest2016年03月29日 06:12:59 +00:00Commented Mar 29, 2016 at 6:12 -
\$\begingroup\$ The problem isn't that it is a named function. The problem is that it is on the global scope and does the oposite of checking the handler. I see what you have gone for with the function,but the name isn't the right one. I like the idea, not the name. But the big problem is that you simply put the code without any explanation. A review should explain that my code sucks, why it sucks and hoe to make it non-sucky. I don't get that from yours. Please, try to explain yourself a little on the answer. So everybody can understand why you went that way and +1 your answer. I see the code, not the "why". \$\endgroup\$Ismael Miguel– Ismael Miguel2016年03月29日 07:55:22 +00:00Commented Mar 29, 2016 at 7:55
-
\$\begingroup\$ ` The problem is that it is on the global scope` -
var fire
,var onchange
,window.PageVisibility
are also in global scope. As for the name that handler - the problem is that it acts like a handler 'setter' in one case, and like a handlerlauncher
- in another case. So, the name should be pretty creative, orcheckHandler
should split into two separate functions. \$\endgroup\$RomanPerekhrest– RomanPerekhrest2016年03月29日 14:32:21 +00:00Commented Mar 29, 2016 at 14:32 -
4\$\begingroup\$ @RomanPerekhrest I've noticed that you have posted 6 answers and only have 3 upvotes here on Code Review, but you have 11K on stackoverflow.com. We are a little different on code review, generally a written explanation rather than an alternate solution is considered a better answer here on code review. You have at least one answer here on code review that is good at codereview.stackexchange.com/questions/118184/…. It might help improve your scores if you read the help page codereview.stackexchange.com/help/how-to-answer. \$\endgroup\$2016年09月20日 14:34:42 +00:00Commented Sep 20, 2016 at 14:34
Explore related questions
See similar questions with these tags.
window.PageVisibility
object don't return anything, they just read an array valueget onvisible(){ handlers['visible'][0]; }
. Could you explain why ? \$\endgroup\$return
-.- I've ninja-edited it. \$\endgroup\$