Skip to content

Commit 6e9baca

Browse files
authored
Merge pull request #22838 from opf/feature/73473-backlogs-card-draggable
[#73473] Make whole Backlog card draggable
2 parents 7606b87 + 88b2ea2 commit 6e9baca

20 files changed

Lines changed: 503 additions & 104 deletions

File tree

frontend/src/assets/sass/backlogs/_master_backlog.sass

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,6 @@ $op-backlogs-header--points-min-width-narrow: 2rem
3838
grid-template-columns: 1fr minmax($op-backlogs-header--points-min-width, max-content) auto
3939
align-items: center
4040

41-
&--collapsible
42-
margin-left: calc(var(--stack-padding-normal) / 2)
43-
4441
&--actions
4542
margin-left: var(--stack-gap-normal)
4643

@@ -67,16 +64,13 @@ $op-backlogs-header--points-min-width-narrow: 2rem
6764

6865
.op-backlogs-story
6966
display: grid
70-
grid-template-columns: var(--control-xsmall-size) 1fr minmax($op-backlogs-header--points-min-width, max-content) auto
67+
grid-template-columns: 1fr minmax($op-backlogs-header--points-min-width, max-content) auto
7168
grid-template-rows: auto auto
72-
grid-template-areas: "drag_handle info_line points menu" ". subject subject subject"
69+
grid-template-areas: "info_line points menu" "subject subject subject"
7370
align-items: center
7471
margin-top: calc(-1 * var(--base-size-4))
7572
margin-bottom: var(--base-size-4)
7673

77-
.op-backlogs-story--drag_handle_button
78-
padding: var(--base-size-4)
79-
8074
.op-backlogs-story--points
8175
margin-left: var(--stack-gap-normal)
8276
font-variant-numeric: tabular-nums
@@ -168,7 +162,7 @@ $op-backlogs-header--points-min-width-narrow: 2rem
168162
display: none
169163

170164
.op-backlogs-story
171-
grid-template-columns: var(--control-xsmall-size) 1fr minmax($op-backlogs-header--points-min-width-narrow, max-content) auto
165+
grid-template-columns: 1fr minmax($op-backlogs-header--points-min-width-narrow, max-content) auto
172166

173167
.op-backlogs-container, .op-sprint-planning-container
174168
height: unset

frontend/src/global_styles/primer/_overrides.sass

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,12 @@ ul.SegmentedControl,
156156
.ActionListItem-label[class^="__hl_"], .ActionListItem-label[class*=" __hl_"]
157157
color: var(--control-fgColor-disabled) !important
158158

159+
// hide hover when dragging with legacy Dragula implementation
160+
.Box-row--hover-gray,
161+
.Box-row--hover-blue
162+
body[data-dragging="active"] &
163+
background-color: unset
164+
159165
.Box-row--focus-gray
160166
&:focus-visible
161167
background-color: var(--bgColor-muted)
@@ -167,8 +173,25 @@ ul.SegmentedControl,
167173
.Box-row--clickable
168174
cursor: pointer
169175

170-
.Box-row:is(.Box-row--draggable)
171-
padding-left: 0
176+
.Box-row--draggable:not(.Box-row--clickable)
177+
cursor: grab
178+
179+
// `:is(.Box-row--clickable)` bumps selector specificity so these rules win
180+
// over Dragula's `.gu-*` transit styles; without it the mirror and source
181+
// rows render transparent mid-drag. Revisit once #74172 / #73729 replace the
182+
// legacy Dragula implementation.
183+
.Box-row:is(.Box-row--clickable)
184+
&[data-dragging="mirror"]
185+
background-color: var(--bgColor-default)
186+
border: var(--borderWidth-default) solid var(--borderColor-default)
187+
border-radius: var(--borderRadius-medium)
188+
box-shadow: var(--shadow-floating-medium)
189+
opacity: 1
190+
191+
&[data-dragging="source"]
192+
opacity: 0.4
193+
background-color: var(--bgColor-subtle)
194+
box-shadow: inset 0 0 0 var(--borderWidth-default) var(--borderColor-muted)
172195

173196
// Apply the mobile styles as soon as the banner itself is small
174197
// Styles are copied from the PVC repo

frontend/src/global_styles/vendor/_dragula.sass

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
body[data-dragging="active"] *
2+
cursor: grabbing !important
3+
14
.gu-mirror
25
position: fixed !important
36
margin: 0 !important
Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
/*
2+
* -- copyright
3+
* OpenProject is an open source project management software.
4+
* Copyright (C) the OpenProject GmbH
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the GNU General Public License version 3.
8+
*
9+
* OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
10+
* Copyright (C) 2006-2013 Jean-Philippe Lang
11+
* Copyright (C) 2010-2013 the ChiliProject Team
12+
*
13+
* This program is free software; you can redistribute it and/or
14+
* modify it under the terms of the GNU General Public License
15+
* as published by the Free Software Foundation; either version 2
16+
* of the License, or (at your option) any later version.
17+
*
18+
* This program is distributed in the hope that it will be useful,
19+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
20+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
21+
* GNU General Public License for more details.
22+
*
23+
* You should have received a copy of the GNU General Public License
24+
* along with this program; if not, write to the Free Software
25+
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
26+
*
27+
* See COPYRIGHT and LICENSE files for more details.
28+
* ++
29+
*/
30+
31+
import GenericDragAndDropController from './generic-drag-and-drop.controller';
32+
33+
describe('GenericDragAndDropController', () => {
34+
let controller:GenericDragAndDropController;
35+
36+
beforeEach(() => {
37+
controller = Object.create(GenericDragAndDropController.prototype) as GenericDragAndDropController;
38+
});
39+
40+
function setValue(name:'handleValue'|'handleSelectorValue', value:boolean|string) {
41+
Object.defineProperty(controller, name, { value, configurable: true });
42+
}
43+
44+
function draggableRow() {
45+
const row = document.createElement('li');
46+
row.className = 'Box-row Box-row--draggable';
47+
row.tabIndex = 0;
48+
row.dataset.draggableId = '42';
49+
row.dataset.draggableType = 'story';
50+
row.dataset.dropUrl = '/drop';
51+
return row;
52+
}
53+
54+
function callCanStartDrag(el:Element|null|undefined, handle:Element|null|undefined):boolean {
55+
const canStartDrag = Reflect.get(controller, 'canStartDrag') as (
56+
this:GenericDragAndDropController,
57+
el:Element|null|undefined,
58+
handle:Element|null|undefined
59+
) => boolean;
60+
61+
return canStartDrag.call(controller, el, handle);
62+
}
63+
64+
function callAriaPressedTarget(el:Element):Element|null {
65+
const ariaPressedTarget = Reflect.get(controller, 'ariaPressedTarget') as (
66+
this:GenericDragAndDropController,
67+
el:Element
68+
) => Element|null;
69+
70+
return ariaPressedTarget.call(controller, el);
71+
}
72+
73+
function callResolveMirrorContainer():Element {
74+
const resolveMirrorContainer = Reflect.get(controller, 'resolveMirrorContainer') as (
75+
this:GenericDragAndDropController
76+
) => Element;
77+
78+
return resolveMirrorContainer.call(controller);
79+
}
80+
81+
describe('canStartDrag', () => {
82+
it('allows dragging a draggable row in handle-less mode', () => {
83+
const row = draggableRow();
84+
85+
setValue('handleValue', false);
86+
setValue('handleSelectorValue', '.DragHandle');
87+
88+
expect(callCanStartDrag(row, row)).toBeTrue();
89+
});
90+
91+
it('rejects rows that are not draggable in handle-less mode', () => {
92+
const row = document.createElement('li');
93+
row.className = 'Box-row';
94+
row.tabIndex = 0;
95+
96+
setValue('handleValue', false);
97+
setValue('handleSelectorValue', '.DragHandle');
98+
99+
expect(callCanStartDrag(row, row)).toBeFalse();
100+
});
101+
102+
it('rejects empty placeholder rows in handle-less mode', () => {
103+
const row = draggableRow();
104+
row.dataset.emptyListItem = 'true';
105+
106+
setValue('handleValue', false);
107+
setValue('handleSelectorValue', '.DragHandle');
108+
109+
expect(callCanStartDrag(row, row)).toBeFalse();
110+
});
111+
112+
it('rejects interactive descendants in handle-less mode', () => {
113+
const row = draggableRow();
114+
const button = document.createElement('button');
115+
row.appendChild(button);
116+
117+
setValue('handleValue', false);
118+
setValue('handleSelectorValue', '.DragHandle');
119+
120+
expect(callCanStartDrag(row, button)).toBeFalse();
121+
});
122+
123+
it('allows drag handles in handle mode', () => {
124+
const row = draggableRow();
125+
const handle = document.createElement('button');
126+
handle.className = 'DragHandle';
127+
row.appendChild(handle);
128+
129+
setValue('handleValue', true);
130+
setValue('handleSelectorValue', '.DragHandle');
131+
132+
expect(callCanStartDrag(row, handle)).toBeTrue();
133+
});
134+
});
135+
136+
describe('ariaPressedTarget', () => {
137+
it('returns null in handle-less mode', () => {
138+
const row = draggableRow();
139+
140+
setValue('handleValue', false);
141+
setValue('handleSelectorValue', '.DragHandle');
142+
143+
expect(callAriaPressedTarget(row)).toBeNull();
144+
});
145+
146+
it('returns the handle element in handle mode', () => {
147+
const row = draggableRow();
148+
const handle = document.createElement('button');
149+
handle.className = 'DragHandle';
150+
row.appendChild(handle);
151+
152+
setValue('handleValue', true);
153+
setValue('handleSelectorValue', '.DragHandle');
154+
155+
expect(callAriaPressedTarget(row)).toBe(handle);
156+
});
157+
});
158+
159+
describe('resolveMirrorContainer', () => {
160+
it('returns the configured mirror container target when present', () => {
161+
const mirrorContainer = document.createElement('div');
162+
163+
Object.defineProperty(controller, 'hasMirrorContainerTarget', { value: true, configurable: true });
164+
Object.defineProperty(controller, 'mirrorContainerTarget', { value: mirrorContainer, configurable: true });
165+
166+
expect(callResolveMirrorContainer()).toBe(mirrorContainer);
167+
});
168+
169+
it('falls back to document.body when no mirror container target exists', () => {
170+
Object.defineProperty(controller, 'hasMirrorContainerTarget', { value: false, configurable: true });
171+
172+
expect(callResolveMirrorContainer()).toBe(document.body);
173+
});
174+
});
175+
});

frontend/src/stimulus/controllers/dynamic/generic-drag-and-drop.controller.ts

Lines changed: 49 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import { Controller } from '@hotwired/stimulus';
3232
import { FetchRequest } from '@rails/request.js';
3333
import { debugLog } from 'core-app/shared/helpers/debug_output';
34+
import { closestInteractiveElement } from 'core-stimulus/helpers/interactive-element-helper';
3435
import type { DomAutoscrollService } from 'core-app/shared/helpers/drag-and-drop/dom-autoscroll.service';
3536
import dragula, { Drake } from 'dragula';
3637
import invariant from 'tiny-invariant';
@@ -42,16 +43,20 @@ interface TargetConfig {
4243
}
4344

4445
export default class GenericDragAndDropController extends Controller {
45-
static targets = ['container', 'scrollContainer'];
46+
static targets = ['container', 'scrollContainer', 'mirrorContainer'];
4647

4748
containerTargets:HTMLElement[];
4849
scrollContainerTargets:HTMLElement[];
50+
declare readonly hasMirrorContainerTarget:boolean;
51+
declare readonly mirrorContainerTarget:HTMLElement;
4952

5053
static values = {
54+
handle: { type: Boolean, default: true },
5155
handleSelector: { type: String, default: '.DragHandle' },
5256
positionMode: { type: String, default: 'index' },
5357
};
5458

59+
declare readonly handleValue:boolean;
5560
declare readonly handleSelectorValue:string;
5661
declare readonly positionModeValue:string;
5762

@@ -69,6 +74,9 @@ export default class GenericDragAndDropController extends Controller {
6974
}
7075

7176
disconnect() {
77+
// A Turbo morph mid-drag can replace the element tree without the
78+
// dragend event firing, so clear the body-level cursor flag defensively.
79+
document.body.removeAttribute('data-dragging');
7280
this.autoscroll?.destroy();
7381
this.autoscroll = null;
7482
this.drake?.destroy();
@@ -118,22 +126,28 @@ export default class GenericDragAndDropController extends Controller {
118126
this.drake = dragula(
119127
this.containers,
120128
{
121-
moves: (_el, _source, handle, _sibling) => !!handle && !!handle.closest(this.handleSelectorValue),
129+
moves: (el, _source, handle, _sibling) => this.canStartDrag(el, handle),
122130
accepts: (el:Element, target:Element, source:Element, sibling:Element) => this.accepts(el, target, source, sibling),
123131
revertOnSpill: true, // enable reverting of elements if they are dropped outside of a valid target
132+
mirrorContainer: this.resolveMirrorContainer(),
124133
},
125134
)
135+
.on('cloned', (clone, _original, type) => {
136+
clone.setAttribute('data-dragging', type);
137+
})
126138
.on('drag', (el, source) => {
127139
this.dragOriginSource = source;
128140
this.dragOriginNextSibling = el.nextElementSibling;
129141

130-
const handle = el.querySelector(this.handleSelectorValue)!;
131-
handle.setAttribute('aria-pressed', 'true');
142+
el.setAttribute('data-dragging', 'source');
143+
document.body.setAttribute('data-dragging', 'active');
144+
this.ariaPressedTarget(el)?.setAttribute('aria-pressed', 'true');
132145
})
133146
.on('dragend', (el) => {
134-
const handle = el.querySelector(this.handleSelectorValue)!;
135-
handle.setAttribute('aria-pressed', 'false');
136-
})
147+
el.removeAttribute('data-dragging');
148+
document.body.removeAttribute('data-dragging');
149+
this.ariaPressedTarget(el)?.setAttribute('aria-pressed', 'false');
150+
})
137151
// eslint-disable-next-line @typescript-eslint/no-misused-promises
138152
.on('drop', this.drop.bind(this));
139153

@@ -215,6 +229,25 @@ export default class GenericDragAndDropController extends Controller {
215229
return data;
216230
}
217231

232+
private canStartDrag(el:Element|null|undefined, handle:Element|null|undefined):boolean {
233+
if (!this.isDraggableElement(el)) {
234+
return false;
235+
}
236+
237+
if (!this.handleValue) {
238+
return closestInteractiveElement(handle ?? null, el) == null;
239+
}
240+
241+
return handle?.closest(this.handleSelectorValue) != null;
242+
}
243+
244+
private isDraggableElement(el:Element|null|undefined):boolean {
245+
return el instanceof HTMLElement
246+
&& el.getAttribute('data-empty-list-item') !== 'true'
247+
&& el.dataset.draggableType !== undefined
248+
&& el.dataset.dropUrl !== undefined;
249+
}
250+
218251
// if the target has a container accessor, use that as the container instead of the element itself
219252
// we need this e.g. in Primer's borderbox component as we cannot add required data attributes to the ul element there
220253
private resolveContainerElement(target:HTMLElement):HTMLElement {
@@ -227,6 +260,10 @@ export default class GenericDragAndDropController extends Controller {
227260
return container;
228261
}
229262

263+
private resolveMirrorContainer():Element {
264+
return this.hasMirrorContainerTarget ? this.mirrorContainerTarget : document.body;
265+
}
266+
230267
// Returns the data-draggable-id of the element preceding el in its container,
231268
// or null if el is the first item (signals "move to top").
232269
private resolveTargetPrevious(el:Element):string|null {
@@ -245,4 +282,9 @@ export default class GenericDragAndDropController extends Controller {
245282

246283
return targetPosition + 1;
247284
}
285+
286+
private ariaPressedTarget(el:Element):Element|null {
287+
if (!this.handleValue) return null;
288+
return el.querySelector(this.handleSelectorValue);
289+
}
248290
}

0 commit comments

Comments
 (0)