No Issue - Implemented first run page

firstRun.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
 /* globals checkShareResource, getDocLink, openSharePopup, setLinks, E */

[Thomas Greiner, 2018/02/16 16:19:14]

Detail: We no longer use `checkShareResource()` and `openSharePopup()` so no
need to list them here. We can even remove those two functions from commons.js
now since this was the only page that was using them.

 
 "use strict";
 
 (function()
 {
-  function onDOMLoaded()
-  {
-    // Set up logo image
-    let logo = E("logo");
-    logo.src = "skin/abp-128.png";
-    let errorCallback = function()
-    {
-      logo.removeEventListener("error", errorCallback, false);
-      // We are probably in Chrome/Opera/Safari, the image has a different path.
-      logo.src = "icons/detailed/abp-128.png";
-    };
-    logo.addEventListener("error", errorCallback, false);
-
-    // Set up URLs
-    getDocLink("donate", (link) =>
-    {
-      E("donate").href = link;
-    });
-
-    getDocLink("contributors", (link) =>
-    {
-      E("contributors").href = link;
-    });
-
-    getDocLink("acceptable_ads_criteria", (link) =>
-    {
-      setLinks("acceptable-ads-explanation", link, openFilters);
-    });
-
-    getDocLink("contribute", (link) =>
-    {
-      setLinks("share-headline", link);
-    });
-
-    browser.runtime.sendMessage({
-      type: "app.get",
-      what: "issues"
-    }, (issues) =>
-    {
-      // Show warning if filterlists settings were reinitialized
-      if (issues.filterlistsReinitialized)
-      {
-        E("filterlistsReinitializedWarning").removeAttribute("hidden");
-        setLinks("filterlistsReinitializedWarning", openFilters);
-      }
-    });
-
-    updateSocialLinks();
-
-    ext.onMessage.addListener((message) =>
-    {
-      if (message.type == "subscriptions.respond")
-      {
-        updateSocialLinks();
-      }
-    });
-    browser.runtime.sendMessage({
-      type: "subscriptions.listen",
-      filter: ["added", "removed", "updated", "disabled"]
-    });
-  }
-
-  function updateSocialLinks()
-  {
-    for (let network of ["twitter", "facebook", "gplus"])
-    {
-      let link = E("share-" + network);
-      checkShareResource(link.getAttribute("data-script"), (isBlocked) =>
-      {
-        // Don't open the share page if the sharing script would be blocked
-        if (isBlocked)
-          link.removeEventListener("click", onSocialLinkClick, false);
-        else
-          link.addEventListener("click", onSocialLinkClick, false);
-      });
-    }
-  }
-
-  function onSocialLinkClick(event)
-  {
-    if (window.matchMedia("(max-width: 970px)").matches)
-      return;
-
-    event.preventDefault();
-
-    getDocLink(event.target.id, (link) =>
-    {
-      openSharePopup(link);
-    });
-  }
-
   function openFilters()
   {
     browser.runtime.sendMessage({type: "app.open", what: "options"});
   }
 
+  function navigationClick(event)
+  {
+    document.getElementById("navbar-menu").classList.toggle("visible");

[Thomas Greiner, 2018/02/16 16:19:12]

Detail: I don't mind using either `E()` or `document.getElementById()` but at
least we should be consistent by using either the one or the other but not both.

+  }
+
+  function initMenu()
+  {
+    document.getElementById("navbar-menu-toggle").addEventListener("click", navigationClick, false);

[Thomas Greiner, 2018/02/16 16:19:11]

Detail: The third function argument parameter is redundant. We used to pass it
in the past which is why it's still used for "DOMContentLoaded" but we have
since stopped doing that.

[Thomas Greiner, 2018/02/16 16:19:14]

Coding style: "Line length: 80 characters or less"
Also applies to line 50.

Note that you can now automatically detect and fix coding style issues by
running `npm run lint` (see https://github.com/adblockplus/adblockplusui#linting
as well as https://github.com/adblockplus/adblockplusui#installing-dependencies
for more info on that).

+  }
+
+  function onDOMLoaded()
+  {
+    const optionsTrigger = E("options-trigger");

[Thomas Greiner, 2018/02/16 16:19:13]

Coding style: "Use const for constant expressions that could as well have been
inlined (e.g. static parameters, imported modules, etc.)."

In this case the value of `optionsTrigger` is not a constant expression but a
function call.

+
+    optionsTrigger.addEventListener("click", (e) =>
+    {
+      e.preventDefault();
+      openFilters();
+    });
+
+    setLinks("first-column-description", " https://adblockplus.org/terms");

[Thomas Greiner, 2018/02/16 16:19:13]

As done with the Updates page, don't hardcode any links but instead use
documentation links.

+    setLinks("third-column-description", "https://adblockplus.org/acceptable-ads#optout");
+    setLinks("copyright-notice", "https://eyeo.com");
+
+    initMenu();
+  }
+
   document.addEventListener("DOMContentLoaded", onDOMLoaded, false);
 }());