Issue 5540 - implement notification

new-options.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
@@ skipping 517 matching lines @@
         alert(errors.join("\n"));
       else if (onSuccess)
         onSuccess();
     });
   }
 
   function openDocLink(id)
   {
     getDocLink(id, (link) =>
     {
-      if (id == "share-general")
-        openSharePopup(link);
-      else
-        location.href = link;
+      location.href = link;
     });
   }
 
   function switchTab(id)
   {
     location.hash = id;
   }
 
   function execAction(action, element)
   {
@@ skipping 340 matching lines @@
 
     getDocLink("subscriptions", (link) =>
     {
       setLinks("filter-lists-description", link);
     });
 
     E("custom-filters-raw").setAttribute("placeholder",
       getMessage("options_customFilters_edit_placeholder", ["/ads/track/*"]));
 
     // Help tab
-    getDocLink("faq", (link) =>
-    {
-      E("link-faq").setAttribute("href", link);
+    getDocLink("adblock_plus_report_issue", (link) =>
+    {
+      setLinks("report-issue", link);
+    });
+    getDocLink("adblock_plus_report_ad", (link) =>
+    {
+      setLinks("report-ad", link);
+    });
+    getDocLink("adblock_plus_report_bug", (link) =>
+    {
+      setLinks("report-bug", link);
+    });
+    getDocLink("reporter_other_link", (link) =>
+    {
+      setLinks("report-forum", link);
     });
     getDocLink("social_twitter", (link) =>
     {
-      E("link-twitter").setAttribute("href", link);
+      E("twitter").setAttribute("href", link);
     });
     getDocLink("social_facebook", (link) =>
     {
-      E("link-facebook").setAttribute("href", link);
+      E("facebook").setAttribute("href", link);
     });
     getDocLink("social_gplus", (link) =>
     {
-      E("link-gplus").setAttribute("href", link);
-    });
-    getDocLink("social_renren", (link) =>
-    {
-      E("link-renren").setAttribute("href", link);
+      E("google-plus").setAttribute("href", link);
     });
     getDocLink("social_weibo", (link) =>
     {
-      E("link-weibo").setAttribute("href", link);
-    });
-
-    // Set forum link
-    ext.backgroundPage.sendMessage({
-      type: "app.get",
-      what: "platform"
-    },
-    (platform) =>
-    {
-      ext.backgroundPage.sendMessage({
-        type: "app.get",
-        what: "application"
-      },
-      (application) =>
-      {
-        if (platform == "chromium" && application != "opera")
-          application = "chrome";
-
-        getDocLink(application + "_support", (link) =>
-        {
-          E("link-forum").setAttribute("href", link);
-        });
-      });
+      E("weibo").setAttribute("href", link);
     });
 
     E("dialog").addEventListener("keydown", function(e)
     {
       switch (getKey(e))
       {
         case "Escape":
           closeDialog();
           break;
         case "Tab":
@@ skipping 36 matching lines @@
 
   function closeDialog()
   {
     let dialog = E("dialog");
     dialog.setAttribute("aria-hidden", true);
     dialog.removeAttribute("aria-labelledby");
     document.body.removeAttribute("data-dialog");
     focusedBeforeDialog.focus();
   }
 
   function showNotification(text)

[ire, 2017/08/25 09:59:45]

This doesn't need to hold up this review, but we could manage focus a bit better
here. For a keyboard user, when they "click" on the "Add website" button, focus
disappears. 

I have 3 suggestions for how to handle this, in order of preference:

1. Move focus to the close notification button. This moves the attention of the
keyboard user to the notification, which is sort of the purpose of it. When the
user clicks to hide (or the notification times out), move their focus back to
the input field

2. Move focus back to the input field immediately. This makes sense since the
field was just cleared.

3. Move focus to the delete button for that website. This has the potential for
the user to mistakenly delete the website they just added, which is why it's my
last choice. 

It's up to you if you want to handle this in this review, or later on. 

[saroyanm, 2017/08/25 11:04:19]

It's bit difficult because we don't know where is user located when the
notification is being triggered, please note that the notification is not being
triggered when user click on Add button, but when a filter is being added, this
might also happen in the Advanced tab if user adds a filter that is a
whitlisting filter.

I do agree that we should improve the focus handling, but it's bit more complex
as it looks to be, so considering the fact that notification is being hidden
automatically I think it's fine keeping focus where it is. 

Also user don't need to navigate to the Add Website button, but trigger submit
by hitting enter.  

[ire, 2017/08/25 12:10:07]

Okay good point. In light of what you said, I think my second option is now my
preference (moving focus back to the input field after it's cleared). But
perhaps this is something we should discuss in the next meeting with Jeen to
help us decide what the best implementation would be

[saroyanm, 2017/08/25 12:22:55]

I agree

   {
     E("notification").setAttribute("aria-hidden", false);
     E("notification-text").textContent = text;
     setTimeout(hideNotification, 3000);
   }
 
   function hideNotification()
   {
     E("notification").setAttribute("aria-hidden", true);
     E("notification-text").textContent = "";
@@ skipping 373 matching lines @@
   });
   ext.backgroundPage.sendMessage({
     type: "subscriptions.listen",
     filter: ["added", "disabled", "homepage", "lastDownload", "removed",
              "title", "downloadStatus", "downloading"]
   });
 
   window.addEventListener("DOMContentLoaded", onDOMLoaded, false);
   window.addEventListener("hashchange", onHashChange, false);
 }