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 2 commits into
base: main
Choose a base branch
from
Open

Drag and drop LED output reordering #5001

Changes from all commits
Commits
Show all changes
2 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
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
62 changes: 50 additions & 12 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>
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

Remove the UTF-8 BOM character.

The file starts with a UTF-8 Byte Order Mark (). While technically valid, BOM characters can cause issues with some parsers, web servers, and build tools. It's generally recommended to save HTML files as UTF-8 without BOM.

🤖 Prompt for AI Agents 
In wled00/data/settings_leds.htm around line 1, the file begins with a UTF-8 BOM
character which can break some parsers and tools; remove the BOM by deleting the
leading invisible character (or re-save the file as "UTF-8 without BOM" in your
editor/IDE) so the file starts with "<!DOCTYPE html>" and commit the change.

<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
}
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 @@ -270,7 +271,7 @@
let dC = 0; // count of digital buses (for parallel I2S)
let LTs = d.Sf.querySelectorAll("#mLC select[name^=LT]");
LTs.forEach((s,i)=>{
if (i < LTs.length-1) s.disabled = true; // prevent changing type (as we can't update options)
s.disabled = (i < LTs.length-1); // prevent changing type (as we can't update options)
// is the field a LED type?
var n = s.name.substring(2,3); // bus number (0-Z)
var t = parseInt(s.value);
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 +621 to +637
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);
});
});
}
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