From fb9f3cc0b4cae3decd115da2a773978d43b26e4c Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 17 May 2017 14:49:26 -0700 Subject: [PATCH] Restore the "buoyant" header in Differential Summary: Fixes T1591. This was removed long ago because it was a mess to implement and caused a bunch of weird issues, and also my tolerance for dealing with weird JS issues was much, much lower. I have now survived the fires of JX.Scrollbar and would love to address 200 small nitpicks about obscure browser behaviors on Linux, so open the floodgates again. A secondary goal here is to create room to add a global view state menu on the right, with 300 options like "hide all inlines", "hide done inlines", "hide collapsed inlines", "hide ghosts", "show ghosts", "enable filetree", "disable filetree", etc, etc. Not sure how much of this I'll actually do. I have one more experiment I want to try first. Test Plan: {F4963294} Reviewers: chad Reviewed By: chad Maniphest Tasks: T1591 Differential Revision: https://secure.phabricator.com/D17945 --- resources/celerity/map.php | 82 +++++++++---------- .../view/DifferentialChangesetDetailView.php | 1 + .../differential/changeset-view.css | 18 ++++ webroot/rsrc/css/core/z-index.css | 4 + .../rsrc/js/application/diff/DiffChangeset.js | 14 ++++ .../js/application/diff/DiffChangesetList.js | 82 +++++++++++++++++++ .../rsrc/js/core/behavior-phabricator-nav.js | 11 +++ 7 files changed, 171 insertions(+), 41 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 2f9026785d..d175534828 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -9,11 +9,11 @@ return array( 'names' => array( 'conpherence.pkg.css' => 'ff161f2d', 'conpherence.pkg.js' => 'b5b51108', - 'core.pkg.css' => 'a5a2d647', - 'core.pkg.js' => '0f87a6eb', + 'core.pkg.css' => '4937a7d7', + 'core.pkg.js' => 'a0c8fb20', 'darkconsole.pkg.js' => '1f9a31bc', - 'differential.pkg.css' => '697405d4', - 'differential.pkg.js' => '07c56ffc', + 'differential.pkg.css' => '52b014e7', + 'differential.pkg.js' => '1efe85bf', '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' => '15be1064', + 'rsrc/css/application/differential/changeset-view.css' => 'e7bd2a79', 'rsrc/css/application/differential/core.css' => '5b7b8ff4', 'rsrc/css/application/differential/phui-inline-comment.css' => 'ffd1a542', 'rsrc/css/application/differential/revision-comment.css' => '14b8565a', @@ -116,7 +116,7 @@ return array( 'rsrc/css/core/core.css' => '9f4cb463', 'rsrc/css/core/remarkup.css' => 'd1a5e11e', 'rsrc/css/core/syntax.css' => 'cae95e89', - 'rsrc/css/core/z-index.css' => '0233d039', + 'rsrc/css/core/z-index.css' => '9d8f7c4b', 'rsrc/css/diviner/diviner-shared.css' => '896f1d43', 'rsrc/css/font/font-awesome.css' => 'e838e088', 'rsrc/css/font/font-lato.css' => 'c7ccd872', @@ -390,8 +390,8 @@ return array( 'rsrc/js/application/dashboard/behavior-dashboard-move-panels.js' => '408bf173', 'rsrc/js/application/dashboard/behavior-dashboard-query-panel-select.js' => '453c5375', 'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => 'd4eecc63', - 'rsrc/js/application/diff/DiffChangeset.js' => '731125f3', - 'rsrc/js/application/diff/DiffChangesetList.js' => '59d1ceb1', + 'rsrc/js/application/diff/DiffChangeset.js' => '3268dd83', + 'rsrc/js/application/diff/DiffChangesetList.js' => '0a4f7809', 'rsrc/js/application/diff/DiffInline.js' => '3337c065', 'rsrc/js/application/diff/behavior-preview-link.js' => '051c7832', 'rsrc/js/application/differential/behavior-comment-preview.js' => 'b064af76', @@ -503,7 +503,7 @@ return array( 'rsrc/js/core/behavior-more.js' => 'a80d0378', 'rsrc/js/core/behavior-object-selector.js' => 'e0ec7f2f', 'rsrc/js/core/behavior-oncopy.js' => '2926fff2', - 'rsrc/js/core/behavior-phabricator-nav.js' => '08163386', + 'rsrc/js/core/behavior-phabricator-nav.js' => '947753e0', 'rsrc/js/core/behavior-phabricator-remarkup-assist.js' => 'acd29eee', 'rsrc/js/core/behavior-read-only-warning.js' => 'ba158207', 'rsrc/js/core/behavior-refresh-csrf.js' => 'ab2f381b', @@ -565,7 +565,7 @@ return array( 'conpherence-thread-manager' => '4d863052', 'conpherence-transaction-css' => '85129c68', 'd3' => 'a11a5ff2', - 'differential-changeset-view-css' => '15be1064', + 'differential-changeset-view-css' => 'e7bd2a79', 'differential-core-view-css' => '5b7b8ff4', 'differential-revision-add-comment-css' => 'c47f8c40', 'differential-revision-comment-css' => '14b8565a', @@ -658,7 +658,7 @@ return array( 'javelin-behavior-phabricator-keyboard-pager' => 'a8da01f0', 'javelin-behavior-phabricator-keyboard-shortcuts' => '01fca1f0', 'javelin-behavior-phabricator-line-linker' => '1499a8cb', - 'javelin-behavior-phabricator-nav' => '08163386', + 'javelin-behavior-phabricator-nav' => '947753e0', 'javelin-behavior-phabricator-notification-example' => '8ce821c5', 'javelin-behavior-phabricator-object-selector' => 'e0ec7f2f', 'javelin-behavior-phabricator-oncopy' => '2926fff2', @@ -775,8 +775,8 @@ return array( 'phabricator-darklog' => 'c8e1ffe3', 'phabricator-darkmessage' => 'c48cccdd', 'phabricator-dashboard-css' => 'fe5b1869', - 'phabricator-diff-changeset' => '731125f3', - 'phabricator-diff-changeset-list' => '59d1ceb1', + 'phabricator-diff-changeset' => '3268dd83', + 'phabricator-diff-changeset-list' => '0a4f7809', 'phabricator-diff-inline' => '3337c065', 'phabricator-drag-and-drop-file-upload' => '58dea2fa', 'phabricator-draggable-list' => 'bea6e7f4', @@ -816,7 +816,7 @@ return array( 'phabricator-uiexample-reactor-select' => 'a155550f', 'phabricator-uiexample-reactor-sendclass' => '1def2711', 'phabricator-uiexample-reactor-sendproperties' => 'b1f0ccee', - 'phabricator-zindex-css' => '0233d039', + 'phabricator-zindex-css' => '9d8f7c4b', 'phame-css' => 'b3a0b3a3', 'pholio-css' => 'ca89d380', 'pholio-edit-css' => '07676f51', @@ -946,16 +946,6 @@ return array( 'javelin-stratcom', 'javelin-workflow', ), - '08163386' => array( - 'javelin-behavior', - 'javelin-behavior-device', - 'javelin-stratcom', - 'javelin-dom', - 'javelin-magical-init', - 'javelin-vector', - 'javelin-request', - 'javelin-util', - ), '0825c27a' => array( 'javelin-behavior', 'javelin-dom', @@ -983,6 +973,9 @@ return array( 'javelin-dom', 'javelin-router', ), + '0a4f7809' => array( + 'javelin-install', + ), '0f764c35' => array( 'javelin-install', 'javelin-util', @@ -999,9 +992,6 @@ return array( 'javelin-dom', 'javelin-history', ), - '15be1064' => array( - 'phui-inline-comment-view-css', - ), '17bb8539' => array( 'javelin-behavior', 'javelin-stratcom', @@ -1122,6 +1112,17 @@ return array( 'javelin-dom', 'javelin-vector', ), + '3268dd83' => array( + 'javelin-dom', + 'javelin-util', + 'javelin-stratcom', + 'javelin-install', + 'javelin-workflow', + 'javelin-router', + 'javelin-behavior-device', + 'javelin-vector', + 'phabricator-diff-inline', + ), '327a00d1' => array( 'javelin-behavior', 'javelin-stratcom', @@ -1349,9 +1350,6 @@ return array( 'javelin-vector', 'javelin-dom', ), - '59d1ceb1' => array( - 'javelin-install', - ), '5c54cbf3' => array( 'javelin-behavior', 'javelin-stratcom', @@ -1450,17 +1448,6 @@ return array( 'javelin-workflow', 'phabricator-draggable-list', ), - '731125f3' => array( - 'javelin-dom', - 'javelin-util', - 'javelin-stratcom', - 'javelin-install', - 'javelin-workflow', - 'javelin-router', - 'javelin-behavior-device', - 'javelin-vector', - 'phabricator-diff-inline', - ), '7319e029' => array( 'javelin-behavior', 'javelin-dom', @@ -1619,6 +1606,16 @@ return array( 'javelin-workflow', 'javelin-dom', ), + '947753e0' => array( + 'javelin-behavior', + 'javelin-behavior-device', + 'javelin-stratcom', + 'javelin-dom', + 'javelin-magical-init', + 'javelin-vector', + 'javelin-request', + 'javelin-util', + ), '949c0fe5' => array( 'javelin-install', ), @@ -2127,6 +2124,9 @@ return array( 'javelin-workflow', 'javelin-magical-init', ), + 'e7bd2a79' => array( + 'phui-inline-comment-view-css', + ), 'e9581f08' => array( 'javelin-behavior', 'javelin-stratcom', diff --git a/src/applications/differential/view/DifferentialChangesetDetailView.php b/src/applications/differential/view/DifferentialChangesetDetailView.php index 42be1c6444..86fba06c40 100644 --- a/src/applications/differential/view/DifferentialChangesetDetailView.php +++ b/src/applications/differential/view/DifferentialChangesetDetailView.php @@ -183,6 +183,7 @@ final class DifferentialChangesetDetailView extends AphrontView { 'autoload' => $this->getAutoload(), 'loaded' => $this->getLoaded(), 'undoTemplates' => hsprintf('%s', $renderer->renderUndoTemplates()), + 'path' => $display_filename, ), 'class' => $class, 'id' => $id, diff --git a/webroot/rsrc/css/application/differential/changeset-view.css b/webroot/rsrc/css/application/differential/changeset-view.css index 0cec38ffdc..1931ce354b 100644 --- a/webroot/rsrc/css/application/differential/changeset-view.css +++ b/webroot/rsrc/css/application/differential/changeset-view.css @@ -386,3 +386,21 @@ tr.differential-inline-loading { .differential-review-stage { position: relative; } + +.diff-banner { + position: fixed; + top: 0; + left: 0; + right: 0; + background: rgba(255, 255, 255, 0.95); + box-shadow: 0px 1px 3px rgba(0, 0, 0, 0.1); + border-bottom: 1px solid {$lightgreyborder}; + padding: 12px 18px; + vertical-align: middle; + font-weight: bold; + font-size: {$biggerfontsize}; +} + +.diff-banner .phui-icon-view { + margin-right: 4px; +} diff --git a/webroot/rsrc/css/core/z-index.css b/webroot/rsrc/css/core/z-index.css index bdab2e97ed..75cd394988 100644 --- a/webroot/rsrc/css/core/z-index.css +++ b/webroot/rsrc/css/core/z-index.css @@ -93,6 +93,10 @@ div.phui-calendar-day-event { z-index: 6; } +.diff-banner { + z-index: 6; +} + .conpherence-durable-column { z-index: 7; } diff --git a/webroot/rsrc/js/application/diff/DiffChangeset.js b/webroot/rsrc/js/application/diff/DiffChangeset.js index c3359c2007..148be860b9 100644 --- a/webroot/rsrc/js/application/diff/DiffChangeset.js +++ b/webroot/rsrc/js/application/diff/DiffChangeset.js @@ -19,6 +19,7 @@ JX.install('DiffChangeset', { this._node = node; var data = this._getNodeData(); + this._renderURI = data.renderURI; this._ref = data.ref; this._whitespace = data.whitespace; @@ -30,6 +31,8 @@ JX.install('DiffChangeset', { this._leftID = data.left; this._rightID = data.right; + this._path = data.path; + this._inlines = []; }, @@ -58,6 +61,7 @@ JX.install('DiffChangeset', { _visible: true, _undoNode: null, + _path: null, getLeftChangesetID: function() { return this._leftID; @@ -227,6 +231,9 @@ JX.install('DiffChangeset', { JX.Router.getInstance().queue(routable); }, + getPath: function() { + return this._path; + }, /** * Receive a response to a context request. @@ -428,6 +435,13 @@ JX.install('DiffChangeset', { return JX.Stratcom.getData(this._node); }, + getVectors: function() { + return { + pos: JX.$V(this._node), + dim: JX.Vector.getDim(this._node) + }; + }, + _onresponse: function(sequence, response) { if (sequence != this._sequence) { // If this isn't the most recent request, ignore it. This normally diff --git a/webroot/rsrc/js/application/diff/DiffChangesetList.js b/webroot/rsrc/js/application/diff/DiffChangesetList.js index ff51cd4666..2798d0d1c6 100644 --- a/webroot/rsrc/js/application/diff/DiffChangesetList.js +++ b/webroot/rsrc/js/application/diff/DiffChangesetList.js @@ -51,6 +51,9 @@ JX.install('DiffChangesetList', { var onresize = JX.bind(this, this._ifawake, this._onresize); JX.Stratcom.listen('resize', null, onresize); + var onscroll = JX.bind(this, this._ifawake, this._onscroll); + JX.Stratcom.listen('scroll', null, onscroll); + var onselect = JX.bind(this, this._ifawake, this._onselect); JX.Stratcom.listen( 'mousedown', @@ -113,6 +116,8 @@ JX.install('DiffChangesetList', { _rangeOrigin: null, _rangeTarget: null, + _bannerNode: null, + sleep: function() { this._asleep = true; @@ -807,6 +812,12 @@ JX.install('DiffChangesetList', { this._redrawFocus(); this._redrawSelection(); this._redrawHover(); + + this._redrawBanner(); + }, + + _onscroll: function() { + this._redrawBanner(); }, _onselect: function(e) { @@ -1265,6 +1276,77 @@ JX.install('DiffChangesetList', { this._rangeTarget = null; this.resetHover(); + }, + + _redrawBanner: function() { + var node = this._getBannerNode(); + var changeset = this._getVisibleChangeset(); + + // Don't do anything if nothing has changed. This seems to avoid some + // flickering issues in Safari, at least. + if (this._bannerChangeset === changeset) { + return; + } + this._bannerChangeset = changeset; + + if (!changeset) { + JX.DOM.remove(node); + return; + } + + var icon = new JX.PHUIXIconView() + .setIcon('fa-file') + .getNode(); + JX.DOM.setContent(node, [icon, ' ', changeset.getPath()]); + + document.body.appendChild(node); + }, + + _getBannerNode: function() { + if (!this._bannerNode) { + var attributes = { + className: 'diff-banner', + id: 'diff-banner' + }; + + this._bannerNode = JX.$N('div', attributes); + } + + return this._bannerNode; + }, + + _getVisibleChangeset: function() { + if (this.isAsleep()) { + return null; + } + + if (JX.Device.getDevice() != 'desktop') { + return null; + } + + // Never show the banner if we're very near the top of the page. + var margin = 480; + var s = JX.Vector.getScroll(); + if (s.y < margin) { + return null; + } + + var v = JX.Vector.getViewport(); + for (var ii = 0; ii < this._changesets.length; ii++) { + var changeset = this._changesets[ii]; + var c = changeset.getVectors(); + + // If the changeset starts above the upper half of the screen... + if (c.pos.y < (s.y + (v.y / 2))) { + // ...and ends below the lower half of the screen, this is the + // current visible changeset. + if ((c.pos.y + c.dim.y) > (s.y + (v.y / 2))) { + return changeset; + } + } + } + + return null; } } diff --git a/webroot/rsrc/js/core/behavior-phabricator-nav.js b/webroot/rsrc/js/core/behavior-phabricator-nav.js index cd132550a8..e37680abf0 100644 --- a/webroot/rsrc/js/core/behavior-phabricator-nav.js +++ b/webroot/rsrc/js/core/behavior-phabricator-nav.js @@ -130,8 +130,19 @@ JX.behavior('phabricator-nav', function(config) { return; } + // When the buoyant header is visible, move the menu down below it. This + // is a bit of a hack. + var banner_height = 0; + try { + var banner = JX.$('diff-banner'); + banner_height = JX.Vector.getDim(banner).y; + } catch (error) { + // Ignore if there's no banner on the page. + } + local.style.top = Math.max( 0, + banner_height, JX.$V(content).y - Math.max(0, JX.Vector.getScroll().y)) + 'px'; }