Skip to content

Commit 7402cb3

Browse files
committed
[#73473] Make entire WP card draggable
Uses existing Dragula-based `generic-drag-and-drop.controller`, adding support for a `handle` Boolean value (rather than relying on an empty `handle-selector` value, since empty attribute values can be ambiguous in HTML).
1 parent 10f2ed7 commit 7402cb3

14 files changed

Lines changed: 349 additions & 97 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: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ ul.SegmentedControl,
168168
cursor: pointer
169169

170170
.Box-row:is(.Box-row--draggable)
171-
padding-left: 0
171+
cursor: grab
172172

173173
// Apply the mobile styles as soon as the banner itself is small
174174
// Styles are copied from the PVC repo
Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
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+
describe('canStartDrag', () => {
74+
it('allows dragging a draggable row in handle-less mode', () => {
75+
const row = draggableRow();
76+
77+
setValue('handleValue', false);
78+
setValue('handleSelectorValue', '.DragHandle');
79+
80+
expect(callCanStartDrag(row, row)).toBeTrue();
81+
});
82+
83+
it('rejects rows that are not draggable in handle-less mode', () => {
84+
const row = document.createElement('li');
85+
row.className = 'Box-row';
86+
row.tabIndex = 0;
87+
88+
setValue('handleValue', false);
89+
setValue('handleSelectorValue', '.DragHandle');
90+
91+
expect(callCanStartDrag(row, row)).toBeFalse();
92+
});
93+
94+
it('rejects empty placeholder rows in handle-less mode', () => {
95+
const row = draggableRow();
96+
row.dataset.emptyListItem = 'true';
97+
98+
setValue('handleValue', false);
99+
setValue('handleSelectorValue', '.DragHandle');
100+
101+
expect(callCanStartDrag(row, row)).toBeFalse();
102+
});
103+
104+
it('rejects interactive descendants in handle-less mode', () => {
105+
const row = draggableRow();
106+
const button = document.createElement('button');
107+
row.appendChild(button);
108+
109+
setValue('handleValue', false);
110+
setValue('handleSelectorValue', '.DragHandle');
111+
112+
expect(callCanStartDrag(row, button)).toBeFalse();
113+
});
114+
115+
it('allows drag handles in handle mode', () => {
116+
const row = draggableRow();
117+
const handle = document.createElement('button');
118+
handle.className = 'DragHandle';
119+
row.appendChild(handle);
120+
121+
setValue('handleValue', true);
122+
setValue('handleSelectorValue', '.DragHandle');
123+
124+
expect(callCanStartDrag(row, handle)).toBeTrue();
125+
});
126+
});
127+
128+
describe('ariaPressedTarget', () => {
129+
it('returns null in handle-less mode', () => {
130+
const row = draggableRow();
131+
132+
setValue('handleValue', false);
133+
setValue('handleSelectorValue', '.DragHandle');
134+
135+
expect(callAriaPressedTarget(row)).toBeNull();
136+
});
137+
138+
it('returns the handle element in handle mode', () => {
139+
const row = draggableRow();
140+
const handle = document.createElement('button');
141+
handle.className = 'DragHandle';
142+
row.appendChild(handle);
143+
144+
setValue('handleValue', true);
145+
setValue('handleSelectorValue', '.DragHandle');
146+
147+
expect(callAriaPressedTarget(row)).toBe(handle);
148+
});
149+
});
150+
});

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

Lines changed: 50 additions & 6 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 { isInteractiveElement } 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';
@@ -48,10 +49,12 @@ export default class GenericDragAndDropController extends Controller {
4849
scrollContainerTargets:HTMLElement[];
4950

5051
static values = {
52+
handle: { type: Boolean, default: true },
5153
handleSelector: { type: String, default: '.DragHandle' },
5254
positionMode: { type: String, default: 'index' },
5355
};
5456

57+
declare readonly handleValue:boolean;
5558
declare readonly handleSelectorValue:string;
5659
declare readonly positionModeValue:string;
5760

@@ -118,7 +121,7 @@ export default class GenericDragAndDropController extends Controller {
118121
this.drake = dragula(
119122
this.containers,
120123
{
121-
moves: (_el, _source, handle, _sibling) => !!handle && !!handle.closest(this.handleSelectorValue),
124+
moves: (el, _source, handle, _sibling) => this.canStartDrag(el, handle),
122125
accepts: (el:Element, target:Element, source:Element, sibling:Element) => this.accepts(el, target, source, sibling),
123126
revertOnSpill: true, // enable reverting of elements if they are dropped outside of a valid target
124127
},
@@ -127,13 +130,11 @@ export default class GenericDragAndDropController extends Controller {
127130
this.dragOriginSource = source;
128131
this.dragOriginNextSibling = el.nextElementSibling;
129132

130-
const handle = el.querySelector(this.handleSelectorValue)!;
131-
handle.setAttribute('aria-pressed', 'true');
133+
this.ariaPressedTarget(el)?.setAttribute('aria-pressed', 'true');
132134
})
133135
.on('dragend', (el) => {
134-
const handle = el.querySelector(this.handleSelectorValue)!;
135-
handle.setAttribute('aria-pressed', 'false');
136-
})
136+
this.ariaPressedTarget(el)?.setAttribute('aria-pressed', 'false');
137+
})
137138
// eslint-disable-next-line @typescript-eslint/no-misused-promises
138139
.on('drop', this.drop.bind(this));
139140

@@ -215,6 +216,44 @@ export default class GenericDragAndDropController extends Controller {
215216
return data;
216217
}
217218

219+
private canStartDrag(el:Element|null|undefined, handle:Element|null|undefined):boolean {
220+
if (!this.isDraggableElement(el)) {
221+
return false;
222+
}
223+
224+
if (!this.handleValue) {
225+
return !this.hasInteractiveDragStart(el, handle);
226+
}
227+
228+
return handle?.closest(this.handleSelectorValue) != null;
229+
}
230+
231+
private hasInteractiveDragStart(el:Element|null|undefined, handle:Element|null|undefined):boolean {
232+
if (!el) {
233+
return false;
234+
}
235+
236+
let current = handle;
237+
238+
while (current && current !== el) {
239+
if (isInteractiveElement(current)) {
240+
return true;
241+
}
242+
243+
current = current.parentElement;
244+
}
245+
246+
return false;
247+
}
248+
249+
private isDraggableElement(el:Element|null|undefined):boolean {
250+
return el instanceof HTMLElement
251+
&& el.getAttribute('data-empty-list-item') !== 'true'
252+
&& el.dataset.draggableId !== undefined
253+
&& el.dataset.draggableType !== undefined
254+
&& el.dataset.dropUrl !== undefined;
255+
}
256+
218257
// if the target has a container accessor, use that as the container instead of the element itself
219258
// we need this e.g. in Primer's borderbox component as we cannot add required data attributes to the ul element there
220259
private resolveContainerElement(target:HTMLElement):HTMLElement {
@@ -245,4 +284,9 @@ export default class GenericDragAndDropController extends Controller {
245284

246285
return targetPosition + 1;
247286
}
287+
288+
private ariaPressedTarget(el:Element):Element|null {
289+
if (!this.handleValue) return null;
290+
return el.querySelector(this.handleSelectorValue);
291+
}
248292
}
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
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+
const ariaInteractiveRoles = new Set([
32+
'button',
33+
'checkbox',
34+
'combobox',
35+
'link',
36+
'listbox',
37+
'menuitem',
38+
'menuitemcheckbox',
39+
'menuitemradio',
40+
'option',
41+
'radio',
42+
'slider',
43+
'spinbutton',
44+
'switch',
45+
'tab',
46+
'textbox',
47+
'treeitem',
48+
]);
49+
50+
export function isInteractiveElement(el:Element|null):el is HTMLElement {
51+
if (!(el instanceof HTMLElement)) return false;
52+
if (el.hasAttribute('disabled')) return false;
53+
if (el.getAttribute('aria-disabled') === 'true') return false;
54+
if (el.hidden) return false;
55+
56+
const tag = el.tagName.toLowerCase();
57+
const role = el.getAttribute('role');
58+
const tabIndex = el.tabIndex;
59+
60+
const nativeInteractive = tag === 'button'
61+
|| tag === 'select'
62+
|| tag === 'textarea'
63+
|| tag === 'summary'
64+
|| (tag === 'input' && (el as HTMLInputElement).type !== 'hidden')
65+
|| (tag === 'a' && el.hasAttribute('href'))
66+
|| (tag === 'audio' && el.hasAttribute('controls'))
67+
|| (tag === 'video' && el.hasAttribute('controls'));
68+
69+
return nativeInteractive
70+
|| (role != null && ariaInteractiveRoles.has(role))
71+
|| el.isContentEditable
72+
|| tabIndex >= 0;
73+
}

modules/backlogs/app/components/backlogs/inbox_item_component.html.erb

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,16 +29,6 @@ See COPYRIGHT and LICENSE files for more details.
2929

3030
<% container.with_row(**row_options) do %>
3131
<%= grid_layout("op-backlogs-story", tag: :article) do |grid| %>
32-
<% grid.with_area(:drag_handle, classes: "hide-when-print") do %>
33-
<%= if draggable?
34-
render(
35-
Primer::OpenProject::DragHandle.new(
36-
classes: "op-backlogs-story--drag_handle_button",
37-
label: t(".label_drag_work_package", name: work_package.subject)
38-
)
39-
)
40-
end %>
41-
<% end %>
4232
<% grid.with_area(:info_line) do %>
4333
<%= render(WorkPackages::InfoLineComponent.new(work_package:)) %>
4434
<% end %>

0 commit comments

Comments
 (0)