From a8073b3a12c1d8141c727fa39a4723f1f5a44cfe Mon Sep 17 00:00:00 2001
From: Thibaut Courouble <thibaut@me.com>
Date: Sat, 15 Apr 2017 11:30:47 -0400
Subject: [PATCH] Fix and improve doc picker tab navigation

Rel #609.
---
 assets/javascripts/lib/util.coffee                 | 10 +++++++---
 assets/javascripts/views/layout/settings.coffee    |  5 -----
 assets/javascripts/views/sidebar/doc_picker.coffee |  2 +-
 assets/stylesheets/components/_settings.scss       |  4 ++--
 views/app.erb                                      | 11 +++++------
 5 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/assets/javascripts/lib/util.coffee b/assets/javascripts/lib/util.coffee
index e1dd3cf3..ca1fbc7a 100644
--- a/assets/javascripts/lib/util.coffee
+++ b/assets/javascripts/lib/util.coffee
@@ -167,7 +167,8 @@ $.scrollTo = (el, parent, position = 'center', options = {}) ->
   return unless parent
 
   parentHeight = parent.clientHeight
-  return unless parent.scrollHeight > parentHeight
+  parentScrollHeight = parent.scrollHeight
+  return unless parentScrollHeight > parentHeight
 
   top = $.offset(el, parent).top
   offsetTop = parent.firstElementChild.offsetTop
@@ -181,14 +182,17 @@ $.scrollTo = (el, parent, position = 'center', options = {}) ->
       scrollTop = parent.scrollTop
       height = el.offsetHeight
 
+      lastElementOffset = parent.lastElementChild.offsetTop + parent.lastElementChild.offsetHeight
+      offsetBottom = if lastElementOffset > 0 then parentScrollHeight - lastElementOffset else 0
+
       # If the target element is above the visible portion of its scrollable
       # ancestor, move it near the top with a gap = options.topGap * target's height.
       if top - offsetTop <= scrollTop + height * (options.topGap or 1)
         parent.scrollTop = top - offsetTop - height * (options.topGap or 1)
       # If the target element is below the visible portion of its scrollable
       # ancestor, move it near the bottom with a gap = options.bottomGap * target's height.
-      else if top >= scrollTop + parentHeight - height * ((options.bottomGap or 1) + 1)
-        parent.scrollTop = top - parentHeight + height * ((options.bottomGap or 1) + 1)
+      else if top + offsetBottom >= scrollTop + parentHeight - height * ((options.bottomGap or 1) + 1)
+        parent.scrollTop = top + offsetBottom - parentHeight + height * ((options.bottomGap or 1) + 1)
   return
 
 $.scrollToWithImageLock = (el, parent, args...) ->
diff --git a/assets/javascripts/views/layout/settings.coffee b/assets/javascripts/views/layout/settings.coffee
index d669f002..cdc790c6 100644
--- a/assets/javascripts/views/layout/settings.coffee
+++ b/assets/javascripts/views/layout/settings.coffee
@@ -11,7 +11,6 @@ class app.views.Settings extends app.View
   @events:
     submit: 'onSubmit'
     click: 'onClick'
-    focus: 'onFocus'
 
   @shortcuts:
     enter: 'onEnter'
@@ -69,10 +68,6 @@ class app.views.Settings extends app.View
       app.router.show '/'
     return
 
-  onFocus: (event) =>
-    $.scrollTo event.target, @el, 'continuous', bottomGap: 2
-    return
-
   onAppCacheProgress: (event) =>
     if event.lengthComputable
       percentage = Math.round event.loaded * 100 / event.total
diff --git a/assets/javascripts/views/sidebar/doc_picker.coffee b/assets/javascripts/views/sidebar/doc_picker.coffee
index b0869aec..e018794f 100644
--- a/assets/javascripts/views/sidebar/doc_picker.coffee
+++ b/assets/javascripts/views/sidebar/doc_picker.coffee
@@ -67,7 +67,7 @@ class app.views.DocPicker extends app.View
   onDOMFocus: (event) =>
     target = event.target
     if target.tagName is 'INPUT'
-      $.scrollTo target.parentNode, null, 'continuous', bottomGap: 2
+      $.scrollTo target.parentNode, null, 'continuous'
     else if target.classList.contains(app.views.ListFold.targetClass)
       target.blur()
       unless @mouseDown and @mouseDown > Date.now() - 100
diff --git a/assets/stylesheets/components/_settings.scss b/assets/stylesheets/components/_settings.scss
index 075db6e3..41d019a6 100644
--- a/assets/stylesheets/components/_settings.scss
+++ b/assets/stylesheets/components/_settings.scss
@@ -12,13 +12,13 @@
   &._in { display: block; }
 
   ._sidebar {
+    display: block !important;
+
     &:after { // padding bottom
       content: '';
       display: block;
       height: $headerHeight;
     }
-
-    ._sidebar-hidden & { display: block; }
   }
 
   ._header { justify-content: space-between; }
diff --git a/views/app.erb b/views/app.erb
index 534d7023..18d92f12 100644
--- a/views/app.erb
+++ b/views/app.erb
@@ -47,12 +47,11 @@
         <button type="button" class="_settings-tab active" data-tab="doc-picker" hidden>Docs</button><button type="button" class="_settings-tab" data-tab="settings" hidden>Settings</button>
       </nav>
     </div>
-    <div class="_sidebar">
-      <div class="_sidebar-footer">
-        <button type="submit" class="_settings-btn">
-          <svg viewBox="0 0 24 24"><path d="M9 16.17L4.83 12l-1.42 1.41L9 19 21 7l-1.41-1.41z"></path></svg> Apply
-        </button>
-      </div>
+    <div class="_sidebar" tabindex="-1"></div>
+    <div class="_sidebar-footer">
+      <button type="submit" class="_settings-btn">
+        <svg viewBox="0 0 24 24"><path d="M9 16.17L4.83 12l-1.42 1.41L9 19 21 7l-1.41-1.41z"></path></svg> Apply
+      </button>
     </div>
   </form>
 </div>