patternjavascriptMinor
Stack Overflow tab notifier user-script
Viewed 0 times
scriptstackoverflowusertabnotifier
Problem
I searched for a notification script that shows inbox and rep change items on the browser tab in stackapps.com but either they didn't work or were outdated.
So, I've done this raw script myself and want to make it fool proof and robust:
This is my first ever user script. So want to make it better. (Even if it wasn't the first, I would have wanted to make it better)
```
// ==UserScript==
// @name Tab Notifier
// @namespace http://2200ce.wordpress.com/
// @version 0.1
// @description A notification system that displays inbox and rep change notification on the tab
// @author Amit Joki - http://stackoverflow.com/users/3001736/amit-joki
// @include http://stackoverflow.com/*
// @grant none
// @require https://cdn.rawgit.com/kapetan/jquery-observe/master/jquery-observe.js
// @match ://.stackexchange.com/*
// @match ://.stackoverflow.com/*
// @match ://.superuser.com/*
// @match ://.serverfault.com/*
// @match ://.askubuntu.com/*
// @match ://.stackapps.com/*
// @match ://.mathoverflow.net/*
// ==/UserScript==
var userscript = function($) {
$('.icon-inbox .unread-count, .icon-achievements .unread-count').observe({
attributes: true,
attributeFilter: ['style']
}, function() {
var title = document.title;
var isInbox = $(this).parent().is(".icon-inbox") ? true : false;
if (!$(this).attr('style')) {
document.title = updater(title, $(this), isInbox);
} else if ($(this).attr("style") == "display: none;") {
debugger;
var frags = title.replace(/• /,"•").split(/\s(?=\w)/);
var rgx = (isInbox ? /\[\d+\]/ : /\[\+\d+\]/);
if(frags[0].indexOf("•") == -1){
title = frags[0].replace(rgx, "") + frags.slice(1).join(" ");
} else {
rgx = /^(\[)(\+\d+) •(\d+)(\])/;
title = frags[0].replace(rgx, isInbox ? "$1$2$$4" :
So, I've done this raw script myself and want to make it fool proof and robust:
This is my first ever user script. So want to make it better. (Even if it wasn't the first, I would have wanted to make it better)
```
// ==UserScript==
// @name Tab Notifier
// @namespace http://2200ce.wordpress.com/
// @version 0.1
// @description A notification system that displays inbox and rep change notification on the tab
// @author Amit Joki - http://stackoverflow.com/users/3001736/amit-joki
// @include http://stackoverflow.com/*
// @grant none
// @require https://cdn.rawgit.com/kapetan/jquery-observe/master/jquery-observe.js
// @match ://.stackexchange.com/*
// @match ://.stackoverflow.com/*
// @match ://.superuser.com/*
// @match ://.serverfault.com/*
// @match ://.askubuntu.com/*
// @match ://.stackapps.com/*
// @match ://.mathoverflow.net/*
// ==/UserScript==
var userscript = function($) {
$('.icon-inbox .unread-count, .icon-achievements .unread-count').observe({
attributes: true,
attributeFilter: ['style']
}, function() {
var title = document.title;
var isInbox = $(this).parent().is(".icon-inbox") ? true : false;
if (!$(this).attr('style')) {
document.title = updater(title, $(this), isInbox);
} else if ($(this).attr("style") == "display: none;") {
debugger;
var frags = title.replace(/• /,"•").split(/\s(?=\w)/);
var rgx = (isInbox ? /\[\d+\]/ : /\[\+\d+\]/);
if(frags[0].indexOf("•") == -1){
title = frags[0].replace(rgx, "") + frags.slice(1).join(" ");
} else {
rgx = /^(\[)(\+\d+) •(\d+)(\])/;
title = frags[0].replace(rgx, isInbox ? "$1$2$$4" :
Solution
Instead of this:
Since
This
The indentation is off in the else block, and the formatting doesn't follow common conventions:
It would be better to write this code like this:
In this code:
It's misleading that
then it's reassigned to something else inside the
This makes the reader look suspiciously at the code after the
Looking at the code,
Which means,
you're just reusing the variable inside the
But it's not a good practice to reuse a variable for more than one purpose.
It would be better to create a new variable inside the
for example:
var isInbox = $(this).parent().is(".icon-inbox") ? true : false;Since
.is(...) is boolean, you can use its returned value directly without the ternary operator:var isInbox = $(this).parent().is(".icon-inbox");This
else is pointless, just delete it:} else { }The indentation is off in the else block, and the formatting doesn't follow common conventions:
else if(rgx.test(title)){
title = title.replace(rgx, "[" + txt + "] ");
}
else {title = "[" + txt + "] " + title;}It would be better to write this code like this:
else if(rgx.test(title)){
title = title.replace(rgx, "[" + txt + "] ");
} else {
title = "[" + txt + "] " + title;
}In this code:
var rgx = (isInbox ? /\[\d+\]/ : /\[\+\d+\]/);
if (title.indexOf("•") > -1) {
rgx = isInbox ? /^(\• )\d+/ : /^(\[)+\d+/;
title = title.replace(rgx, "$1" + txt);
}
// ...It's misleading that
rgx is first assigned to something,then it's reassigned to something else inside the
if branch.This makes the reader look suspiciously at the code after the
if statement to see if rgx is used again for something.Looking at the code,
rgx is not used again.Which means,
you're just reusing the variable inside the
if branch.But it's not a good practice to reuse a variable for more than one purpose.
It would be better to create a new variable inside the
if branch,for example:
var rgx = (isInbox ? /\[\d+\]/ : /\[\+\d+\]/);
if (title.indexOf("•") > -1) {
var rgx2 = isInbox ? /^(\• )\d+/ : /^(\[)+\d+/;
title = title.replace(rgx2, "$1" + txt);
}Code Snippets
var isInbox = $(this).parent().is(".icon-inbox") ? true : false;var isInbox = $(this).parent().is(".icon-inbox");else if(rgx.test(title)){
title = title.replace(rgx, "[" + txt + "] ");
}
else {title = "[" + txt + "] " + title;}else if(rgx.test(title)){
title = title.replace(rgx, "[" + txt + "] ");
} else {
title = "[" + txt + "] " + title;
}var rgx = (isInbox ? /\[\d+\]/ : /\[\+\d+\]/);
if (title.indexOf("•") > -1) {
rgx = isInbox ? /^(\• )\d+/ : /^(\[)+\d+/;
title = title.replace(rgx, "$1" + txt);
}
// ...Context
StackExchange Code Review Q#73557, answer score: 7
Revisions (0)
No revisions yet.