From aba209e9990fe1c975c0786be1bfbc4a9a9d173c Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 20 May 2017 04:10:32 -0700 Subject: [PATCH] Hide the Differential scroll objective list on trackpad systems Summary: Ref T12733. In the longer run I'd like to just push this out from the edge, but that currently gets us into trouble since we start bumping into content. On my system, the trackpad scrollbar also expands in size when moused over, so the minimum number of pixels we need to push it out is approximatley 15px. This hits body content and the persistent chat. For now, just disable this element on trackpad systems. Test Plan: Disconnected all USB peripherals, quit and relaunched Safari, saw no objective list. Reconnected mouse, relaunched Safari, saw objective list. Reviewers: chad Reviewed By: chad Maniphest Tasks: T12733 Differential Revision: https://secure.phabricator.com/D17974 --- resources/celerity/map.php | 47 ++++++++++--------- .../differential/changeset-view.css | 5 ++ .../rsrc/externals/javelin/lib/Scrollbar.js | 8 ++-- .../application/diff/ScrollObjectiveList.js | 6 +++ 4 files changed, 39 insertions(+), 27 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 5b6c54b2f1..bf2fa84a72 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -10,10 +10,10 @@ return array( 'conpherence.pkg.css' => 'ff161f2d', 'conpherence.pkg.js' => 'b5b51108', 'core.pkg.css' => '5ffe8b79', - 'core.pkg.js' => 'e822b496', + 'core.pkg.js' => '6b2da600', 'darkconsole.pkg.js' => '1f9a31bc', - 'differential.pkg.css' => '4d7dd14e', - 'differential.pkg.js' => '0dfe037d', + 'differential.pkg.css' => 'bf87589e', + 'differential.pkg.js' => 'ee4f14c5', 'diffusion.pkg.css' => 'b93d9b8c', 'diffusion.pkg.js' => '84c8f8fd', 'favicon.ico' => '30672e08', @@ -64,7 +64,7 @@ return array( 'rsrc/css/application/dashboard/dashboard.css' => 'fe5b1869', 'rsrc/css/application/diff/inline-comment-summary.css' => '51efda3a', 'rsrc/css/application/differential/add-comment.css' => 'c47f8c40', - 'rsrc/css/application/differential/changeset-view.css' => '54774a28', + 'rsrc/css/application/differential/changeset-view.css' => 'fa476ec0', 'rsrc/css/application/differential/core.css' => '5b7b8ff4', 'rsrc/css/application/differential/phui-inline-comment.css' => 'ffd1a542', 'rsrc/css/application/differential/revision-comment.css' => '14b8565a', @@ -247,7 +247,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' => '087e919c', + 'rsrc/externals/javelin/lib/Scrollbar.js' => '9065f639', 'rsrc/externals/javelin/lib/Sound.js' => '949c0fe5', 'rsrc/externals/javelin/lib/URI.js' => 'c989ade3', 'rsrc/externals/javelin/lib/Vector.js' => '2caa8fb8', @@ -394,7 +394,7 @@ return array( 'rsrc/js/application/diff/DiffChangesetList.js' => 'a716ca27', 'rsrc/js/application/diff/DiffInline.js' => '77e14b60', 'rsrc/js/application/diff/ScrollObjective.js' => '0eee7a00', - 'rsrc/js/application/diff/ScrollObjectiveList.js' => '1ca4d9db', + 'rsrc/js/application/diff/ScrollObjectiveList.js' => '085dd101', 'rsrc/js/application/diff/behavior-preview-link.js' => '051c7832', 'rsrc/js/application/differential/behavior-comment-preview.js' => '51c5ad07', 'rsrc/js/application/differential/behavior-diff-radios.js' => 'e1ff79b1', @@ -567,7 +567,7 @@ return array( 'conpherence-thread-manager' => '4d863052', 'conpherence-transaction-css' => '85129c68', 'd3' => 'a11a5ff2', - 'differential-changeset-view-css' => '54774a28', + 'differential-changeset-view-css' => 'fa476ec0', 'differential-core-view-css' => '5b7b8ff4', 'differential-revision-add-comment-css' => 'c47f8c40', 'differential-revision-comment-css' => '14b8565a', @@ -732,7 +732,7 @@ return array( 'javelin-resource' => '44959b73', 'javelin-routable' => 'b3e7d692', 'javelin-router' => '29274e2b', - 'javelin-scrollbar' => '087e919c', + 'javelin-scrollbar' => '9065f639', 'javelin-sound' => '949c0fe5', 'javelin-stratcom' => '6ad39b6f', 'javelin-tokenizer' => '8d3bc1b2', @@ -800,7 +800,7 @@ return array( 'phabricator-prefab' => 'c5af80a2', 'phabricator-remarkup-css' => 'd1a5e11e', 'phabricator-scroll-objective' => '0eee7a00', - 'phabricator-scroll-objective-list' => '1ca4d9db', + 'phabricator-scroll-objective-list' => '085dd101', 'phabricator-search-results-css' => 'f87d23ad', 'phabricator-shaped-request' => '7cbe244b', 'phabricator-slowvote-css' => 'a94b7230', @@ -956,11 +956,14 @@ return array( 'javelin-stratcom', 'javelin-util', ), - '087e919c' => array( - 'javelin-install', + '085dd101' => array( 'javelin-dom', + 'javelin-util', 'javelin-stratcom', - 'javelin-vector', + 'javelin-install', + 'javelin-workflow', + 'javelin-scrollbar', + 'phabricator-scroll-objective', ), '08f4ccc3' => array( 'phui-oi-list-view-css', @@ -1034,14 +1037,6 @@ return array( 'javelin-request', 'javelin-uri', ), - '1ca4d9db' => array( - 'javelin-dom', - 'javelin-util', - 'javelin-stratcom', - 'javelin-install', - 'javelin-workflow', - 'phabricator-scroll-objective', - ), '1de8bf63' => array( 'javelin-behavior', 'javelin-dom', @@ -1324,9 +1319,6 @@ return array( '5294060f' => array( 'phui-theme-css', ), - '54774a28' => array( - 'phui-inline-comment-view-css', - ), '54b612ba' => array( 'javelin-color', 'javelin-install', @@ -1611,6 +1603,12 @@ return array( 'javelin-dom', 'javelin-request', ), + '9065f639' => array( + 'javelin-install', + 'javelin-dom', + 'javelin-stratcom', + 'javelin-vector', + ), '92b9ec77' => array( 'javelin-behavior', 'javelin-stratcom', @@ -2208,6 +2206,9 @@ return array( 'javelin-install', 'javelin-dom', ), + 'fa476ec0' => array( + 'phui-inline-comment-view-css', + ), 'fbe497e7' => array( 'javelin-behavior', 'javelin-util', diff --git a/webroot/rsrc/css/application/differential/changeset-view.css b/webroot/rsrc/css/application/differential/changeset-view.css index eee0e167f3..6a2552fbd1 100644 --- a/webroot/rsrc/css/application/differential/changeset-view.css +++ b/webroot/rsrc/css/application/differential/changeset-view.css @@ -423,6 +423,11 @@ tr.differential-inline-loading { overflow: hidden; } +.scroll-objective-list.has-aesthetic-scrollbar { + /* For now, hide this element on systems with aesthetic scrollbars. */ + display: none; +} + .scroll-objective { display: block; position: absolute; diff --git a/webroot/rsrc/externals/javelin/lib/Scrollbar.js b/webroot/rsrc/externals/javelin/lib/Scrollbar.js index 7596939581..cd69eb4def 100644 --- a/webroot/rsrc/externals/javelin/lib/Scrollbar.js +++ b/webroot/rsrc/externals/javelin/lib/Scrollbar.js @@ -34,7 +34,7 @@ JX.install('Scrollbar', { // width. If it doesn't, we're already in an environment with an aesthetic // scrollbar (like Safari on OSX with no mouse connected, or an iPhone) // and we don't need to do anything. - if (JX.Scrollbar._getScrollbarControlWidth() === 0) { + if (JX.Scrollbar.getScrollbarControlWidth() === 0) { return; } @@ -104,7 +104,7 @@ JX.install('Scrollbar', { /** * Compute the width of the browser's scrollbar control, in pixels. */ - _getScrollbarControlWidth: function() { + getScrollbarControlWidth: function() { var self = JX.Scrollbar; if (self._controlWidth === null) { @@ -140,7 +140,7 @@ JX.install('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) { + if (self.getScrollbarControlWidth() === 0) { return 12; } @@ -357,7 +357,7 @@ JX.install('Scrollbar', { */ _resizeViewport: function() { var fdim = JX.Vector.getDim(this._frame); - fdim.x += JX.Scrollbar._getScrollbarControlWidth(); + fdim.x += JX.Scrollbar.getScrollbarControlWidth(); fdim.setDim(this._viewport); }, diff --git a/webroot/rsrc/js/application/diff/ScrollObjectiveList.js b/webroot/rsrc/js/application/diff/ScrollObjectiveList.js index f5beb751e6..7aae1fb1b4 100644 --- a/webroot/rsrc/js/application/diff/ScrollObjectiveList.js +++ b/webroot/rsrc/js/application/diff/ScrollObjectiveList.js @@ -5,6 +5,7 @@ * javelin-stratcom * javelin-install * javelin-workflow + * javelin-scrollbar * phabricator-scroll-objective * @javelin */ @@ -81,6 +82,11 @@ JX.install('ScrollObjectiveList', { document.body.appendChild(node); + // If we're on OSX without a mouse or some other system with zero-width + // trackpad-style scrollbars, adjust the display appropriately. + var aesthetic = (JX.Scrollbar.getScrollbarControlWidth() === 0); + JX.DOM.alterClass(node, 'has-aesthetic-scrollbar', aesthetic); + var d = JX.Vector.getDocument(); var list_dimensions = JX.Vector.getDim(node);