@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Fix three issues with scrolling while dragging items (e.g., on workboards)

Summary:
Fixes T5979. There are three issues here:

- We cache document positions when you pick an item up, but don't recalculate them after you scroll, so they get out of date. Dirty the cache when the user scrolls.
- When we rebuild the cache during a drag (previously, this never happened), the position of the object you're dragging is computed wrong (since it has been moved to be under the cursor). Adjust the effective position of the object you've picked up to put it back in the right place in the list.
- When you fiddle around at the bottom of a column you can get jumpy redraws as the height adjusts. Put `min-height` on the container during a drag to prevent this.

Test Plan: In Safari, Chrome and Firefox, dragged items around on columns before and after scrolling the workboard panel.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5979

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

+117 -34
+22 -21
resources/celerity/map.php
··· 8 8 return array( 9 9 'names' => array( 10 10 'core.pkg.css' => '974635bb', 11 - 'core.pkg.js' => '47fd11f0', 11 + 'core.pkg.js' => 'f0e2c091', 12 12 'darkconsole.pkg.js' => 'df001cab', 13 13 'differential.pkg.css' => '36884139', 14 14 'differential.pkg.js' => '73337d1d', ··· 412 412 'rsrc/js/application/policy/behavior-policy-rule-editor.js' => 'fe9a552f', 413 413 'rsrc/js/application/ponder/behavior-votebox.js' => '4e9b766b', 414 414 'rsrc/js/application/projects/behavior-boards-dropdown.js' => '0ec56e1d', 415 - 'rsrc/js/application/projects/behavior-project-boards.js' => 'a6c6a058', 415 + 'rsrc/js/application/projects/behavior-project-boards.js' => '0676345e', 416 416 'rsrc/js/application/projects/behavior-project-create.js' => '065227cc', 417 417 'rsrc/js/application/projects/behavior-reorder-columns.js' => 'e1d25dfb', 418 418 'rsrc/js/application/releeph/releeph-preview-branch.js' => 'b2b4fbaf', ··· 438 438 'rsrc/js/application/uiexample/notification-example.js' => '7a9677fc', 439 439 'rsrc/js/core/Busy.js' => '6453c869', 440 440 'rsrc/js/core/DragAndDropFileUpload.js' => '8c49f386', 441 - 'rsrc/js/core/DraggableList.js' => '2a6a1041', 441 + 'rsrc/js/core/DraggableList.js' => 'a16ec1c6', 442 442 'rsrc/js/core/FileUpload.js' => 'a4ae61bf', 443 443 'rsrc/js/core/Hovercard.js' => '7e8468ae', 444 444 'rsrc/js/core/KeyboardShortcut.js' => '1ae869f2', ··· 636 636 'javelin-behavior-policy-control' => 'f3fef818', 637 637 'javelin-behavior-policy-rule-editor' => 'fe9a552f', 638 638 'javelin-behavior-ponder-votebox' => '4e9b766b', 639 - 'javelin-behavior-project-boards' => 'a6c6a058', 639 + 'javelin-behavior-project-boards' => '0676345e', 640 640 'javelin-behavior-project-create' => '065227cc', 641 641 'javelin-behavior-refresh-csrf' => '7814b593', 642 642 'javelin-behavior-releeph-preview-branch' => 'b2b4fbaf', ··· 713 713 'phabricator-crumbs-view-css' => 'a49339de', 714 714 'phabricator-dashboard-css' => 'a2bfdcbf', 715 715 'phabricator-drag-and-drop-file-upload' => '8c49f386', 716 - 'phabricator-draggable-list' => '2a6a1041', 716 + 'phabricator-draggable-list' => 'a16ec1c6', 717 717 'phabricator-fatal-config-template-css' => '25d446d6', 718 718 'phabricator-feed-css' => '7bfc6f12', 719 719 'phabricator-file-upload' => 'a4ae61bf', ··· 851 851 'javelin-stratcom', 852 852 'javelin-workflow', 853 853 ), 854 + '0676345e' => array( 855 + 'javelin-behavior', 856 + 'javelin-dom', 857 + 'javelin-util', 858 + 'javelin-vector', 859 + 'javelin-stratcom', 860 + 'javelin-workflow', 861 + 'phabricator-draggable-list', 862 + ), 854 863 '06e05112' => array( 855 864 'javelin-behavior', 856 865 'javelin-stratcom', ··· 987 996 'javelin-install', 988 997 'javelin-util', 989 998 ), 990 - '2a6a1041' => array( 991 - 'javelin-install', 992 - 'javelin-dom', 993 - 'javelin-stratcom', 994 - 'javelin-util', 995 - 'javelin-vector', 996 - 'javelin-magical-init', 997 - ), 998 999 '2b228192' => array( 999 1000 'javelin-behavior', 1000 1001 'javelin-dom', ··· 1477 1478 'javelin-dom', 1478 1479 'javelin-reactor-dom', 1479 1480 ), 1481 + 'a16ec1c6' => array( 1482 + 'javelin-install', 1483 + 'javelin-dom', 1484 + 'javelin-stratcom', 1485 + 'javelin-util', 1486 + 'javelin-vector', 1487 + 'javelin-magical-init', 1488 + ), 1480 1489 'a4ae61bf' => array( 1481 1490 'javelin-install', 1482 1491 'javelin-dom', ··· 1484 1493 ), 1485 1494 'a5d7cf86' => array( 1486 1495 'javelin-dom', 1487 - ), 1488 - 'a6c6a058' => array( 1489 - 'javelin-behavior', 1490 - 'javelin-dom', 1491 - 'javelin-util', 1492 - 'javelin-stratcom', 1493 - 'javelin-workflow', 1494 - 'phabricator-draggable-list', 1495 1496 ), 1496 1497 'a80d0378' => array( 1497 1498 'javelin-behavior',
+4 -1
src/view/layout/AphrontMultiColumnView.php
··· 139 139 ->addPadding(PHUI::PADDING_MEDIUM_BOTTOM); 140 140 } 141 141 142 - return phutil_tag( 142 + return javelin_tag( 143 143 'div', 144 144 array( 145 145 'class' => 'aphront-multi-column-view', 146 146 'id' => $this->getID(), 147 + // TODO: It would be nice to convert this to an AphrontTagView and 148 + // use addSigil() from Workboards instead of hard-coding this. 149 + 'sigil' => 'aphront-multi-column-view', 147 150 ), 148 151 $board); 149 152 }
+39
webroot/rsrc/js/application/projects/behavior-project-boards.js
··· 3 3 * @requires javelin-behavior 4 4 * javelin-dom 5 5 * javelin-util 6 + * javelin-vector 6 7 * javelin-stratcom 7 8 * javelin-workflow 8 9 * phabricator-draggable-list ··· 88 89 return 0; 89 90 } 90 91 92 + function getcontainer() { 93 + return JX.DOM.find( 94 + JX.$(config.boardID), 95 + 'div', 96 + 'aphront-multi-column-view'); 97 + } 98 + 99 + function onbegindrag(item) { 100 + // If the longest column on the board is taller than the window, the board 101 + // will scroll vertically. Dragging an item to the longest column may 102 + // make it longer, by the total height of the board, plus the height of 103 + // the drop target. 104 + 105 + // If this happens, the scrollbar will jump around and the scroll position 106 + // can be adjusted in a disorienting way. To reproduce this, drag a task 107 + // to the bottom of the longest column on a scrolling board and wave the 108 + // task in and out of the column. The scroll bar will jump around and 109 + // it will be hard to lock onto a target. 110 + 111 + // To fix this, set the minimum board height to the current board height 112 + // plus the size of the drop target (which is the size of the item plus 113 + // a bit of margin). This makes sure the scroll bar never needs to 114 + // recalculate. 115 + 116 + var item_size = JX.Vector.getDim(item); 117 + var container = getcontainer(); 118 + var container_size = JX.Vector.getDim(container); 119 + 120 + container.style.minHeight = (item_size.y + container_size.y + 12) + 'px'; 121 + } 122 + 123 + function onenddrag() { 124 + getcontainer().style.minHeight = ''; 125 + } 126 + 91 127 function ondrop(list, item, after) { 92 128 list.lock(); 93 129 JX.DOM.alterClass(item, 'drag-sending', true); ··· 150 186 list.listen('didReceive', JX.bind(list, onupdate, cols[ii])); 151 187 152 188 list.listen('didDrop', JX.bind(null, ondrop, list)); 189 + 190 + list.listen('didBeginDrag', JX.bind(null, onbegindrag)); 191 + list.listen('didEndDrag', JX.bind(null, onenddrag)); 153 192 154 193 lists.push(list); 155 194
+52 -12
webroot/rsrc/js/core/DraggableList.js
··· 53 53 _ghostNode : null, 54 54 _group : null, 55 55 _lastMousePosition: null, 56 + _lastAdjust: null, 56 57 57 58 getRootNode : function() { 58 59 return this._root; ··· 174 175 175 176 for (var ii = 0; ii < this._group.length; ii++) { 176 177 this._group[ii]._clearTarget(); 177 - this._group[ii]._generateTargets(); 178 178 } 179 179 180 180 if (!this.invoke('didBeginDrag', this._dragging).getPrevented()) { ··· 189 189 } 190 190 }, 191 191 192 - _generateTargets : function() { 193 - var targets = []; 194 - var items = this.findItems(); 195 - for (var ii = 0; ii < items.length; ii++) { 196 - targets.push({ 197 - item: items[ii], 198 - y: JX.$V(items[ii]).y + (JX.Vector.getDim(items[ii]).y / 2) 199 - }); 192 + _getTargets : function() { 193 + if (this._targets === null) { 194 + var targets = []; 195 + var items = this.findItems(); 196 + for (var ii = 0; ii < items.length; ii++) { 197 + var item = items[ii]; 198 + 199 + var ipos = JX.$V(item); 200 + if (item == this._dragging) { 201 + // If the item we're measuring is also the item we're dragging, 202 + // we need to measure its position as though it was still in the 203 + // list, not its current position in the document (which is 204 + // under the cursor). To do this, adjust the measured position by 205 + // removing the offsets we added to put the item underneath the 206 + // cursor. 207 + if (this._lastAdjust) { 208 + ipos.x -= this._lastAdjust.x; 209 + ipos.y -= this._lastAdjust.y; 210 + } 211 + } 212 + 213 + targets.push({ 214 + item: items[ii], 215 + y: ipos.y + (JX.Vector.getDim(items[ii]).y / 2) 216 + }); 217 + } 218 + targets.sort(function(u, v) { return v.y - u.y; }); 219 + this._targets = targets; 200 220 } 201 - targets.sort(function(u, v) { return v.y - u.y; }); 202 - this._targets = targets; 221 + 222 + return this._targets; 223 + }, 224 + 225 + _dirtyTargetCache: function() { 226 + if (this._hasGroup()) { 227 + var group = this._group; 228 + for (var ii = 0; ii < group.length; ii++) { 229 + group[ii]._targets = null; 230 + } 231 + } else { 232 + this._targets = null; 233 + } 203 234 204 235 return this; 205 236 }, ··· 264 295 265 296 _getCurrentTarget : function(p) { 266 297 var ghost = this.getGhostNode(); 267 - var targets = this._targets; 298 + var targets = this._getTargets(); 268 299 var dragging = this._dragging; 269 300 270 301 var adjust_h = JX.Vector.getDim(ghost).y; ··· 348 379 return; 349 380 } 350 381 382 + if (e.getType() == 'scroll') { 383 + // If this is a scroll event, the positions of drag targets may have 384 + // changed. 385 + this._dirtyTargetCache(); 386 + } 387 + 351 388 var p = JX.$V(this._lastMousePosition.x, this._lastMousePosition.y); 352 389 353 390 var group = this._group; ··· 402 439 } 403 440 404 441 p.y -= origin.y; 442 + this._lastAdjust = new JX.Vector(p.x, p.y); 405 443 p.setPos(this._dragging); 406 444 407 445 e.kill(); ··· 442 480 for (var ii = 0; ii < group.length; ii++) { 443 481 JX.DOM.alterClass(group[ii].getRootNode(), 'drag-target-list', false); 444 482 group[ii]._clearTarget(); 483 + group[ii]._dirtyTargetCache(); 484 + group[ii]._lastAdjust = null; 445 485 } 446 486 447 487 if (!this.invoke('didEndDrag', dragging).getPrevented()) {