Issue 6003 - Better handling of no-content-for-platform-message on help.eyeo.com

static/js/main.js

 (function()
 {
   document.addEventListener("DOMContentLoaded", function()
   {
 
     /**************************************************************************
      * General
      **************************************************************************/
 
     // Change html class name from "no-js" to "js"
@@ skipping 217 matching lines @@
     }
 
     /**************************************************************************
      * BrowserSelect
      **************************************************************************/
 
     function BrowserSelect(select)
     {
       this.select = select;
       CustomSelect.apply(this, [this.select]);
+      this.noContentMessage = document.getElementById("no-content-for-platform-message");
       
       this.BROWSER_STORAGE_KEY = "BROWSER";
       this.BROWSER_AUTODETECTED_STORAGE_KEY = "BROWSER_AUTODETECTED";
       this.SUPPORTED_BROWSERS = ["chrome", "opera", "samsungBrowser", 
                                  "yandexbrowser", "maxthon", "msie", 
                                  "msedge", "firefox", "ios", "safari"];
       this.DEFAULT_BROWSER = "chrome";
       
+      this.setCurrentArticleSupportedBrowsers();
+      
       this.select
         .addEventListener("click", this._onClickOrKeyDown.bind(this), false);
       
       this.select
         .addEventListener("keydown", this._onClickOrKeyDown.bind(this), false);
 
+      this.noContentMessage
+        .addEventListener("click", this._onClickNoContentMessage.bind(this), false);
+
       var storedBrowser = localStorage.getItem(this.BROWSER_STORAGE_KEY);
       if (storedBrowser) this.selectOption(storedBrowser);
       else this.detectBrowser();
     }
 
     BrowserSelect.prototype = Object.create(CustomSelect.prototype);
     BrowserSelect.prototype.constructor = BrowserSelect;
 
+    BrowserSelect.prototype.setCurrentArticleSupportedBrowsers = function()
+    {
+      for (var i = 0; i < this.SUPPORTED_BROWSERS.length; i++)
+      {
+        var supportedBrowser = this.SUPPORTED_BROWSERS[i];
+        if (!document.querySelector(".platform-"+supportedBrowser))
+        {
+          this.noContentMessage.querySelector("."+supportedBrowser)
+              .setAttribute("hidden", "true");
+        }
+      }
+    };
+
     BrowserSelect.prototype.detectBrowser = function()
     {
       for (var i = 0; i < this.SUPPORTED_BROWSERS.length; i++)
       {
         var supportedBrowser = this.SUPPORTED_BROWSERS[i];
         if (bowser[supportedBrowser])
         {
           localStorage.setItem(this.BROWSER_AUTODETECTED_STORAGE_KEY, "true");
           return this.selectOption(supportedBrowser);
         }
@@ skipping 29 matching lines @@
       {
         var autodetected = document
                               .getElementById("browser-select-autodetected")
                               .innerHTML;
         selectedOption += "<span class='muted'>(" + autodetected + ")</span>";
       }
 
       this.select
           .querySelector(".custom-select-selected")
           .innerHTML = selectedOption;
+
+      this.noContentMessage.setAttribute("hidden", "true");

[juliandoucette, 2017/12/05 16:38:52]

Why are you setting and then conditionally removing this attribute instead of
*just* conditionally setting it?

[ire, 2017/12/06 14:43:39]

Done.

[ire, 2017/12/06 14:43:39]

Since this is the default I wanted to always set it and only unhide it. But I
can move this to the else part of the if statement.

[juliandoucette, 2017/12/11 15:15:27]

Acknowledged.

       
       if (!document.querySelector(".platform-" + browser))
       {
-        this.handleNoContentForBrowser(browser);
+        this.noContentMessage.removeAttribute("hidden");
       }
     };
 
-    BrowserSelect.prototype.handleNoContentForBrowser = function(browser)
-    {
-      var section = document.createElement("section");
-      section.classList.add("platform-" + browser);
-      section.innerHTML = document
-                            .getElementById("no-content-for-platform-message")
-                            .innerHTML;
-
-      document
-        .querySelector(".article-body")
-        .insertAdjacentElement("afterbegin", section);
-    }
-
     BrowserSelect.prototype._onClickOrKeyDown = function(event)
     {
       var option = event.target.closest(".custom-select-option");
       if (!option) return;
 
       var IS_ENTER_KEY = event.key == "Enter" || event.keyCode == 13;
       if (event.keyCode && !IS_ENTER_KEY) return;
 
       localStorage.removeItem(this.BROWSER_AUTODETECTED_STORAGE_KEY);
 
       // Uncheck previously checked option
       this.select
           .querySelector("[aria-checked='true']")
           .setAttribute("aria-checked", "false");
 
       this.selectOption(option.getAttribute("data-value"));
 
       this.close();
     };
 
+    BrowserSelect.prototype._onClickNoContentMessage = function(event)
+    {
+      if (event.target.tagName != "BUTTON") return;
+
+      var browser = event.target.parentElement.className;

[juliandoucette, 2017/12/05 16:38:52]

suggest: use a data value e.g. data-browser instead of a class name

- because this is "data" and that's what "data attributes" are for
- because classes can be / are often modified by separate scripts (generally,
not particularly on abp.org)

[ire, 2017/12/06 14:43:39]

I agree with you. Done.

For some reason (which I can't figure out right now) I used the name
`data-value` in the browser select dropdown, so I'm using it here as well. I
should have called it `data-browser` and might change it in a different issue 

[juliandoucette, 2017/12/11 15:15:27]

I think data-value makes sense in the dropdown because it emulates option.value.
I don't think that it makes sense on an anchor for the purpose of selecting
though because the "value like" property of an achor is it's href.

[ire, 2017/12/12 10:21:13]

Acknowledged.

+      this.selectOption(browser);
+    };
+
     var browserSelect = document.getElementById("browser-select");
     if (browserSelect)
     {
       new BrowserSelect(browserSelect);
     }
 
   }, false);
 }());