Skip to content

Drag and drop LED output reordering #5001

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
+131 −79
Open

Drag and drop LED output reordering #5001

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
00fab2e
Drag and drop LED output reordering
blazoncek Oct 13, 2025
a18b133
Bugfix
blazoncek Oct 20, 2025
a03f758
Add ability to edit LED type on any output
blazoncek Nov 2, 2025
7f8977f
Remove memory calculation, add comment
blazoncek Nov 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 49 additions & 11 deletions wled00/data/settings_leds.htm
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<!DOCTYPE html>
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
Expand Down Expand Up @@ -131,7 +131,7 @@
if (!en) {
// limiter disabled
d.Sf.PPL.checked = false;
// d.Sf.querySelectorAll("#mLC select[name^=LAsel]").forEach((e)=>{e.selectedIndex = 0;}); // select default LED mA
// d.Sf.querySelectorAll("#mLC select[name^=LL]").forEach((e)=>{e.selectedIndex = 0;}); // select default LED mA
// d.Sf.querySelectorAll("#mLC input[name^=LA]").forEach((e)=>{e.min = 0; e.value = 0;}); // set min & value to 0
Comment on lines +134 to 135
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Remove commented-out code.

Dead code should be removed to improve maintainability. If this functionality might be needed later, document the reason in a comment or rely on version control history.

Apply this diff:

 		if (!en) {
 			// limiter disabled
 			d.Sf.PPL.checked = false;
-//			d.Sf.querySelectorAll("#mLC select[name^=LL]").forEach((e)=>{e.selectedIndex = 0;}); // select default LED mA
-//			d.Sf.querySelectorAll("#mLC input[name^=LA]").forEach((e)=>{e.min = 0; e.value = 0;}); // set min & value to 0
 		}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// d.Sf.querySelectorAll("#mLC select[name^=LL]").forEach((e)=>{e.selectedIndex = 0;}); // select default LED mA
// d.Sf.querySelectorAll("#mLC input[name^=LA]").forEach((e)=>{e.min = 0; e.value = 0;}); // set min & value to 0
if (!en) {
// limiter disabled
d.Sf.PPL.checked = false;
}
🤖 Prompt for AI Agents 
In wled00/data/settings_leds.htm around lines 135 to 136, there are two
commented-out JavaScript lines that should be removed as dead code; delete those
two commented lines entirely, and if preserving intent is required add a short
one-line comment explaining why functionality was removed or rely on VCS history
instead of keeping commented code.

}
UI();
Expand Down Expand Up @@ -160,9 +160,10 @@
if (ppl) d.Sf.MA.value = sumMA; // populate UI ABL value if PPL used
}
// enable and update LED Amps
function enLA(s,n)
function enLA(s)
{
const abl = d.Sf.ABL.checked;
const n = s.name.substring(2); // bus number (0-Z)
const t = parseInt(d.Sf["LT"+n].value); // LED type SELECT
gId('LAdis'+n).style.display = s.selectedIndex==5 ? "inline" : "none"; // show/hide custom mA field
if (s.value!=="0") d.Sf["LA"+n].value = s.value; // set value from select object
Expand All @@ -177,9 +178,9 @@
});
d.Sf.ABL.checked = en;
// select appropriate LED current
d.Sf.querySelectorAll("#mLC select[name^=LAsel]").forEach((sel,x)=>{
d.Sf.querySelectorAll("#mLC select[name^=LL]").forEach((sel)=>{
sel.value = 0; // set custom
var n = chrID(x);
var n = sel.name.substring(2); // bus number (0-Z)
if (en)
switch (parseInt(d.Sf["LA"+n].value)) {
case 0: break; // disable ABL
Expand All @@ -190,7 +191,7 @@
case 255: sel.value = 255; break;
}
else sel.value = 0;
enLA(sel,n); // configure individual limiter
enLA(sel); // configure individual limiter
});
enABL();
gId('m1').innerHTML = maxM;
Expand Down Expand Up @@ -464,12 +465,12 @@

if (n==1) {
// npm run build has trouble minimizing spaces inside string
var cn = `<div class="iST">
var cn = `<div class="iST" draggable="true" ondragstart="hDS(event)" id="l${s}" style="cursor:grab;">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add keyboard navigation and ARIA attributes for accessibility.

The drag-and-drop implementation lacks keyboard navigation support and ARIA attributes, preventing users who rely on keyboards or screen readers from reordering LED outputs. This violates WCAG 2.1 accessibility guidelines.

Consider adding:

  1. Keyboard shortcuts (e.g., Ctrl+Up/Down arrows to reorder)
  2. Visible reorder buttons (↑/↓) as an alternative
  3. ARIA attributes: role="list", role="listitem", aria-label="Drag to reorder", aria-grabbed, aria-dropeffect
  4. Focus management after reordering

Example ARIA enhancement:

-<div class="iST" draggable="true" ondragstart="hDS(event)" id="l${s}" style="cursor:grab;">
+<div class="iST" draggable="true" ondragstart="hDS(event)" id="l${s}" style="cursor:grab;" role="listitem" aria-label="LED output ${i+1}, drag to reorder" tabindex="0">

Also applies to: 621-637

🤖 Prompt for AI Agents 
In wled00/data/settings_leds.htm around line 468 (and similarly 621-637), the
draggable LED item lacks keyboard navigation and ARIA attributes; add
accessibility by marking the container as role="list" and each item as
role="listitem" with aria-label="Drag to reorder", aria-grabbed (true/false) and
aria-dropeffect where appropriate, add tabindex="0" to make items focusable,
implement keyboard handlers (e.g., Ctrl+ArrowUp/Ctrl+ArrowDown or Home/End) to
move items programmatically and update aria-grabbed/dropeffect states, provide
visible ↑/↓ reorder buttons as alternatives that also update the underlying
order and focus, and ensure focus is moved to the item (or its button) after
reordering so screen reader users remain oriented.

<hr class="sml">
${i+1}:
<span id="n${s}">${i+1}</span>:
<select name="LT${s}" onchange="UI(true)"></select><br>
<div id="abl${s}">
mA/LED: <select name="LAsel${s}" onchange="enLA(this,'${s}');UI();">
mA/LED: <select name="LL${s}" onchange="enLA(this);UI();">
<option value="55" selected>55mA (typ. 5V WS281x)</option>
<option value="35">35mA (eco WS2812)</option>
<option value="30">30mA (typ. 12V)</option>
Expand Down Expand Up @@ -521,7 +522,7 @@
}
}
});
enLA(d.Sf["LAsel"+s],s); // update LED mA
enLA(gN("LL"+s)); // update LED mA
// disable inappropriate LED types
let sel = d.getElementsByName("LT"+s)[0];
// 32 & S2 supports mono I2S as well as parallel so we need to take that into account; S3 only supports parallel
Expand Down Expand Up @@ -617,6 +618,43 @@
c += `<span style="cursor: pointer;" onclick="off('BT${s}')">&nbsp;&#x2715;</span><br>`;
gId("btns").innerHTML = c;
}
function hDS(e) {
e.dataTransfer.setData('text', e.currentTarget.id);
}
function hDO(e) {
e.preventDefault();
}
function hDrop(e) {
e.preventDefault();
let t = e.target;
if (t.id === "mLC") t = t.children[0]; // dropped on a container, not on an element
else while (t && !t.classList.contains("iST")) t = t.parentNode; // find target element
if (!t || t.id === "") return false; // not dropping on a valid target
const id = e.dataTransfer.getData("text");
t.parentNode.insertBefore(gId(id), t);
recalcIds();
UI();
}
Comment on lines +605 to +621
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Minor: Handle edge case when dropping on empty container.

Line 630 assumes t.children[0] exists when dropping on the container. While unlikely in practice (there's always at least one LED output), this could cause a runtime error if the container is empty.

Consider adding a guard:

 	let t = e.target;
-	if (t.id === "mLC") t = t.children[0]; // dropped on a container, not on an element
+	if (t.id === "mLC") {
+		if (!t.children[0]) return; // empty container
+		t = t.children[0];
+	}
 	else while (t && !t.classList.contains("iST")) t = t.parentNode; // find target element
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function hDS(e) {
e.dataTransfer.setData('text', e.currentTarget.id);
}
function hDO(e) {
e.preventDefault();
}
function hDrop(e) {
e.preventDefault();
let t = e.target;
if (t.id === "mLC") t = t.children[0]; // dropped on a container, not on an element
else while (t && !t.classList.contains("iST")) t = t.parentNode; // find target element
if (!t || t.id === "") return false; // not dropping on a valid target
const id = e.dataTransfer.getData("text");
t.parentNode.insertBefore(gId(id), t);
recalcIds();
UI();
}
function hDrop(e) {
e.preventDefault();
let t = e.target;
if (t.id === "mLC") {
if (!t.children[0]) return; // empty container
t = t.children[0];
}
else while (t && !t.classList.contains("iST")) t = t.parentNode; // find target element
if (!t || t.id === "") return false; // not dropping on a valid target
const id = e.dataTransfer.getData("text");
t.parentNode.insertBefore(gId(id), t);
recalcIds();
UI();
}
🤖 Prompt for AI Agents 
In wled00/data/settings_leds.htm around lines 621 to 637, the drop handler
assumes t.children[0] exists when the drop target is the container ("mLC"); add
a guard so you don't access children[0] on an empty container: if the container
has no children, treat the container itself as the insertion point (or append
the dragged element to the container) instead of dereferencing children[0]; keep
the rest of the logic (recalcIds and UI) unchanged.

function recalcIds() {
gId("mLC").querySelectorAll(".iST").forEach((e,i)=>{
let sOld = e.id.substring(1);
let sNew = chrID(i);
// update all element IDs and names
e.id = "l"+sNew;
e.querySelector("#n"+sOld).innerText = (i+1);
// names: LT,LL,LA,MA,CO,WO,SP,LS,LC,L0,L1,L2,L3,L4,HS,CV,SL,RF,AW
["LT","LL","LA","MA","CO","WO","SP","LS","LC","L0","L1","L2","L3","L4","HS","CV","SL","RF","AW"].forEach((n)=>{
let el = e.querySelector("[name^="+n+"]");
if (el) el.name = n + sNew;
});
// IDs: l,n,abl,LAdis,PSU,co,ls,dig?w,dig?l,psd,dig?c,p0d,p1d,p2d,p3d,p4d,net?h,dig?r,dig?s,dig?f,dig?a
["l","n","abl","LAdis","PSU","co","ls","dig?w","dig?l","psd","dig?c","p0d","p1d","p2d","p3d","p4d","net?h","dig?r","dig?s","dig?f","dig?a"].forEach((n)=>{
if (n.indexOf("?") < 0) n += "?";
let el = e.querySelector("#"+n.replace("?", sOld));
if (el) el.id = n.replace("?", sNew);
});
});
}
Comment on lines +622 to +641
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Update startsDirty array after reordering.

The recalcIds function renumbers all LED blocks but doesn't update the startsDirty array, which tracks custom start indices by bus index. After drag-and-drop reordering, the array indices no longer match the reordered buses, potentially causing incorrect custom start index behavior.

Apply this fix to update startsDirty when reordering:

 		function recalcIds() {
+			let newStartsDirty = [];
 			gId("mLC").querySelectorAll(".iST").forEach((e,i)=>{
 				let sOld = e.id.substring(1);
 				let sNew = chrID(i);
+				let oldIndex = toNum(sOld);
+				newStartsDirty[i] = startsDirty[oldIndex] || false;
 				// update all element IDs and names
 				e.id = "l"+sNew;
 				e.querySelector("#n"+sOld).innerText = (i+1);
 				// names: LT,LL,LA,MA,CO,WO,SP,LS,LC,L0,L1,L2,L3,L4,HS,CV,SL,RF,AW
 				["LT","LL","LA","MA","CO","WO","SP","LS","LC","L0","L1","L2","L3","L4","HS","CV","SL","RF","AW"].forEach((n)=>{
 					let el = e.querySelector("[name^="+n+"]");
 					if (el) el.name = n + sNew;
 				});
 				// IDs: l,n,abl,LAdis,PSU,co,ls,dig?w,dig?l,psd,dig?c,p0d,p1d,p2d,p3d,p4d,net?h,dig?r,dig?s,dig?f,dig?a
 				["l","n","abl","LAdis","PSU","co","ls","dig?w","dig?l","psd","dig?c","p0d","p1d","p2d","p3d","p4d","net?h","dig?r","dig?s","dig?f","dig?a"].forEach((n)=>{
 					if (n.indexOf("?") < 0) n += "?";
 					let el = e.querySelector("#"+n.replace("?", sOld));
 					if (el) el.id = n.replace("?", sNew);
 				});
 			});
+			startsDirty = newStartsDirty;
 		}

Committable suggestion skipped: line range outside the PR's diff.

function tglSi(cs) {
customStarts = cs;
if (!customStarts) startsDirty = []; //set all starts to clean
Expand Down Expand Up @@ -847,7 +885,7 @@ <h2>LED &amp; Hardware setup</h2>
</div>
</div>
<h3>Hardware setup</h3>
<div id="mLC">LED outputs:</div>
<div id="mLC" ondragover="hDO(event)" ondrop="hDrop(event)">LED outputs:</div>
<hr class="sml">
<button type="button" id="+" onclick="addLEDs(1,false)">+</button>
<button type="button" id="-" onclick="addLEDs(-1,false)">-</button><br>
Expand Down