标记和队列项目的桌面通知

我们最近的选举结束后，我被选为这里代码审查主持人（谢谢，社区成员！）。在主持人工具中，我最想念的第一件事就是当有任何标志时的桌面通知。

由于不存在任何当前工具，我认为我只需要创建一个...

按照“分解您的问题，以便您组合解决方案”的方法（无论是否意识到，我都相信我通常会照做），我认为我将需要以下物品：

桌面通知
自动重新加载页面（也可能与AJAX请求一起使用）
它应该在Stack Exchange上起作用
加载/保存本地数据

起初我从SO和通用的Stack Exchange用户脚本模板中使用了一些建议，但后来我意识到我不需要那些东西（它们只是阻止了我使用某些GM_ *函数）。

该脚本在/review URL（在任何Stack Exchange站点的URL）上添加了指向/review/desktop-notifications的链接，从技术上讲，该链接指向404页面，但获得了一些附加信息

/review链接如下所示：

/review/desktop-notifications如下所示： br />

桌面通知如下所示：

源代码可作为我的SE脚本的一部分获得GitHub存储库。感谢所有功能请求和请求请求。

该代码已经过Tampermonkey 3.11 for Google Chrome的测试，但我相信它也可以与其他用户脚本处理程序一起使用。

代码

// ==UserScript==
// @name Moderator Flag Notification
// @author Simon Forsberg
// @description Shows a desktop notification when there are flags or review items to be handled.
// @namespace https://github.com/Zomis/SE-Scripts
// @grant GM_getValue
// @grant GM_setValue
// @grant GM_notification
// @match *://*.stackexchange.com/review*
// @match *://*.stackoverflow.com/review*
// @match *://*.superuser.com/review*
// @match *://*.serverfault.com/review*
// @match *://*.askubuntu.com/review*
// @match *://*.stackapps.com/review*
// @match *://*.mathoverflow.net/review*
// ==/UserScript==

if (window.location.href.indexOf('/desktop-notifications') === -1) {
    $('.tools-rev h1').append('<span class="lsep">|</span><a href="/review/desktop-notifications">Desktop Notifications</a></h1>');
} else {
    var KEY_NEXT = 'NextReload';
    var DELAY = 60 * 1000;
    var currentTime = Date.now ? Date.now() : new Date().getTime();
    var lastTime = GM_getValue(KEY_NEXT, 0);
    var nextTime = currentTime + DELAY;
    GM_setValue(KEY_NEXT, nextTime);

    var timeDiff = Math.abs(lastTime - currentTime);
    setTimeout(function(){ window.location.reload(); }, DELAY);

    $('.subheader h1').html('Desktop Notifications');
    $('.leftcol').html('Keep your browser open on this page and it will be automatically reloaded and alert you when something wants your attention.').removeClass('leftcol');
    $('.rightcol').remove();

    var title = document.title.split(' - '); // keep the site name
    document.title = 'Desktop Notifications - ' + title[1];

    // a way to detect that the script is being executed because of an automatic script reload, not by the user.
    if (timeDiff <= DELAY * 2) {
        var notifications = [];

        var topbarFlag = $('.topbar-links .mod-only .icon-flag .flag-count').html();
        var topbarFlagCount = parseInt(topbarFlag);
        if (topbarFlagCount > 0) {
            notifications.push(topbarFlagCount + ' Flags');
        }

        var reviewItems = $('.icon-tools-flag span');
        var reviewCount = 0;
        if (reviewItems.length > 0) {
            reviewCount = parseInt(reviewItems.html());
            if (reviewCount > 0) {
                notifications.push(reviewCount + ' Review Items');
            }
        }

        if (notifications.length > 0) {
            var details = {
                title: document.title,
                text: notifications.join('\n')
            };
            GM_notification(details, null);
        }
    }
}

主要问题

我的代码可读吗？
我遵循最佳实践吗？？
我可以很好地使用GM_ *功能吗？
其他评论也值得赞赏。

次要更新：

使用新的Stack交换顶部栏，将一行更改为此：

var topbarFlag = $('.top-bar .indicator-badge').html();

#1 楼

幻数

您在消除代码顶部的所有幻数/值方面做得非常好。但是，仍然有这句话：

var DELAY = 60 * 1000;

60是哪里来的？还有1000吗？

这些仍然是幻数，但是不值得使用新的常数；我只需要添加一条简单的注释来解释DELAY的设置。
如果需要改变自然，应该责备自然。另请参阅
var DELAY = 60 * 1000;是一个神奇的数字。
解释60和1000的含义是好的（甚至是必需的），但要给它们自己的常量...

基数参数

您在代码中多次调用了函数100。就像这里：

var topbarFlagCount = parseInt(topbarFlag);

这里：

reviewCount = parseInt(reviewItems.html());

虽然不为人所知，但parseInt函数实际上需要一个名为parseInt的第二个参数。此参数用于指定第一个参数位于哪个数字底数。

最常见的是，此数字为10（以10为底数或十进制）。您可以在此处阅读有关此参数的更多信息。例如，您可以将其写为以下一行之一：

var topbarFlagCount = parseInt(topbarFlag, 10);

回到以前的帖子

不久前，您还发布了另一个UserScript问题。为了与FireFox兼容，您应该像这样封装UserScript的开始部分：

/** @preserve
// ==UserScript==
.....
// ==/UserScript==
*/

有很多UserScript约定是您无法遵循的
到。

首先，对于Firefox，您需要将标头部分包装到一个
“保留的”注释块中：

...

需要保留，以便在
之后保留注释块javascript已处理。这样，“编译”版本可以保留引用，并使FireFox知道它的含义。

其他浏览器可能没有相同的要求。 />

压缩行

此行：

setTimeout(function(){ window.location.reload(); }, DELAY);

先请我阅读几次，然后再阅读可以匹配radix和{}。我认为，如果您将其扩展，它将更加可读，例如：

setTimeout(function() {
    window.location.reload(); 
}, DELAY);

包装代码

这是我想指出的只是一小件事。

您应该将代码包含在以下内容中：

(function (window, undefined) {
    (function ($) {
        [code]
    })(window.jQuery);
})(Function("return this")());

原因是因为名称()，window和undefined都是有效的变量名称，可以轻松地在任何变量中使用。

仅当您没有编写任何重要内容时才执行此操作。

对于例如，打开您的控制台并键入：

var $ = 3;
console.log($);

您将获得输出

3

对于$和window来说，它们不会发生同样的事情：如果您要尝试：。这是因为对于这些变量，覆盖它们只会在本地更改其值；它们不会在全局范围内更改。

在此聊天对话中了解更多。

基本上，通过将代码包装在这些匿名函数中，可以确保无论其他脚本使用这些变量名做什么，这些变量将始终是您想要的方式。用于设置undefined的对象需要使用$对象，第一个匿名函数确保该对象是有效的。

这里还有更多的理由说明为什么$具有匿名函数。

还原$别名，然后创建并执行一个函数，以在函数范围内将$作为jQuery别名提供给
。在
函数内部，原始$对象不可用。对于大多数不依赖任何其他库的插件来说，这很好。

编辑：

使用内部匿名的原因该函数是由于函数window引起的。如果文档前面的任何代码要调用此函数，则别名$将与jQuery.noConflict分开。此功能将别名恢复为$，以防万一调用此功能。

在此聊天对话中了解更多。

感谢Ismael Miguel。

拆分代码

在较大的内部jQuery语句中，您有两个部分：一个用于获取所有标志，另一个用于获取审阅中的所有内容队列。

要减轻主代码的负担，您应该将它们拆分为各自的函数，可能分别称为jQuery和if。 “;

您的代码应带有getFlags。

\ $ \ begingroup \ $
我知道parseInt的工作原理，但是当我从已经将值写入基数10的DOM中读取时，是否真的需要指定基数10？例如，永远不会出现parseInt（'08'）。
\ $ \ endgroup \ $
–西蒙·福斯伯格
15年8月20日在10:57

\ $ \ begingroup \ $
@SimonAndréForsberg不，但是您应该始终使用基数。这是额外的2个字节，可以节省您数小时的头痛。只是权重。
\ $ \ endgroup \ $
–伊斯梅尔·米格尔（Ismael Miguel）
15年8月20日在11:00

\ $ \ begingroup \ $
@SimonAndréForsberg并不是完全必要的，但是（1）它可以提供意外且难以找到的结果，并且（2）MDN是一种非常流行且可靠的JavaScript来源，据说总是指定它。有关更多信息，请访问该链接。
\ $ \ endgroup \ $
– SirPython
15年8月20日在15:25

#2 楼

SirPython已经给出了非常不错的评价。

，但是还有一些挑剔的要点。只是很小的东西。

您具有以下一行：

var currentTime = Date.now ? Date.now() : new Date().getTime();

您可以使用以下任何一种方式：

var currentTime = new Date().getTime();
var currentTime = +(new Date());

我实际上更喜欢后一种，因为它速度更快。我在某处看到了一个jsperf。

此外，您还有一个与此类似的if：

if (notifications.length > 0) {

您没有需要> 0，因为您不能具有undefined元素或-1元素。只需将其更改为：

if (notifications.length) {

继续击败可怜的if s，让我们来看一下：

var reviewItems = $('.icon-tools-flag span');
var reviewCount = 0;
if (reviewItems.length > 0) {
    reviewCount = parseInt(reviewItems.html());
    if (reviewCount > 0) {
        notifications.push(reviewCount + ' Review Items');
    }
}

为什么当您立即将reviewCound设置为正确的值时，为什么整个混乱？

像这样：

var reviewItems = $('.icon-tools-flag span');
if (reviewItems.length > 0) {
    var reviewCount = parseInt(reviewItems.html(), 10);
    if (reviewCount > 0) {
        notifications.push(reviewCount + ' Review Items');
    }
}

如果您真的想减少脂肪：

var reviewCount = parseInt($('.icon-tools-flag span').html(), 10);
if (reviewCount > 0) {
    notifications.push(reviewCount + ' Review Items');
}

在那里，您的整个块分为4行。而且仍然可读！

要考虑的事情是，您正在导航到触发错误404的页面。我建议将/404添加到URL，现在就可以了/review/desktop-notifications/404，这将是实际尝试获取404页面的记录。

或者您可以寻找其他方法，例如在其中创建代码或弹出窗口的情况下创建一个空的<iframe>。

\ $ \ begingroup \ $
我不同意将/ 404添加到URL。还有其他用户脚本也以相同的方式进行操作。我使用Date.now吗？ ...：...;由于存在一个我建议推荐的SO问题，因此我确信还有数不胜数的其他方法可以做到。由于我习惯使用强类型语言，因此我更喜欢if（a> 0）而不是if（a）
\ $ \ endgroup \ $
–西蒙·福斯伯格
15年8月20日在10:53

\ $ \ begingroup \ $
@SimonAndréForsberg/ 404只是您可能想尝试的东西。实际上，这超出了审查范围。而且我知道您已经习惯了（a> 0）。但是C还是一种强类型语言，并且if（a）与if（a> 0）的工作原理相同。我了解您采用这种方式的原因。但是，总的来说，您实际上并不需要> 0。
\ $ \ endgroup \ $
–伊斯梅尔·米格尔（Ismael Miguel）
2015年8月20日在11:01

\ $ \ begingroup \ $
好的，“强类型语言”不是正确的术语……“类型推断”也许吗？我还是不喜欢我知道我不需要它，但是疼吗？
\ $ \ endgroup \ $
–西蒙·福斯伯格
15年8月20日在11:11

\ $ \ begingroup \ $
@SimonAndréForsberg不，但是这会让我们想知道您的代码是否损坏。因为如果您正在检查notifications.length> 0，则可能会得到负的长度。这意味着一个错误。强制对其进行检查。但是，在这段代码中，它不会有所作为。在较大的项目中，它将。
\ $ \ endgroup \ $
–伊斯梅尔·米格尔（Ismael Miguel）
15年8月20日在11:16

\ $ \ begingroup \ $
我完全不同意这种评估。仅仅因为您编写if（notifications.length> 0）并不意味着您可能会得到负数长度。
\ $ \ endgroup \ $
–西蒙·福斯伯格
15年8月20日在11:30

编程黑洞网

标记和队列项目的桌面通知

#1 楼

评论

#2 楼

评论