Stop using JX.Scrollbar for main page content

Summary:
Ref T8151. This is option (5). It needs a few adjustments but feels pretty good. Major issues are:

  - Without a mouse, the scrollbars overlap by default, so we //must// move the column off the right margin.
  - Scrolling sometimes "bleeds" between the chat vs the main frame in a way that's not as discrete as the old framed content, but feels generally reasonable to me.

If we pursue this, I'd plan to make these additional changes:

  - Move the panel away from the right margin only if the page scrollbars are zero-width (i.e., in OSX trackpad mode).
  - Fix the notch in the upper right corner when the chat is moved away from the right margin.
  - Probably remove the body "overflow-y: scroll" on Conpherence and Workboards.
  - Update the resizing code to deal with 300px vs 315px widths.
  - We can probably clean up some JX.Scrollbar "main panel" code.

Here's the "bad" case, where I've visually separated the column to provide room for a scrollbar. This isn't ideal, but looks and feels OK to me:

{F398375}

Test Plan:
  - Tried Firefox, Chrome, Safari, with and without a mouse.
  - Tried normal Conpherence.

Reviewers: btrahan, chad

Reviewed By: btrahan

Subscribers: avivey, epriestley

Maniphest Tasks: T8151

Differential Revision: https://secure.phabricator.com/D12789

resources/celerity/map.php

@@ -7,8 +7,8 @@
  */
 return array(
   'names' => array(
-    'core.pkg.css' => 'a821cfc9',
+    'core.pkg.css' => 'ed3d6355',
-    'core.pkg.js' => '919c56b5',
+    'core.pkg.js' => '31eaf90a',
     'darkconsole.pkg.js' => 'e7393ebb',
     'differential.pkg.css' => 'bb338e4b',
     'differential.pkg.js' => '895b8d62',
@@ -37,7 +37,7 @@ return array(
     'rsrc/css/application/base/main-menu-view.css' => '663e3810',
     'rsrc/css/application/base/notification-menu.css' => '3c9d8aa1',
     'rsrc/css/application/base/phabricator-application-launch-view.css' => '16ca323f',
-    'rsrc/css/application/base/standard-page-view.css' => 'd3e1abe9',
+    'rsrc/css/application/base/standard-page-view.css' => '2acd4611',
     'rsrc/css/application/chatlog/chatlog.css' => '852140ff',
     'rsrc/css/application/conduit/conduit-api.css' => '7bc725c4',
     'rsrc/css/application/config/config-options.css' => '7fedf08b',
@@ -45,11 +45,11 @@ return array(
     'rsrc/css/application/config/config-welcome.css' => '6abd79be',
     'rsrc/css/application/config/setup-issue.css' => '22270af2',
     'rsrc/css/application/config/unhandled-exception.css' => '37d4f9a2',
-    'rsrc/css/application/conpherence/durable-column.css' => '2e68a92f',
+    'rsrc/css/application/conpherence/durable-column.css' => '8c43d6ac',
     'rsrc/css/application/conpherence/menu.css' => 'f389e048',
     'rsrc/css/application/conpherence/message-pane.css' => '0e75feef',
     'rsrc/css/application/conpherence/notification.css' => 'd208f806',
-    'rsrc/css/application/conpherence/transaction.css' => '885433f0',
+    'rsrc/css/application/conpherence/transaction.css' => '42a457f6',
     'rsrc/css/application/conpherence/update.css' => '1099a660',
     'rsrc/css/application/conpherence/widget-pane.css' => '2af42ebe',
     'rsrc/css/application/contentsource/content-source-view.css' => '4b8b05d4',
@@ -108,10 +108,10 @@ return array(
     'rsrc/css/application/slowvote/slowvote.css' => '266df6a1',
     'rsrc/css/application/tokens/tokens.css' => '3d0f239e',
     'rsrc/css/application/uiexample/example.css' => '528b19de',
-    'rsrc/css/core/core.css' => '76e8ee93',
+    'rsrc/css/core/core.css' => 'aaea7a7a',
     'rsrc/css/core/remarkup.css' => '0037bdbf',
     'rsrc/css/core/syntax.css' => '6b7b24d9',
-    'rsrc/css/core/z-index.css' => '3b612549',
+    'rsrc/css/core/z-index.css' => '8414a09b',
     'rsrc/css/diviner/diviner-shared.css' => '38813222',
     'rsrc/css/font/font-awesome.css' => 'e2e712fe',
     'rsrc/css/font/font-source-sans-pro.css' => '8906c07b',
@@ -211,7 +211,7 @@ return array(
     'rsrc/externals/javelin/lib/Resource.js' => '44959b73',
     'rsrc/externals/javelin/lib/Routable.js' => 'b3e7d692',
     'rsrc/externals/javelin/lib/Router.js' => '29274e2b',
-    'rsrc/externals/javelin/lib/Scrollbar.js' => 'eaa5b321',
+    'rsrc/externals/javelin/lib/Scrollbar.js' => '4f812f8a',
     'rsrc/externals/javelin/lib/Sound.js' => '949c0fe5',
     'rsrc/externals/javelin/lib/URI.js' => '6eff08aa',
     'rsrc/externals/javelin/lib/Vector.js' => '2caa8fb8',
@@ -350,7 +350,7 @@ return array(
     'rsrc/js/application/config/behavior-reorder-fields.js' => '14a827de',
     'rsrc/js/application/conpherence/ConpherenceThreadManager.js' => '9e507b59',
     'rsrc/js/application/conpherence/behavior-drag-and-drop-photo.js' => 'cf86d16a',
-    'rsrc/js/application/conpherence/behavior-durable-column.js' => '657c2b50',
+    'rsrc/js/application/conpherence/behavior-durable-column.js' => 'ac11eb8a',
     'rsrc/js/application/conpherence/behavior-menu.js' => '804b0773',
     'rsrc/js/application/conpherence/behavior-pontificate.js' => '21ba5861',
     'rsrc/js/application/conpherence/behavior-quicksand-blacklist.js' => '7927a7d3',
@@ -512,12 +512,12 @@ return array(
     'conduit-api-css' => '7bc725c4',
     'config-options-css' => '7fedf08b',
     'config-welcome-css' => '6abd79be',
-    'conpherence-durable-column-view' => '2e68a92f',
+    'conpherence-durable-column-view' => '8c43d6ac',
     'conpherence-menu-css' => 'f389e048',
     'conpherence-message-pane-css' => '0e75feef',
     'conpherence-notification-css' => 'd208f806',
     'conpherence-thread-manager' => '9e507b59',
-    'conpherence-transaction-css' => '885433f0',
+    'conpherence-transaction-css' => '42a457f6',
     'conpherence-update-css' => '1099a660',
     'conpherence-widget-pane-css' => '2af42ebe',
     'differential-changeset-view-css' => 'e19cfd6e',
@@ -584,7 +584,7 @@ return array(
     'javelin-behavior-diffusion-locate-file' => '6d3e1947',
     'javelin-behavior-diffusion-pull-lastmodified' => '2b228192',
     'javelin-behavior-doorkeeper-tag' => 'e5822781',
-    'javelin-behavior-durable-column' => '657c2b50',
+    'javelin-behavior-durable-column' => 'ac11eb8a',
     'javelin-behavior-error-log' => '6882e80a',
     'javelin-behavior-event-all-day' => 'ca5fa62a',
     'javelin-behavior-fancy-datepicker' => '5c0f680f',
@@ -681,7 +681,7 @@ return array(
     'javelin-resource' => '44959b73',
     'javelin-routable' => 'b3e7d692',
     'javelin-router' => '29274e2b',
-    'javelin-scrollbar' => 'eaa5b321',
+    'javelin-scrollbar' => '4f812f8a',
     'javelin-sound' => '949c0fe5',
     'javelin-stratcom' => '6c53634d',
     'javelin-tokenizer' => 'ab5f468d',
@@ -718,7 +718,7 @@ return array(
     'phabricator-busy' => '59a7976a',
     'phabricator-chatlog-css' => '852140ff',
     'phabricator-content-source-view-css' => '4b8b05d4',
-    'phabricator-core-css' => '76e8ee93',
+    'phabricator-core-css' => 'aaea7a7a',
     'phabricator-countdown-css' => '86b7b0a0',
     'phabricator-dashboard-css' => '17937d22',
     'phabricator-drag-and-drop-file-upload' => '07de8873',
@@ -747,7 +747,7 @@ return array(
     'phabricator-side-menu-view-css' => 'c1db9e9c',
     'phabricator-slowvote-css' => '266df6a1',
     'phabricator-source-code-view-css' => '2ceee894',
-    'phabricator-standard-page-view' => 'd3e1abe9',
+    'phabricator-standard-page-view' => '2acd4611',
     'phabricator-textareautils' => '5c93c52c',
     'phabricator-title' => 'df5e11d2',
     'phabricator-tooltip' => '1d298e3a',
@@ -762,7 +762,7 @@ return array(
     'phabricator-uiexample-reactor-select' => 'a155550f',
     'phabricator-uiexample-reactor-sendclass' => '1def2711',
     'phabricator-uiexample-reactor-sendproperties' => 'b1f0ccee',
-    'phabricator-zindex-css' => '3b612549',
+    'phabricator-zindex-css' => '8414a09b',
     'phame-css' => '88bd4705',
     'pholio-css' => '95174bdd',
     'pholio-edit-css' => '3ad9d1ee',
@@ -1134,6 +1134,12 @@ return array(
       'javelin-stratcom',
       'javelin-request',
     ),
+    '4f812f8a' => array(
+      'javelin-install',
+      'javelin-dom',
+      'javelin-stratcom',
+      'javelin-vector',
+    ),
     '4fdb476d' => array(
       'javelin-behavior',
       'javelin-stratcom',
@@ -1279,16 +1285,6 @@ return array(
       'javelin-workflow',
       'javelin-dom',
     ),
-    '657c2b50' => array(
-      'javelin-behavior',
-      'javelin-dom',
-      'javelin-stratcom',
-      'javelin-behavior-device',
-      'javelin-scrollbar',
-      'javelin-quicksand',
-      'phabricator-keyboard-shortcut',
-      'conpherence-thread-manager',
-    ),
     '6882e80a' => array(
       'javelin-dom',
     ),
@@ -1685,6 +1681,16 @@ return array(
       'javelin-stratcom',
       'javelin-install',
     ),
+    'ac11eb8a' => array(
+      'javelin-behavior',
+      'javelin-dom',
+      'javelin-stratcom',
+      'javelin-behavior-device',
+      'javelin-scrollbar',
+      'javelin-quicksand',
+      'phabricator-keyboard-shortcut',
+      'conpherence-thread-manager',
+    ),
     'b1a59974' => array(
       'javelin-behavior',
       'javelin-aphlict',
@@ -1928,12 +1934,6 @@ return array(
       'phabricator-phtize',
       'javelin-dom',
     ),
-    'eaa5b321' => array(
-      'javelin-install',
-      'javelin-dom',
-      'javelin-stratcom',
-      'javelin-vector',
-    ),
     'efe49472' => array(
       'javelin-install',
       'javelin-util',

src/view/page/PhabricatorStandardPageView.php

@@ -377,15 +377,6 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView {
       }
     }
 
-    if (!$this->isQuicksandBlacklistURI()) {
-      Javelin::initBehavior(
-        'scrollbar',
-        array(
-          'nodeID' => 'phabricator-standard-page',
-          'isMainContent' => true,
-        ));
-    }
-
     $main_page = phutil_tag(
       'div',
       array(

webroot/rsrc/css/application/base/standard-page-view.css

@@ -126,27 +126,6 @@ a.handle-disabled {
   margin: 2px 2px -2px 0;
 }
 
-.main-page-frame {
-  position: absolute;
-  top: 0;
-  bottom: 0;
-  right: 0;
-  left: 0;
-  overflow: hidden;
-}
-
-.phabricator-standard-page {
-  /* If we don't activate JX.Scrollbar because the default scrollbars are
-     satisfactory, make sure the page still has sensible behavior. These
-     settings will be overwritten by .jx-scrollbar-frame if JX.Scrollbar
-     activates. */
-  position: relative;
-  height: 100%;
-  overflow-y: scroll;
-
-  -webkit-overflow-scrolling: touch;
-}
-
 .jx-scrollbar-frame {
   position: relative;
   overflow: hidden;

webroot/rsrc/css/application/conpherence/durable-column.css

@@ -2,10 +2,22 @@
  * @provides conpherence-durable-column-view
  */
 
-.with-durable-column .phabricator-standard-page {
+.with-durable-column .phabricator-standard-page-body {
   margin-right: 300px;
 }
 
+.with-durable-margin .phabricator-standard-page-body {
+  margin-right: 312px;
+}
+
+.with-durable-column .phabricator-main-menu {
+  padding-right: 304px;
+}
+
+.with-durable-margin .phabricator-main-menu {
+  padding-right: 316px;
+}
+
 .with-durable-column
 .phabricator-global-upload-instructions {
   font-size: 28px;
@@ -20,8 +32,12 @@
   right: 300px;
 }
 
+.with-durable-margin .global-upload-mask {
+  right: 312px;
+}
+
 .conpherence-durable-column {
-  position: absolute;
+  position: fixed;
   top: 0;
   bottom: 0;
   right: 0;
@@ -29,6 +45,11 @@
   background: #fff;
 }
 
+.with-durable-margin .conpherence-durable-column {
+  right: 12px;
+  box-shadow: 1px 0px 2px rgba(0, 0, 0, 0.10);
+}
+
 .conpherence-durable-column .loading-mask {
   position: absolute;
   top: 90px;
@@ -126,6 +147,10 @@
   border-left: 1px solid {$lightblueborder};
 }
 
+.with-durable-margin .conpherence-durable-column-body {
+  border-right: 1px solid {$lightblueborder};
+}
+
 .conpherence-durable-column-main {
   position: absolute;
   top: 46px;

webroot/rsrc/css/core/core.css

@@ -54,6 +54,10 @@ body {
      breaks lots of things and prevents you from using landscape to see more
      columns in source code views. */
   -webkit-text-size-adjust: none;
+
+  /* Prevent content from resizing abruptly when shifting between scrollable
+     and unscrollable pages. */
+  overflow-y: scroll;
 }
 
 textarea {

webroot/rsrc/css/core/z-index.css

@@ -85,7 +85,6 @@
   z-index: 5;
 }
 
-.conpherence-durable-column-header,
 .phabricator-main-menu {
   z-index: 6;
 }
@@ -94,10 +93,14 @@
   z-index: 6;
 }
 
-.jx-scrollbar-bar {
+.conpherence-durable-column {
   z-index: 7;
 }
 
+.jx-scrollbar-bar {
+  z-index: 8;
+}
+
 .differential-haunt-mode-1 .differential-add-comment-panel,
 .differential-haunt-mode-2 .differential-add-comment-panel {
   z-index: 8;

webroot/rsrc/externals/javelin/lib/Scrollbar.js

@@ -100,6 +100,7 @@ JX.install('Scrollbar', {
   statics: {
     _controlWidth: null,
 
+
     /**
      * Compute the width of the browser's scrollbar control, in pixels.
      */
@@ -118,8 +119,35 @@ JX.install('Scrollbar', {
       }
 
       return self._controlWidth;
+    },
+
+
+    /**
+     * Get the margin width required to avoid double scrollbars.
+     *
+     * For most browsers which render a real scrollbar control, this is 0.
+     * Adjacent elements may touch the edge of the content directly without
+     * overlapping.
+     *
+     * On OSX with a trackpad, scrollbars are only drawn when content is
+     * scrolled. Content panes with internal scrollbars may overlap adjacent
+     * scrollbars if they are not laid out with a margin.
+     *
+     * @return int Control margin width in pixels.
+     */
+    getScrollbarControlMargin: function() {
+      var self = JX.Scrollbar;
+
+      // If this browser and OS don't render a real scrollbar control, we
+      // need to leave a margin. Generally, this is OSX with no mouse attached.
+      if (self._getScrollbarControlWidth() === 0) {
+        return 12;
       }
 
+      return 0;
+    }
+
+
   },
 
   members: {

webroot/rsrc/js/application/conpherence/behavior-durable-column.js

@@ -29,9 +29,11 @@ JX.behavior('durable-column', function(config, statics) {
   var loadThreadID = null;
   var scrollbar = null;
 
-  var columnWidth = 300;
+  var margin = JX.Scrollbar.getScrollbarControlMargin();
+
+  var columnWidth = (300 + margin);
   // This is the smallest window size where we'll enable the column.
-  var minimumViewportWidth = 768;
+  var minimumViewportWidth = (768 - margin);
 
   var quick = JX.$('phabricator-standard-page-body');
 
@@ -72,6 +74,8 @@ JX.behavior('durable-column', function(config, statics) {
 
   function _drawColumn(visible) {
     JX.DOM.alterClass(document.body, 'with-durable-column', visible);
+    JX.DOM.alterClass(document.body, 'with-durable-margin', !!margin);
+
     var column = _getColumnNode();
     if (visible) {
       JX.DOM.show(column);