diff --git a/resources/celerity/map.php b/resources/celerity/map.php --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -11,8 +11,8 @@ 'conpherence.pkg.js' => '020aebcf', 'core.pkg.css' => '61b7e380', 'core.pkg.js' => 'fc49f65b', - 'differential.pkg.css' => 'cb99cd21', - 'differential.pkg.js' => 'ae77bf85', + 'differential.pkg.css' => '9b5ee013', + 'differential.pkg.js' => '1043ee5a', 'diffusion.pkg.css' => '42c75c37', 'diffusion.pkg.js' => 'a98c0bf7', 'maniphest.pkg.css' => '35995d6d', @@ -62,7 +62,7 @@ 'rsrc/css/application/diff/diff-tree-view.css' => 'e2d3e222', 'rsrc/css/application/diff/inline-comment-summary.css' => '81eb368d', 'rsrc/css/application/differential/add-comment.css' => '7e5900d9', - 'rsrc/css/application/differential/changeset-view.css' => '489b6995', + 'rsrc/css/application/differential/changeset-view.css' => '5fb26c90', 'rsrc/css/application/differential/core.css' => '7300a73e', 'rsrc/css/application/differential/phui-inline-comment.css' => '48acce5b', 'rsrc/css/application/differential/revision-comment.css' => '7dbc8d1d', @@ -155,7 +155,7 @@ 'rsrc/css/phui/phui-fontkit.css' => '1ec937e5', 'rsrc/css/phui/phui-form-view.css' => '01b796c0', 'rsrc/css/phui/phui-form.css' => '1f177cb7', - 'rsrc/css/phui/phui-formation-view.css' => '82a3b73e', + 'rsrc/css/phui/phui-formation-view.css' => '9a1eff7e', 'rsrc/css/phui/phui-head-thing.css' => 'd7f293df', 'rsrc/css/phui/phui-header-view.css' => '36c86a58', 'rsrc/css/phui/phui-hovercard.css' => '6ca90fa0', @@ -378,8 +378,8 @@ 'rsrc/js/application/dashboard/behavior-dashboard-move-panels.js' => 'a2ab19be', 'rsrc/js/application/dashboard/behavior-dashboard-query-panel-select.js' => '1e413dc9', 'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => '0116d3e8', - 'rsrc/js/application/diff/DiffChangeset.js' => '27305b60', - 'rsrc/js/application/diff/DiffChangesetList.js' => '62a3a351', + 'rsrc/js/application/diff/DiffChangeset.js' => '9a713ba5', + 'rsrc/js/application/diff/DiffChangesetList.js' => 'adf069cd', 'rsrc/js/application/diff/DiffInline.js' => '16e97ebc', 'rsrc/js/application/diff/DiffPathView.js' => '8207abf9', 'rsrc/js/application/diff/DiffTreeView.js' => '5d83623b', @@ -559,7 +559,7 @@ 'conpherence-transaction-css' => '3a3f5e7e', 'd3' => '9d068042', 'diff-tree-view-css' => 'e2d3e222', - 'differential-changeset-view-css' => '489b6995', + 'differential-changeset-view-css' => '5fb26c90', 'differential-core-view-css' => '7300a73e', 'differential-revision-add-comment-css' => '7e5900d9', 'differential-revision-comment-css' => '7dbc8d1d', @@ -775,8 +775,8 @@ 'phabricator-darklog' => '3b869402', 'phabricator-darkmessage' => '26cd4b73', 'phabricator-dashboard-css' => '5a205b9d', - 'phabricator-diff-changeset' => '27305b60', - 'phabricator-diff-changeset-list' => '62a3a351', + 'phabricator-diff-changeset' => '9a713ba5', + 'phabricator-diff-changeset-list' => 'adf069cd', 'phabricator-diff-inline' => '16e97ebc', 'phabricator-diff-path-view' => '8207abf9', 'phabricator-diff-tree-view' => '5d83623b', @@ -846,7 +846,7 @@ 'phui-fontkit-css' => '1ec937e5', 'phui-form-css' => '1f177cb7', 'phui-form-view-css' => '01b796c0', - 'phui-formation-view-css' => '82a3b73e', + 'phui-formation-view-css' => '9a1eff7e', 'phui-head-thing-view-css' => 'd7f293df', 'phui-header-view-css' => '36c86a58', 'phui-hovercard' => '074f0783', @@ -1146,18 +1146,6 @@ 'javelin-json', 'phabricator-prefab', ), - '27305b60' => array( - 'javelin-dom', - 'javelin-util', - 'javelin-stratcom', - 'javelin-install', - 'javelin-workflow', - 'javelin-router', - 'javelin-behavior-device', - 'javelin-vector', - 'phabricator-diff-inline', - 'phabricator-diff-path-view', - ), '289bf236' => array( 'javelin-install', 'javelin-util', @@ -1334,9 +1322,6 @@ 'javelin-dom', 'phabricator-draggable-list', ), - '489b6995' => array( - 'phui-inline-comment-view-css', - ), '48fe33d0' => array( 'javelin-behavior', 'javelin-dom', @@ -1518,14 +1503,12 @@ '5faf27b9' => array( 'phuix-form-control-view', ), + '5fb26c90' => array( + 'phui-inline-comment-view-css', + ), '60cd9241' => array( 'javelin-behavior', ), - '62a3a351' => array( - 'javelin-install', - 'phuix-button-view', - 'phabricator-diff-tree-view', - ), '65bb0011' => array( 'javelin-behavior', 'javelin-dom', @@ -1814,6 +1797,19 @@ 'javelin-request', 'javelin-util', ), + '9a713ba5' => array( + 'javelin-dom', + 'javelin-util', + 'javelin-stratcom', + 'javelin-install', + 'javelin-workflow', + 'javelin-router', + 'javelin-behavior-device', + 'javelin-vector', + 'phabricator-diff-inline', + 'phabricator-diff-path-view', + 'phuix-button-view', + ), '9aae2b66' => array( 'javelin-install', 'javelin-util', @@ -1939,6 +1935,11 @@ 'javelin-typeahead-ondemand-source', 'javelin-util', ), + 'adf069cd' => array( + 'javelin-install', + 'phuix-button-view', + 'phabricator-diff-tree-view', + ), 'aec8e38c' => array( 'javelin-dom', 'javelin-util', diff --git a/src/applications/differential/engine/DifferentialFileTreeEngine.php b/src/applications/differential/engine/DifferentialFileTreeEngine.php --- a/src/applications/differential/engine/DifferentialFileTreeEngine.php +++ b/src/applications/differential/engine/DifferentialFileTreeEngine.php @@ -77,7 +77,18 @@ ->setHref('#')); $flank_view->setHead($head_view); - $tail_view = id(new PHUIListView()) + $tail_view = id(new PHUIListView()); + + if ($viewer->isLoggedIn()) { + $tail_view->addMenuItem( + id(new PHUIListItemView()) + ->setIcon('fa-comment-o') + ->setName(pht('Add Comment')) + ->setKeyCommand('x') + ->setHref('#')); + } + + $tail_view ->addMenuItem( id(new PHUIListItemView()) ->setIcon('fa-chevron-left') diff --git a/src/applications/differential/view/DifferentialChangesetDetailView.php b/src/applications/differential/view/DifferentialChangesetDetailView.php --- a/src/applications/differential/view/DifferentialChangesetDetailView.php +++ b/src/applications/differential/view/DifferentialChangesetDetailView.php @@ -270,9 +270,11 @@ ->setNavigationMarker(true) ->render(), $buttons, - phutil_tag('h1', + javelin_tag( + 'h1', array( 'class' => 'differential-file-icon-header', + 'sigil' => 'changeset-header', ), array( $icon, diff --git a/src/applications/differential/view/DifferentialChangesetListView.php b/src/applications/differential/view/DifferentialChangesetListView.php --- a/src/applications/differential/view/DifferentialChangesetListView.php +++ b/src/applications/differential/view/DifferentialChangesetListView.php @@ -261,9 +261,8 @@ 'Open in Editor' => pht('Open in Editor'), 'Show All Context' => pht('Show All Context'), 'All Context Shown' => pht('All Context Shown'), - "Can't Toggle Unloaded File" => pht("Can't Toggle Unloaded File"), 'Expand File' => pht('Expand File'), - 'Collapse File' => pht('Collapse File'), + 'Hide Changeset' => pht('Hide Changeset'), 'Show Path in Repository' => pht('Show Path in Repository'), 'Show Directory in Repository' => pht('Show Directory in Repository'), 'View Standalone' => pht('View Standalone'), @@ -318,12 +317,8 @@ 'Jump to previous inline comment, including collapsed comments.' => pht('Jump to previous inline comment, including collapsed comments.'), - 'This file content has been collapsed.' => - pht('This file content has been collapsed.'), - 'Show Content' => pht('Show Content'), - - 'Hide or show the current file.' => - pht('Hide or show the current file.'), + 'Hide or show the current changeset.' => + pht('Hide or show the current changeset.'), 'You must select a file to hide or show.' => pht('You must select a file to hide or show.'), @@ -365,6 +360,11 @@ pht('Show path in repository.'), 'Show directory in repository.' => pht('Show directory in repository.'), + + 'Jump to the comment area.' => + pht('Jump to the comment area.'), + + 'Show Changeset' => pht('Show Changeset'), ), )); @@ -439,7 +439,6 @@ ->setHref(idx($meta, 'detailURI', '#')) ->setMetadata($meta) ->addSigil('differential-view-options'); - } private function appendDefaultQueryParams(PhutilURI $uri, array $params) { diff --git a/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php b/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php --- a/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php +++ b/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php @@ -270,10 +270,14 @@ $badge_view = $this->renderBadgeView(); + $anchor = id(new PhabricatorAnchorView()) + ->setAnchorName('reply'); + $comment_box = id(new PHUIObjectBoxView()) ->setFlush(true) ->addClass('phui-comment-form-view') ->addSigil('phui-comment-form') + ->appendChild($anchor) ->appendChild( phutil_tag( 'h3', diff --git a/webroot/rsrc/css/application/differential/changeset-view.css b/webroot/rsrc/css/application/differential/changeset-view.css --- a/webroot/rsrc/css/application/differential/changeset-view.css +++ b/webroot/rsrc/css/application/differential/changeset-view.css @@ -5,8 +5,6 @@ .differential-changeset { position: relative; - margin: 0; - padding-top: 16px; overflow-x: auto; /* Fixes what seems to be a layout bug in Firefox which causes scrollbars, @@ -312,14 +310,16 @@ border-top: none; } -.differential-changeset h1 { +.differential-changeset .differential-file-icon-header { font-size: {$biggestfontsize}; - padding: 2px 0 20px 12px; + padding: 18px 0 20px 12px; + margin-top: 4px; line-height: 20px; color: {$blacktext}; + cursor: pointer; } -.device-phone .differential-changeset h1 { +.device-phone .differential-changeset .differential-file-icon-header { word-break: break-word; margin-right: 8px; } @@ -343,19 +343,6 @@ text-align: center; } -.differential-collapse-undo { - color: {$darkbluetext}; - padding: 12px; - border: 1px solid {$blue}; - text-align: center; - background-color: {$lightblue}; - margin: 8px; -} - -.differential-collapse-undo a { - font-weight: bold; -} - .differential-file-icon-header .phui-icon-view { display: inline-block; margin: 0 6px 2px 0; @@ -369,6 +356,7 @@ .differential-changeset-buttons { float: right; + margin-top: 16px; margin-right: 12px; } @@ -484,3 +472,13 @@ -webkit-user-select: none; user-select: none; } + +.changeset-content-hidden .differential-file-icon-header { + background: {$lightgreybackground}; + color: {$greytext}; +} + +.changeset-selected .differential-file-icon-header { + background: {$lightyellow}; + color: {$blacktext}; +} diff --git a/webroot/rsrc/css/phui/phui-formation-view.css b/webroot/rsrc/css/phui/phui-formation-view.css --- a/webroot/rsrc/css/phui/phui-formation-view.css +++ b/webroot/rsrc/css/phui/phui-formation-view.css @@ -181,3 +181,13 @@ padding: 0; color: {$lightgreytext}; } + +.phui-flank-view-head .phui-list-view { + box-shadow: 0 1px 1px rgba(0, 0, 0, 0.1); + padding-bottom: 4px; +} + +.phui-flank-view-tail .phui-list-view { + box-shadow: 0 -1px 1px rgba(0, 0, 0, 0.1); + padding-top: 4px; +} diff --git a/webroot/rsrc/js/application/diff/DiffChangeset.js b/webroot/rsrc/js/application/diff/DiffChangeset.js --- a/webroot/rsrc/js/application/diff/DiffChangeset.js +++ b/webroot/rsrc/js/application/diff/DiffChangeset.js @@ -10,6 +10,7 @@ * javelin-vector * phabricator-diff-inline * phabricator-diff-path-view + * phuix-button-view * @javelin */ @@ -48,6 +49,9 @@ if (data.changesetState) { this._loadChangesetState(data.changesetState); } + + var onselect = JX.bind(this, this._onClickHeader); + JX.DOM.listen(this._node, 'mousedown', 'changeset-header', onselect); }, members: { @@ -70,7 +74,6 @@ _inlines: null, _visible: true, - _undoNode: null, _displayPath: null, _changesetList: null, @@ -88,6 +91,8 @@ _isLowImportance: null, _isOwned: null, _isHidden: null, + _isSelected: false, + _viewMenu: null, getEditorURI: function() { return this._editorURI; @@ -118,6 +123,11 @@ return this; }, + setViewMenu: function(menu) { + this._viewMenu = menu; + return this; + }, + getIcon: function() { if (!this._visible) { return 'fa-file-o'; @@ -871,6 +881,16 @@ JX.DOM.alterClass(node, 'diff-tree-path-inlines-completed', is_completed); }, + _onClickHeader: function(e) { + e.prevent(); + + if (this._isSelected) { + this.getChangesetList().selectChangeset(null); + } else { + this.select(false); + } + }, + toggleVisibility: function() { this.setVisible(!this._visible); @@ -888,55 +908,85 @@ setVisible: function(visible) { this._visible = visible; - var diff = JX.DOM.find(this._node, 'table', 'differential-diff'); - var undo = this._getUndoNode(); + var diff = this._getDiffNode(); + var options = this._getViewButtonNode(); + var show = this._getShowButtonNode(); if (this._visible) { JX.DOM.show(diff); - JX.DOM.remove(undo); + JX.DOM.show(options); + JX.DOM.hide(show); } else { JX.DOM.hide(diff); - JX.DOM.appendContent(diff.parentNode, undo); + JX.DOM.hide(options); + JX.DOM.show(show); + + if (this._viewMenu) { + this._viewMenu.close(); + } } JX.Stratcom.invoke('resize'); + var node = this._node; + JX.DOM.alterClass(node, 'changeset-content-hidden', !this._visible); + this.getPathView().setIsHidden(!this._visible); }, - isVisible: function() { - return this._visible; - }, + setIsSelected: function(is_selected) { + this._isSelected = !!is_selected; - _getUndoNode: function() { - if (!this._undoNode) { - var pht = this.getChangesetList().getTranslations(); + var node = this._node; + JX.DOM.alterClass(node, 'changeset-selected', this._isSelected); - var link_attributes = { - href: '#' - }; + return this; + }, - var undo_link = JX.$N('a', link_attributes, pht('Show Content')); + _getDiffNode: function() { + if (!this._diffNode) { + this._diffNode = JX.DOM.find(this._node, 'table', 'differential-diff'); + } + return this._diffNode; + }, + + _getViewButtonNode: function() { + if (!this._viewButtonNode) { + this._viewButtonNode = JX.DOM.find( + this._node, + 'a', + 'differential-view-options'); + } + return this._viewButtonNode; + }, - var onundo = JX.bind(this, this._onundo); - JX.DOM.listen(undo_link, 'click', null, onundo); + _getShowButtonNode: function() { + if (!this._showButtonNode) { + var pht = this.getChangesetList().getTranslations(); - var node_attributes = { - className: 'differential-collapse-undo' - }; + var show_button = new JX.PHUIXButtonView() + .setIcon('fa-angle-double-down') + .setText(pht('Show Changeset')) + .setColor('grey'); - var node_content = [ - pht('This file content has been collapsed.'), - ' ', - undo_link - ]; + var button_node = show_button.getNode(); + this._getViewButtonNode().parentNode.appendChild(button_node); - var undo_node = JX.$N('div', node_attributes, node_content); + var onshow = JX.bind(this, this._onClickShowButton); + JX.DOM.listen(button_node, 'click', null, onshow); - this._undoNode = undo_node; + this._showButtonNode = button_node; } + return this._showButtonNode; + }, - return this._undoNode; + _onClickShowButton: function(e) { + e.prevent(); + this.setVisible(true); + }, + + isVisible: function() { + return this._visible; }, _onundo: function(e) { diff --git a/webroot/rsrc/js/application/diff/DiffChangesetList.js b/webroot/rsrc/js/application/diff/DiffChangesetList.js --- a/webroot/rsrc/js/application/diff/DiffChangesetList.js +++ b/webroot/rsrc/js/application/diff/DiffChangesetList.js @@ -124,6 +124,7 @@ _dropdownMenu: null, _menuButton: null, _menuItems: null, + _selectedChangeset: null, sleep: function() { this._asleep = true; @@ -165,6 +166,9 @@ if (!standalone) { label = pht('Jump to the table of contents.'); this._installKey('t', 'diff-nav', label, this._ontoc); + + label = pht('Jump to the comment area.'); + this._installKey('x', 'diff-nav', label, this._oncomments); } label = pht('Jump to next change.'); @@ -203,7 +207,7 @@ } if (!standalone) { - label = pht('Hide or show the current file.'); + label = pht('Hide or show the current changeset.'); this._installKey('h', 'diff-vis', label, this._onkeytogglefile); } @@ -329,6 +333,11 @@ manager.scrollTo(toc); }, + _oncomments: function(manager) { + var reply = JX.$('reply'); + manager.scrollTo(reply); + }, + getSelectedInline: function() { var cursor = this._cursorItem; @@ -703,6 +712,8 @@ if (cursor !== null) { this._setSelectionState(items[cursor], scroll); + } else { + this._setSelectionState(null, false); } return this; @@ -731,13 +742,17 @@ return; } + var changeset = cursor.changeset; + var tree = this._getTreeView(); - if (cursor.changeset) { + if (changeset) { tree.setSelectedPath(cursor.changeset.getPathView()); } else { tree.setSelectedPath(null); } + this._selectChangeset(changeset); + this.setFocus(cursor.nodes.begin, cursor.nodes.end); if (scroll) { @@ -1047,8 +1062,9 @@ visible_item .setDisabled(true) - .setIcon('fa-expand') - .setName(pht('Can\'t Toggle Unloaded File')); + .setIcon('fa-eye-slash') + .setName(pht('Hide Changeset')); + var diffs = JX.DOM.scry( JX.$(data.containerID), 'table', @@ -1059,17 +1075,7 @@ 'More than one node with sigil "differential-diff" was found in "'+ data.containerID+'."'); } else if (diffs.length == 1) { - var diff = diffs[0]; visible_item.setDisabled(false); - if (!changeset.isVisible()) { - visible_item - .setName(pht('Expand File')) - .setIcon('fa-expand'); - } else { - visible_item - .setName(pht('Collapse File')) - .setIcon('fa-compress'); - } } else { // Do nothing when there is no diff shown in the table. For example, // the file is binary. @@ -1078,6 +1084,7 @@ }); data.menu = menu; + changeset.setViewMenu(menu); menu.open(); }, @@ -1224,6 +1231,7 @@ if (!node) { var tree = this._getTreeView(); tree.setSelectedPath(null); + this._selectChangeset(null); } this._focusStart = node; @@ -1231,6 +1239,22 @@ this._redrawFocus(); }, + _selectChangeset: function(changeset) { + if (this._selectedChangeset === changeset) { + return; + } + + if (this._selectedChangeset !== null) { + this._selectedChangeset.setIsSelected(false); + this._selectedChangeset = null; + } + + this._selectedChangeset = changeset; + if (this._selectedChangeset !== null) { + this._selectedChangeset.setIsSelected(true); + } + }, + _redrawFocus: function() { var node = this._focusStart; var extended_node = this._focusEnd || node; @@ -1247,13 +1271,13 @@ var s = JX.Vector.getAggregateScrollForNode(node); var d = JX.Vector.getDim(node); - p.add(s).add(d.x + 1, 0).setPos(reticle); + p.add(s).add(d.x + 1, 4).setPos(reticle); // Compute the size we need to extend to the full extent of the focused // nodes. JX.Vector.getPos(extended_node) .add(-p.x, -p.y) .add(0, JX.Vector.getDim(extended_node).y) - .add(10, 0) + .add(10, -4) .setDim(reticle); JX.DOM.getContentFrame().appendChild(reticle);