@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.

When dragging nodes, clone them

Summary:
Ref T5240. Currently, when dragging nodes, we leave them where they are in the document and apply "position: relative;" so we can move them around on screen.

- Pros: All the CSS still works.
- Cons: Can't drag them outside the nearest containing element with "overflow: hidden;", many subtle positioning bugs with scrollable containers.

Instead, this diff leaves the thing we're dragging exactly where it is, clones it, and drags the clone instead.

- Pros: You can drag it anywhere. Seems to fix all the scrolling container problems.
- Cons: CSS which depends on a container class no longer works.

The CSS thing is bad, but doesn't seem too unreasonable to fix. Basically, we just need to put some `phui-this-is-a-workboard-card` class on the cards, and use that to style them instead of `phui-workboard-view`, and then do something similar for draggable lists.

Although we no longer need to drag cards to tabs with the current design, I think there's a reasonable chance we'll revisit that later. The current design also calls for scrollable columns, but there would be no way to drag cards outside of their current column with the current approach.

NOTE: This does not attempt to fix the CSS, so dragging is pretty rough, since the "clone" loses a number of container classes and thus a number of rules. I'll clean up the CSS in the next change.

Test Plan:
- Dragged stuff around on task lists, workboards, and sort lists (e.g., pinned applications) in Safari, Firefox and Chrome.
- Scrolled window and containers (workboards) during drag.
- Dragged stuff out of the workboard.
- Dragged stuff offscreen.
- CSS is funky, but I can no longer find any positioning or layout issues in any browser.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T5240

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

+137 -86
+16 -16
resources/celerity/map.php
··· 7 7 */ 8 8 return array( 9 9 'names' => array( 10 - 'core.pkg.css' => '8b9c004a', 11 - 'core.pkg.js' => 'bf947f93', 10 + 'core.pkg.css' => 'cd66e467', 11 + 'core.pkg.js' => 'c5178ede', 12 12 'darkconsole.pkg.js' => 'e7393ebb', 13 13 'differential.pkg.css' => '2de124c9', 14 14 'differential.pkg.js' => '5c2ba922', ··· 105 105 'rsrc/css/core/core.css' => '5b3563c8', 106 106 'rsrc/css/core/remarkup.css' => 'e1c8b32f', 107 107 'rsrc/css/core/syntax.css' => '9fd11da8', 108 - 'rsrc/css/core/z-index.css' => 'a36a45da', 108 + 'rsrc/css/core/z-index.css' => '5c7025bf', 109 109 'rsrc/css/diviner/diviner-shared.css' => 'aa3656aa', 110 110 'rsrc/css/font/font-aleo.css' => '8bdb2835', 111 111 'rsrc/css/font/font-awesome.css' => 'c43323c5', ··· 142 142 'rsrc/css/phui/phui-info-view.css' => '6d7c3509', 143 143 'rsrc/css/phui/phui-list.css' => '9da2aa00', 144 144 'rsrc/css/phui/phui-object-box.css' => '407eaf5a', 145 - 'rsrc/css/phui/phui-object-item-list-view.css' => 'febf4a79', 145 + 'rsrc/css/phui/phui-object-item-list-view.css' => '699de05e', 146 146 'rsrc/css/phui/phui-pager.css' => 'bea33d23', 147 147 'rsrc/css/phui/phui-pinboard-view.css' => '2495140e', 148 148 'rsrc/css/phui/phui-profile-menu.css' => 'ab4fcf5f', ··· 446 446 'rsrc/js/application/uiexample/notification-example.js' => '8ce821c5', 447 447 'rsrc/js/core/Busy.js' => '59a7976a', 448 448 'rsrc/js/core/DragAndDropFileUpload.js' => 'ad10aeac', 449 - 'rsrc/js/core/DraggableList.js' => '255d85da', 449 + 'rsrc/js/core/DraggableList.js' => 'f1844746', 450 450 'rsrc/js/core/FileUpload.js' => '477359c8', 451 451 'rsrc/js/core/Hovercard.js' => 'c6f720ff', 452 452 'rsrc/js/core/KeyboardShortcut.js' => '1ae869f2', ··· 741 741 'phabricator-countdown-css' => 'e7544472', 742 742 'phabricator-dashboard-css' => 'eb458607', 743 743 'phabricator-drag-and-drop-file-upload' => 'ad10aeac', 744 - 'phabricator-draggable-list' => '255d85da', 744 + 'phabricator-draggable-list' => 'f1844746', 745 745 'phabricator-fatal-config-template-css' => '8e6c6fcd', 746 746 'phabricator-feed-css' => 'ecd4ec57', 747 747 'phabricator-file-upload' => '477359c8', ··· 780 780 'phabricator-uiexample-reactor-select' => 'a155550f', 781 781 'phabricator-uiexample-reactor-sendclass' => '1def2711', 782 782 'phabricator-uiexample-reactor-sendproperties' => 'b1f0ccee', 783 - 'phabricator-zindex-css' => 'a36a45da', 783 + 'phabricator-zindex-css' => '5c7025bf', 784 784 'phame-css' => '6d5b3682', 785 785 'pholio-css' => '95174bdd', 786 786 'pholio-edit-css' => '3ad9d1ee', ··· 818 818 'phui-inline-comment-view-css' => '0fdb3667', 819 819 'phui-list-view-css' => '9da2aa00', 820 820 'phui-object-box-css' => '407eaf5a', 821 - 'phui-object-item-list-view-css' => 'febf4a79', 821 + 'phui-object-item-list-view-css' => '699de05e', 822 822 'phui-pager-css' => 'bea33d23', 823 823 'phui-pinboard-view-css' => '2495140e', 824 824 'phui-profile-menu-css' => 'ab4fcf5f', ··· 1021 1021 'phabricator-drag-and-drop-file-upload', 1022 1022 'phabricator-draggable-list', 1023 1023 ), 1024 - '255d85da' => array( 1025 - 'javelin-install', 1026 - 'javelin-dom', 1027 - 'javelin-stratcom', 1028 - 'javelin-util', 1029 - 'javelin-vector', 1030 - 'javelin-magical-init', 1031 - ), 1032 1024 '2926fff2' => array( 1033 1025 'javelin-behavior', 1034 1026 'javelin-dom', ··· 2018 2010 'javelin-util', 2019 2011 'javelin-workflow', 2020 2012 'javelin-json', 2013 + ), 2014 + 'f1844746' => array( 2015 + 'javelin-install', 2016 + 'javelin-dom', 2017 + 'javelin-stratcom', 2018 + 'javelin-util', 2019 + 'javelin-vector', 2020 + 'javelin-magical-init', 2021 2021 ), 2022 2022 'f411b6ae' => array( 2023 2023 'javelin-behavior',
+14 -7
src/view/phui/PHUIObjectItemView.php
··· 383 383 ), 384 384 $this->header); 385 385 386 - $header = javelin_tag( 386 + // Wrap the header content in a <span> with the "slippery" sigil. This 387 + // prevents us from beginning a drag if you click the text (like "T123"), 388 + // but not if you click the white space after the header. 389 + $header = phutil_tag( 387 390 'div', 388 391 array( 389 392 'class' => 'phui-object-item-name', 390 - 'sigil' => 'slippery', 391 393 ), 392 - array( 393 - $this->headIcons, 394 - $header_name, 395 - $header_link, 396 - )); 394 + javelin_tag( 395 + 'span', 396 + array( 397 + 'sigil' => 'slippery', 398 + ), 399 + array( 400 + $this->headIcons, 401 + $header_name, 402 + $header_link, 403 + ))); 397 404 398 405 $icons = array(); 399 406 if ($this->icons) {
+4 -4
webroot/rsrc/css/core/z-index.css
··· 81 81 z-index: 5; 82 82 } 83 83 84 - .drag-dragging { 85 - z-index: 5; 86 - } 87 - 88 84 .phui-calendar-date-number { 89 85 z-index: 5; 90 86 } ··· 112 108 113 109 .device-desktop .phabricator-notification-menu { 114 110 z-index: 9; 111 + } 112 + 113 + .drag-frame { 114 + z-index: 10; 115 115 } 116 116 117 117 .jx-mask {
+24 -3
webroot/rsrc/css/phui/phui-object-item-list-view.css
··· 593 593 } 594 594 595 595 .drag-dragging { 596 - position: relative; 597 - background: {$sh-yellowbackground}; 598 - opacity: 0.9; 596 + opacity: 0.25; 599 597 } 600 598 601 599 .drag-sending { 602 600 opacity: 0.5; 603 601 } 602 + 603 + .drag-clone, 604 + .drag-frame { 605 + /* This allows mousewheel events to pass through the clone and frame while 606 + they are being dragged. Without this, the mousewheel does not work during 607 + a drag operation. */ 608 + pointer-events: none; 609 + } 610 + 611 + .drag-frame { 612 + position: fixed; 613 + overflow: hidden; 614 + left: 0; 615 + right: 0; 616 + top: 0; 617 + bottom: 0; 618 + } 619 + 620 + .drag-clone { 621 + position: absolute; 622 + list-style: none; 623 + } 624 + 604 625 605 626 /* - State --------------------------------------------------------------------- 606 627
+79 -56
webroot/rsrc/js/core/DraggableList.js
··· 44 44 _root : null, 45 45 _dragging : null, 46 46 _locked : 0, 47 - _origin : null, 48 - _originScroll : null, 49 47 _target : null, 50 48 _targets : null, 51 49 _ghostHandler : null, 52 50 _ghostNode : null, 53 51 _group : null, 54 52 _lastMousePosition: null, 55 - _lastAdjust: null, 53 + _frame: null, 54 + _clone: null, 55 + _offset: null, 56 56 57 57 getRootNode : function() { 58 58 return this._root; ··· 131 131 } 132 132 } 133 133 134 - return handler(); 134 + var items = handler(); 135 + 136 + // Make sure the clone element is never included as a target. 137 + for (var ii = 0; ii < items.length; ii++) { 138 + if (items[ii] === this._clone) { 139 + items.splice(ii, 1); 140 + break; 141 + } 142 + } 143 + 144 + return items; 135 145 }, 136 146 137 147 _ondrag : function(e) { ··· 167 177 168 178 e.kill(); 169 179 170 - this._dragging = e.getNode(this._sigil); 171 - this._origin = JX.$V(e); 172 - this._originScroll = JX.Vector.getAggregateScrollForNode(this._dragging); 180 + var drag = e.getNode(this._sigil); 173 181 174 182 for (var ii = 0; ii < this._group.length; ii++) { 175 183 this._group[ii]._clearTarget(); 176 184 } 177 185 178 - if (!this.invoke('didBeginDrag', this._dragging).getPrevented()) { 179 - // Set the height of all the ghosts in the group. In the normal case, 180 - // this just sets this list's ghost height. 181 - for (var jj = 0; jj < this._group.length; jj++) { 182 - var ghost = this._group[jj].getGhostNode(); 183 - ghost.style.height = JX.Vector.getDim(this._dragging).y + 'px'; 184 - } 186 + var pos = JX.$V(drag); 187 + var dim = JX.Vector.getDim(drag); 185 188 186 - JX.DOM.alterClass(this._dragging, 'drag-dragging', true); 189 + // Create and adjust the ghost nodes. 190 + for (var jj = 0; jj < this._group.length; jj++) { 191 + var ghost = this._group[jj].getGhostNode(); 192 + ghost.style.height = dim.y + 'px'; 187 193 } 194 + 195 + // Here's what's going on: we're cloning the thing that's being dragged. 196 + // This is the "clone", stored in "this._clone". We're going to leave the 197 + // original where it is in the document, and put the clone at top-level 198 + // so it can be freely dragged around the whole document, even if it's 199 + // inside a container with overflow hidden. 200 + 201 + // Because the clone has been moved up, CSS classes which rely on some 202 + // parent selector won't work. Draggable objects need to pick up all of 203 + // their CSS properties without relying on container classes. This isn't 204 + // great, but leaving them where they are in the document creates a large 205 + // number of positioning problems with scrollable, absolute, relative, 206 + // or overflow hidden containers. 207 + 208 + // Note that we don't actually want to let the user drag it outside the 209 + // document. One problem is that doing so lets the user drag objects 210 + // infinitely far to the right by dragging them to the edge so the 211 + // document extends, scrolling the document, dragging them to the edge 212 + // of the new larger document, scrolling the document, and so on forever. 213 + 214 + // To prevent this, we're putting a "frame" (stored in "this._frame") at 215 + // top level, then putting the clone inside the frame. The frame has the 216 + // same size as the entire viewport, and overflow hidden, so dragging the 217 + // item outside the document just cuts it off. 218 + 219 + // Create the clone for dragging. 220 + var clone = drag.cloneNode(true); 221 + 222 + pos.setPos(clone); 223 + dim.setDim(clone); 224 + 225 + JX.DOM.alterClass(drag, 'drag-dragging', true); 226 + JX.DOM.alterClass(clone, 'drag-clone', true); 227 + 228 + var frame = JX.$N('div', {className: 'drag-frame'}); 229 + frame.appendChild(clone); 230 + 231 + document.body.appendChild(frame); 232 + 233 + this._dragging = drag; 234 + this._clone = clone; 235 + this._frame = frame; 236 + 237 + var cursor = JX.$V(e); 238 + this._offset = new JX.Vector(pos.x - cursor.x, pos.y - cursor.y); 239 + 240 + this.invoke('didBeginDrag', this._dragging); 188 241 }, 189 242 190 243 _getTargets : function() { ··· 195 248 var item = items[ii]; 196 249 197 250 var ipos = JX.$V(item); 198 - if (item == this._dragging) { 199 - // If the item we're measuring is also the item we're dragging, 200 - // we need to measure its position as though it was still in the 201 - // list, not its current position in the document (which is 202 - // under the cursor). To do this, adjust the measured position by 203 - // removing the offsets we added to put the item underneath the 204 - // cursor. 205 - if (this._lastAdjust) { 206 - ipos.x -= this._lastAdjust.x; 207 - ipos.y -= this._lastAdjust.y; 208 - } 209 - } 210 251 211 252 targets.push({ 212 253 item: items[ii], ··· 398 439 } 399 440 } 400 441 401 - // If the drop target indicator is above the cursor in the document, 402 - // adjust the cursor position for the change in node document position. 403 - // Do this before choosing a new target to avoid a flash of nonsense. 404 - 405 - var scroll = JX.Vector.getAggregateScrollForNode(this._dragging); 406 - 407 - var origin = { 408 - x: this._origin.x + (this._originScroll.x - scroll.x), 409 - y: this._origin.y + (this._originScroll.y - scroll.y) 410 - }; 411 - 412 - var adjust_h = 0; 413 - var adjust_y = 0; 414 - if (this._target !== false) { 415 - var ghost = this.getGhostNode(); 416 - adjust_h = JX.Vector.getDim(ghost).y; 417 - adjust_y = JX.$V(ghost).y; 442 + var f = JX.$V(this._frame); 443 + p.x -= f.x; 444 + p.y -= f.y; 418 445 419 - if (adjust_y <= origin.y) { 420 - p.y -= adjust_h; 421 - } 422 - } 446 + p.y += this._offset.y; 447 + this._clone.style.top = p.y + 'px'; 423 448 424 449 if (this._canDragX()) { 425 - p.x -= origin.x; 426 - } else { 427 - p.x = 0; 450 + p.x += this._offset.x; 451 + this._clone.style.left = p.x + 'px'; 428 452 } 429 - 430 - p.y -= origin.y; 431 - this._lastAdjust = new JX.Vector(p.x, p.y); 432 - p.setPos(this._dragging); 433 453 434 454 e.kill(); 435 455 }, ··· 443 463 444 464 var dragging = this._dragging; 445 465 this._dragging = null; 466 + 467 + JX.DOM.remove(this._frame); 468 + this._frame = null; 469 + this._clone = null; 446 470 447 471 var target = false; 448 472 var ghost = false; ··· 469 493 for (var ii = 0; ii < group.length; ii++) { 470 494 JX.DOM.alterClass(group[ii].getRootNode(), 'drag-target-list', false); 471 495 group[ii]._clearTarget(); 472 - group[ii]._lastAdjust = null; 473 496 } 474 497 475 498 if (!this.invoke('didEndDrag', dragging).getPrevented()) {