Issue 5912 - Hide/Show Content based on selected browser on help.eyeo.com article pages

static/js/main.js

Index: static/js/main.js
===================================================================
--- a/static/js/main.js
+++ b/static/js/main.js
@@ -301,18 +301,36 @@
                               .getElementById('browser-select-autodetected')
                               .innerHTML;
         selectedOption += "<span class='muted'>(" + autodetected + ")</span>";
       }
 
       this.select
           .querySelector(".custom-select-selected")
           .innerHTML = selectedOption;
+      
+      this.checkBrowserContent(browser);
     };
 
+    BrowserSelect.prototype.checkBrowserContent = function(browser)

[juliandoucette, 2017/11/06 15:56:05]

NIT: I don't think that this method name is descriptive enough. Check browser
content for what? What does it do? What does it return? I don't know from the
name. Do you know what I mean?

[ire, 2017/11/08 07:46:59]

I think you're right.

Since the method was essentially do 2 things (1. checking if there is content
and 2. handling the no content state), I decided to remove the first to the
previous function, and only call this function if there is no content.

Renamed to handleNoContentForBrowser()

+    {
+      var browserContent = document.querySelector(".platform-" + browser);
+      if (browserContent) return;
+
+      var section = document.createElement("section");

[juliandoucette, 2017/11/06 15:56:05]

TOL: This could be done with fewer lines of code e.g.

document.getElementById("tutorial").innerHTML += 
  document.getElementById("not-supported-message").innerHTML
    .replace("${browser}", browser);

If...
- The <section> was in the template
- The article body had an id (e.g. tutorial)
- It doesn't matter where the unsupported message is inserted (beginning or end)

[ire, 2017/11/08 07:46:59]

I think it does matter where the unsupported message is inserted. Since it
explicitly says "use the dropdown menu above to choose a different browser", it
makes sense to place it directly below the dropdown menu.

I also need to add a class to the <section> depending on what browser it is.

Using `innerHTML +=` isn't great performance-wise because it causes the entire
innerHTML of the element to be reloaded. Using `insertAdjacentHTML` will only
affect the particular position the element is being written to.

+      section.classList.add("platform-" + browser);
+      section.innerHTML = document
+                            .getElementById("no-content-for-platform-message")
+                            .innerHTML;
+
+      document

[juliandoucette, 2017/11/06 10:45:00]

Why insert this instead of adding a class to the body in order to show the
no-content-for-platform-message like any other section?

[ire, 2017/11/06 14:27:20]

I think it makes sense semantically this way. Because the message *is* the
content for that platform. For example we may want to personalise the message
for each browser, e.g. "There's no content for ${browser}". The way I see it,
the only reason it isn't already in the markup is because of no-js/crawlers. 

Also I don't see the no-content-for-platform-message as a section by itself. So
I don't think I would want a class in the body to display it. I would reserve
those classes to only display the user agent, not to determine if there is
content for that user agent or not. 

+        .querySelector(".article-body")
+        .insertAdjacentElement('afterbegin', section);
+    }
+
     BrowserSelect.prototype._onClickOrKeyDown = function(event)
     {
       if (!event.target.classList.contains("custom-select-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);