Skip to content

Commit 4d04877

Browse files
committed
luci-base: fix click and drag handling
Previously when a table has the sortable property true, the whole row was draggable but without any useful effect on desktop or mobile. Only commencing the drag from the drag button worked as intended. This interfered with text selections or other actions in the table row. Now the drag and touch events are bound to the drag button only. The result is the same but the row contents are now selectable. This change works on both desktop and touch only devices like mobile. Signed-off-by: Paul Donald <newtwen+github@gmail.com>
1 parent 30341ae commit 4d04877

File tree

1 file changed

+45
-21
lines changed
  • modules/luci-base/htdocs/luci-static/resources

1 file changed

+45
-21
lines changed

modules/luci-base/htdocs/luci-static/resources/form.js

Lines changed: 45 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2615,15 +2615,11 @@ const CBITableSection = CBITypedSection.extend(/** @lends LuCI.form.TableSection
26152615
'id': 'cbi-%s-%s'.format(config_name, cfgsections[i]),
26162616
'class': 'tr cbi-section-table-row',
26172617
'data-sid': cfgsections[i],
2618-
'draggable': (drag_sort || touch_sort) ? true : null,
2619-
'mousedown': drag_sort ? L.bind(this.handleDragInit, this) : null,
2620-
'dragstart': drag_sort ? L.bind(this.handleDragStart, this) : null,
26212618
'dragover': drag_sort ? L.bind(this.handleDragOver, this) : null,
26222619
'dragenter': drag_sort ? L.bind(this.handleDragEnter, this) : null,
26232620
'dragleave': drag_sort ? L.bind(this.handleDragLeave, this) : null,
26242621
'dragend': drag_sort ? L.bind(this.handleDragEnd, this) : null,
26252622
'drop': drag_sort ? L.bind(this.handleDrop, this) : null,
2626-
'touchmove': touch_sort ? L.bind(this.handleTouchMove, this) : null,
26272623
'touchend': touch_sort ? L.bind(this.handleTouchEnd, this) : null,
26282624
'data-title': (sectionname && (!this.anonymous || this.sectiontitle)) ? sectionname : null,
26292625
'data-section-id': cfgsections[i]
@@ -2750,7 +2746,7 @@ const CBITableSection = CBITypedSection.extend(/** @lends LuCI.form.TableSection
27502746
},
27512747

27522748
/** @private */
2753-
renderRowActions(section_id, more_label) {
2749+
renderRowActions(section_id, more_label, trEl) {
27542750
const config_name = this.uciconfig ?? this.map.config;
27552751

27562752
if (!this.sortable && !this.extedit && !this.addremove && !more_label && !this.cloneable)
@@ -2761,14 +2757,27 @@ const CBITableSection = CBITypedSection.extend(/** @lends LuCI.form.TableSection
27612757
}, E('div'));
27622758

27632759
if (this.sortable) {
2764-
dom.append(tdEl.lastElementChild, [
2765-
E('button', {
2766-
'title': _('Drag to reorder'),
2767-
'class': 'cbi-button drag-handle center',
2768-
'style': 'cursor:move',
2769-
'disabled': this.map.readonly || null
2770-
}, '☰')
2771-
]);
2760+
const touch_sort = ('ontouchstart' in window);
2761+
const dragHandleProps = {
2762+
'title': _('Drag to reorder'),
2763+
'class': 'cbi-button drag-handle center',
2764+
'style': 'cursor:move; user-select:none; -webkit-user-select:none; display:inline-block;',
2765+
'draggable': !touch_sort,
2766+
'dragstart': !touch_sort ? L.bind(function(ev) {
2767+
this.handleDragStart(ev, trEl);
2768+
}, this) : null,
2769+
'dragend': !touch_sort ? L.bind(function(ev) {
2770+
this.handleDragEnd(ev, trEl);
2771+
}, this) : null,
2772+
'touchmove': touch_sort ? L.bind(function(ev) {
2773+
this.handleTouchMove(ev);
2774+
}, this) : null,
2775+
'touchend': touch_sort ? L.bind(function(ev) {
2776+
this.handleTouchEnd(ev);
2777+
}, this) : null
2778+
};
2779+
const dragHandle = E('div', dragHandleProps, '☰');
2780+
dom.append(tdEl.lastElementChild, [ dragHandle ]);
27722781
}
27732782

27742783
if (this.extedit) {
@@ -2835,13 +2844,15 @@ const CBITableSection = CBITypedSection.extend(/** @lends LuCI.form.TableSection
28352844
},
28362845

28372846
/** @private */
2838-
handleDragStart(ev) {
2839-
if (!scope.dragState?.node.classList.contains('drag-handle')) {
2847+
handleDragStart(ev, trEl) {
2848+
// Only allow drag from the handle
2849+
if (!ev.target || !ev.target.classList || !ev.target.classList.contains('drag-handle')) {
28402850
scope.dragState = null;
28412851
return false;
28422852
}
2843-
2844-
scope.dragState.node = dom.parent(scope.dragState.node, '.tr');
2853+
// Set the row as the drag source
2854+
scope.dragState = scope.dragState || {};
2855+
scope.dragState.node = trEl || dom.parent(ev.target, '.tr');
28452856
ev.dataTransfer.setData('text', 'drag');
28462857
ev.target.style.opacity = 0.4;
28472858
},
@@ -2881,9 +2892,19 @@ const CBITableSection = CBITypedSection.extend(/** @lends LuCI.form.TableSection
28812892
},
28822893

28832894
/** @private */
2884-
handleDragEnd(ev) {
2885-
const n = ev.target;
2886-
2895+
handleDragEnd(ev, trEl) {
2896+
let n;
2897+
if (trEl) {
2898+
n = trEl;
2899+
} else if (ev.target && typeof ev.target.closest === 'function') {
2900+
n = ev.target.closest('tr');
2901+
} else {
2902+
// Fall-back: skip if no valid row
2903+
return;
2904+
}
2905+
if (!n) return;
2906+
// Reset drag handle visual state
2907+
n.querySelector('.drag-handle').style.opacity = '';
28872908
n.style.opacity = '';
28882909
n.classList.add('flash');
28892910
n.parentNode.querySelectorAll('.drag-over-above, .drag-over-below')
@@ -3014,7 +3035,7 @@ const CBITableSection = CBITypedSection.extend(/** @lends LuCI.form.TableSection
30143035

30153036
dragHandle.style.top = `${touchLoc.pageY - (parseInt(dragHandle.style.height) / 2)}px`;
30163037

3017-
rowElem.parentNode.querySelectorAll('[draggable]').forEach((tr, i, trs) => {
3038+
rowElem.parentNode.querySelectorAll('.cbi-section-table-row').forEach((tr, i, trs) => {
30183039
const trRect = tr.getBoundingClientRect();
30193040
const yTop = trRect.top + window.scrollY;
30203041
const yBottom = trRect.bottom + window.scrollY;
@@ -3050,6 +3071,9 @@ const CBITableSection = CBITypedSection.extend(/** @lends LuCI.form.TableSection
30503071
if (!dragHandle)
30513072
return;
30523073

3074+
// Reset drag handle visual state
3075+
dragHandle.style.opacity = '';
3076+
30533077
if (targetElem) {
30543078
const isBelow = targetElem.classList.contains('drag-over-below');
30553079

0 commit comments

Comments
 (0)