Browse Source

Implement review suggestions

Jasper van Merle 6 years ago
parent
commit
c28305b0b7

+ 4 - 2
assets/javascripts/app/serviceworker.coffee

@@ -9,8 +9,10 @@ class app.ServiceWorker
     @notifyUpdate = true
     @notifyUpdate = true
 
 
     navigator.serviceWorker.register(app.config.service_worker_path, {scope: '/'})
     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: ->
   update: ->
     return unless @registration
     return unless @registration

+ 8 - 24
assets/javascripts/app/settings.coffee

@@ -20,7 +20,6 @@ class app.Settings
   ]
   ]
 
 
   LAYOUTS: ['_max-width', '_sidebar-hidden', '_native-scrollbars']
   LAYOUTS: ['_max-width', '_sidebar-hidden', '_native-scrollbars']
-  SIDEBAR_HIDDEN_LAYOUT = '_sidebar-hidden'
 
 
   @defaults:
   @defaults:
     count: 0
     count: 0
@@ -112,36 +111,21 @@ class app.Settings
     return
     return
 
 
   initLayout: ->
   initLayout: ->
-    @toggleDark(@get('dark'))
+    @toggleDark(@get('dark') is 1)
     @toggleLayout(layout, @hasLayout(layout)) for layout in @LAYOUTS
     @toggleLayout(layout, @hasLayout(layout)) for layout in @LAYOUTS
-    @addResizerCSS()
+    @initSidebarWidth()
 
 
   toggleDark: (enable) ->
   toggleDark: (enable) ->
     classList = document.documentElement.classList
     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) ->
   toggleLayout: (layout, enable) ->
     classList = document.body.classList
     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 = @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
     return

+ 1 - 1
assets/javascripts/app/update_checker.coffee

@@ -3,7 +3,7 @@ class app.UpdateChecker
     @lastCheck = Date.now()
     @lastCheck = Date.now()
 
 
     $.on window, 'focus', @onFocus
     $.on window, 'focus', @onFocus
-    app.serviceWorker.on 'updateready', @onUpdateReady if app.serviceWorker
+    app.serviceWorker?.on 'updateready', @onUpdateReady
 
 
     setTimeout @checkDocs, 0
     setTimeout @checkDocs, 0
 
 

+ 1 - 1
assets/javascripts/templates/error_tmpl.coffee

@@ -13,7 +13,7 @@ app.templates.notFoundPage = ->
 app.templates.pageLoadError = ->
 app.templates.pageLoadError = ->
   error """ The page failed to load. """,
   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>
         """ 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>
         """ #{back} &middot; <a href="/##{location.pathname}" target="_top" class="_error-link">Reload</a>
             &middot; <a href="#" class="_error-link" data-retry>Retry</a> """
             &middot; <a href="#" class="_error-link" data-retry>Retry</a> """
 
 

+ 2 - 2
assets/javascripts/templates/pages/offline_tmpl.coffee

@@ -25,8 +25,8 @@ app.templates.offlinePage = (docs) -> """
   <h2 class="_block-heading">Questions & Answers</h2>
   <h2 class="_block-heading">Questions & Answers</h2>
   <dl>
   <dl>
     <dt>How does this work?
     <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?
     <dt>Can I close the tab/browser?
     <dd>#{canICloseTheTab()}
     <dd>#{canICloseTheTab()}
     <dt>What if I don't update a documentation?
     <dt>What if I don't update a documentation?

+ 1 - 6
assets/javascripts/views/layout/resizer.coffee

@@ -11,9 +11,6 @@ class app.views.Resizer extends app.View
   init: ->
   init: ->
     @el.setAttribute('draggable', 'true')
     @el.setAttribute('draggable', 'true')
     @appendTo $('._app')
     @appendTo $('._app')
-
-    @style = $('style[data-resizer]')
-    @size = @style.getAttribute('data-size')
     return
     return
 
 
   MIN = 260
   MIN = 260
@@ -24,13 +21,11 @@ class app.views.Resizer extends app.View
     return unless value > 0
     return unless value > 0
     value = Math.min(Math.max(Math.round(value), MIN), MAX)
     value = Math.min(Math.max(Math.round(value), MIN), MAX)
     newSize = "#{value}px"
     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
     app.settings.setSize(value) if save
     return
     return
 
 
   onDragStart: (event) =>
   onDragStart: (event) =>
-    @style.removeAttribute('disabled')
     event.dataTransfer.effectAllowed = 'link'
     event.dataTransfer.effectAllowed = 'link'
     event.dataTransfer.setData('Text', '')
     event.dataTransfer.setData('Text', '')
     $.on(window, 'dragover', @onDrag)
     $.on(window, 'dragover', @onDrag)

+ 1 - 1
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.
 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 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.
 - 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.
 
 

+ 9 - 14
views/service-worker.js.erb

@@ -35,20 +35,15 @@ self.addEventListener('fetch', event => {
     const cachedResponse = await caches.match(event.request);
     const cachedResponse = await caches.match(event.request);
     if (cachedResponse) return cachedResponse;
     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);
   })());
   })());
 });
 });