[devdocsgjs/main: 657/1867] Implement review suggestions




commit c28305b0b76f1697607ad127045c281c6a8d377e
Author: Jasper van Merle <jaspervmerle gmail com>
Date:   Fri Jul 12 03:06:04 2019 +0200

    Implement review suggestions

 assets/javascripts/app/serviceworker.coffee        |  6 ++--
 assets/javascripts/app/settings.coffee             | 32 ++++++----------------
 assets/javascripts/app/update_checker.coffee       |  2 +-
 assets/javascripts/templates/error_tmpl.coffee     |  2 +-
 .../templates/pages/offline_tmpl.coffee            |  4 +--
 assets/javascripts/views/layout/resizer.coffee     |  7 +----
 docs/maintainers.md                                |  2 +-
 views/service-worker.js.erb                        | 23 ++++++----------
 8 files changed, 27 insertions(+), 51 deletions(-)
---
diff --git a/assets/javascripts/app/serviceworker.coffee b/assets/javascripts/app/serviceworker.coffee
index 188a7e42..40235566 100644
--- a/assets/javascripts/app/serviceworker.coffee
+++ b/assets/javascripts/app/serviceworker.coffee
@@ -9,8 +9,10 @@ class app.ServiceWorker
     @notifyUpdate = true
 
     navigator.serviceWorker.register(app.config.service_worker_path, {scope: '/'})
-      .then((registration) => @updateRegistration(registration))
-      .catch((error) -> console.error 'Could not register service worker:', error)
+      .then(
+        (registration) => @updateRegistration(registration),
+        (error) -> console.error('Could not register service worker:', error)
+      )
 
   update: ->
     return unless @registration
diff --git a/assets/javascripts/app/settings.coffee b/assets/javascripts/app/settings.coffee
index 0540bfb7..af298250 100644
--- a/assets/javascripts/app/settings.coffee
+++ b/assets/javascripts/app/settings.coffee
@@ -20,7 +20,6 @@ class app.Settings
   ]
 
   LAYOUTS: ['_max-width', '_sidebar-hidden', '_native-scrollbars']
-  SIDEBAR_HIDDEN_LAYOUT = '_sidebar-hidden'
 
   @defaults:
     count: 0
@@ -112,36 +111,21 @@ class app.Settings
     return
 
   initLayout: ->
-    @toggleDark(@get('dark'))
+    @toggleDark(@get('dark') is 1)
     @toggleLayout(layout, @hasLayout(layout)) for layout in @LAYOUTS
-    @addResizerCSS()
+    @initSidebarWidth()
 
   toggleDark: (enable) ->
     classList = document.documentElement.classList
-    classList[if enable then 'remove' else 'add']('_theme-default')
-    classList[if enable then 'add' else 'remove']('_theme-dark')
+    classList.toggle('_theme-default', !enable)
+    classList.toggle('_theme-dark', enable)
 
   toggleLayout: (layout, enable) ->
     classList = document.body.classList
-    classList[if enable then 'add' else 'remove'](layout) unless layout is SIDEBAR_HIDDEN_LAYOUT
-    classList[if $.overlayScrollbarsEnabled() then 'add' else 'remove']('_overlay-scrollbars')
+    classList.toggle(layout, enable) unless layout is '_sidebar-hidden'
+    classList.toggle('_overlay-scrollbars', $.overlayScrollbarsEnabled())
 
-  addResizerCSS: ->
+  initSidebarWidth: ->
     size = @get('size')
-    size = if size then size + 'px' else '20rem'
-
-    css = """
-      ._container { margin-left: #{size}; }
-      ._header, ._list { width: #{size}; }
-      ._list-hover.clone { min-width: #{size}; }
-      ._notice, ._path, ._resizer { left: #{size}; }
-    """
-
-    style = document.createElement('style')
-    style.type = 'text/css'
-    style.appendChild(document.createTextNode(css))
-    style.setAttribute('data-size', size)
-    style.setAttribute('data-resizer', '')
-
-    document.head.appendChild(style)
+    document.documentElement.style.setProperty('--sidebarWidth', size + 'px') if size
     return
diff --git a/assets/javascripts/app/update_checker.coffee b/assets/javascripts/app/update_checker.coffee
index b98c6563..3558d6bc 100644
--- a/assets/javascripts/app/update_checker.coffee
+++ b/assets/javascripts/app/update_checker.coffee
@@ -3,7 +3,7 @@ class app.UpdateChecker
     @lastCheck = Date.now()
 
     $.on window, 'focus', @onFocus
-    app.serviceWorker.on 'updateready', @onUpdateReady if app.serviceWorker
+    app.serviceWorker?.on 'updateready', @onUpdateReady
 
     setTimeout @checkDocs, 0
 
diff --git a/assets/javascripts/templates/error_tmpl.coffee b/assets/javascripts/templates/error_tmpl.coffee
index 4b696d40..9cca1f9d 100644
--- a/assets/javascripts/templates/error_tmpl.coffee
+++ b/assets/javascripts/templates/error_tmpl.coffee
@@ -13,7 +13,7 @@ app.templates.notFoundPage = ->
 app.templates.pageLoadError = ->
   error """ The page failed to load. """,
         """ It may be missing from the server (try reloading the app) or you could be offline (try <a 
href="/offline">installing the documentation for offline usage</a> when online again).<br>
-            If you keep seeing this, you're likely behind a proxy or firewall that blocks cross-domain 
requests. """,
+            If you're online and you keep seeing this, you're likely behind a proxy or firewall that blocks 
cross-domain requests. """,
         """ #{back} &middot; <a href="/##{location.pathname}" target="_top" class="_error-link">Reload</a>
             &middot; <a href="#" class="_error-link" data-retry>Retry</a> """
 
diff --git a/assets/javascripts/templates/pages/offline_tmpl.coffee 
b/assets/javascripts/templates/pages/offline_tmpl.coffee
index e8b262aa..436cf1ae 100644
--- a/assets/javascripts/templates/pages/offline_tmpl.coffee
+++ b/assets/javascripts/templates/pages/offline_tmpl.coffee
@@ -25,8 +25,8 @@ app.templates.offlinePage = (docs) -> """
   <h2 class="_block-heading">Questions & Answers</h2>
   <dl>
     <dt>How does this work?
-    <dd>Each page is cached as a key-value pair in <a 
href="https://developer.mozilla.org/en-US/docs/Web/API/IndexedDB_API";>IndexedDB</a> (downloaded from a single 
file).<br>
-        The app also uses <a 
href="https://developer.mozilla.org/en-US/docs/Web/API/Service_Worker_API/Using_Service_Workers";>Service 
Workers</a> and <a href="https://developer.mozilla.org/en-US/docs/Web/API/Web_Storage_API";>localStorage</a> 
to cache the assets and index files.
+    <dd>Each page is cached as a key-value pair in <a 
href="https://devdocs.io/dom/indexeddb_api";>IndexedDB</a> (downloaded from a single file).<br>
+        The app also uses <a href="https://devdocs.io/dom/service_worker_api/using_service_workers";>Service 
Workers</a> and <a href="https://devdocs.io/dom/web_storage_api";>localStorage</a> to cache the assets and 
index files.
     <dt>Can I close the tab/browser?
     <dd>#{canICloseTheTab()}
     <dt>What if I don't update a documentation?
diff --git a/assets/javascripts/views/layout/resizer.coffee b/assets/javascripts/views/layout/resizer.coffee
index 8f0ce9c4..5584bfbe 100644
--- a/assets/javascripts/views/layout/resizer.coffee
+++ b/assets/javascripts/views/layout/resizer.coffee
@@ -11,9 +11,6 @@ class app.views.Resizer extends app.View
   init: ->
     @el.setAttribute('draggable', 'true')
     @appendTo $('._app')
-
-    @style = $('style[data-resizer]')
-    @size = @style.getAttribute('data-size')
     return
 
   MIN = 260
@@ -24,13 +21,11 @@ class app.views.Resizer extends app.View
     return unless value > 0
     value = Math.min(Math.max(Math.round(value), MIN), MAX)
     newSize = "#{value}px"
-    @style.innerHTML = @style.innerHTML.replace(new RegExp(@size, 'g'), newSize)
-    @size = newSize
+    document.documentElement.style.setProperty('--sidebarWidth', newSize)
     app.settings.setSize(value) if save
     return
 
   onDragStart: (event) =>
-    @style.removeAttribute('disabled')
     event.dataTransfer.effectAllowed = 'link'
     event.dataTransfer.setData('Text', '')
     $.on(window, 'dragover', @onDrag)
diff --git a/docs/maintainers.md b/docs/maintainers.md
index 455e6501..8f1554fa 100644
--- a/docs/maintainers.md
+++ b/docs/maintainers.md
@@ -84,7 +84,7 @@ In addition to the [publicly-documented commands](https://github.com/freeCodeCam
 
 Once docs have been uploaded via `thor docs:upload` (if applicable), deploying DevDocs is as simple as 
running `git push heroku master`. See [Heroku's documentation](https://devcenter.heroku.com/articles/git) for 
more information.
 
-- If you're deploying documentation updates, verify that the documentations work properly once the deploy is 
done (you will need to reload [devdocs.io](https://devdocs.io/) a couple times for the service worker to 
update and the new version to load).
+- If you're deploying documentation updates, verify that the documentations work properly once the deploy is 
done. Keep in mind that you'll need to wait a few seconds for the service worker to finish caching the new 
assets. You should see a "DevDocs has been updated" notification appear when the caching is done, after which 
you need to refresh the page to see the changes.
 - If you're deploying frontend changes, monitor [Sentry](https://sentry.io/devdocs/devdocs-js/) for new JS 
errors once the deploy is done.
 - If you're deploying server changes, monitor New Relic (accessible through [the Heroku 
dashboard](https://dashboard.heroku.com/apps/devdocs)) for Ruby exceptions and throughput or response time 
changes once the deploy is done.
 
diff --git a/views/service-worker.js.erb b/views/service-worker.js.erb
index d9eb4a6c..8d5698a7 100644
--- a/views/service-worker.js.erb
+++ b/views/service-worker.js.erb
@@ -35,20 +35,15 @@ self.addEventListener('fetch', event => {
     const cachedResponse = await caches.match(event.request);
     if (cachedResponse) return cachedResponse;
 
-    try {
-      const response = await fetch(event.request);
-      return response;
-    } catch (err) {
-      const url = new URL(event.request.url);
-
-      <%# Attempt to return the index page from the cache if the user is visiting a url like 
devdocs.io/javascript/global_objects/array/find %>
-      <%# The index page will make sure the correct documentation or a proper offline page is shown %>
-      if (url.origin === location.origin && !url.pathname.includes('.')) {
-        const cachedIndex = await caches.match('/');
-        if (cachedIndex) return cachedIndex;
-      }
-
-      throw err;
+    const url = new URL(event.request.url);
+
+    <%# Attempt to return the index page from the cache if the user is visiting a url like 
devdocs.io/offline or devdocs.io/javascript/global_objects/array/find %>
+    <%# The index page will handle the routing %>
+    if (url.origin === location.origin && !url.pathname.includes('.')) {
+      const cachedIndex = await caches.match('/');
+      if (cachedIndex) return cachedIndex;
     }
+
+    return fetch(event.request);
   })());
 });


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]