From 289d308b5700426763d6e84bb1a2e58a569de32b Mon Sep 17 00:00:00 2001 From: Thomas Faour Date: Fri, 24 Jul 2026 18:00:01 -0400 Subject: [PATCH] Disambiguate same-type widgets in the button-assignment dropdowns Two widgets of the same type (e.g. two Photos widgets) both showed up as plain "Photos" with no way to tell which was which. The buttons API now includes each widget's grid placement plus the frame's grid dims; the UI derives a rough position ("top-left", "right", etc.) from that and only appends a number+position suffix when a type actually repeats on the frame, leaving the common single-widget-per-type case unchanged. --- server/app/routers/api_frames.py | 13 +++++-- server/app/static/frame_config.js | 55 ++++++++++++++++++++++++----- server/tests/test_button_actions.py | 29 +++++++++++++++ 3 files changed, 87 insertions(+), 10 deletions(-) diff --git a/server/app/routers/api_frames.py b/server/app/routers/api_frames.py index f38db01..6800e1d 100644 --- a/server/app/routers/api_frames.py +++ b/server/app/routers/api_frames.py @@ -259,7 +259,15 @@ def api_buttons_get(frame: Frame = Depends(require_frame_view), db: Session = De """Everything the button-assignment UI needs in one call: every widget on the frame with the actions its type supports (see app/widgets/*.py's ACTIONS/ACTION_LABELS), plus each button's current - ordered list of (widget, action) bindings.""" + ordered list of (widget, action) bindings. + + Includes each widget's placement (x/y/w/h) and the frame's grid + dimensions -- two widgets of the same type otherwise look identical + in the assignment UI's dropdowns (both just say "Photos"); the + client derives a position label ("top-left" etc.) from this to tell + them apart, the same way you'd tell them apart by eye on the Layout + canvas.""" + cols, rows = grid.grid_dims(frame.orientation) widgets = db.scalars( select(Widget).where(Widget.frame_id == frame.id).order_by(Widget.sort_order) ).all() @@ -267,6 +275,7 @@ def api_buttons_get(frame: Frame = Depends(require_frame_view), db: Session = De { "id": w.id, "widget_type": w.widget_type, + "x": w.x, "y": w.y, "w": w.w, "h": w.h, "actions": [ {"action": action, "label": label} for action, label in getattr(WIDGET_TYPES.get(w.widget_type), "ACTION_LABELS", {}).items() @@ -274,7 +283,7 @@ def api_buttons_get(frame: Frame = Depends(require_frame_view), db: Session = De } for w in widgets ] - result = {"widgets": widget_options} + result = {"widgets": widget_options, "grid": {"cols": cols, "rows": rows}} for button in BUTTONS: rows = db.scalars( select(FrameButtonAction) diff --git a/server/app/static/frame_config.js b/server/app/static/frame_config.js index 3a8804a..27f6c18 100644 --- a/server/app/static/frame_config.js +++ b/server/app/static/frame_config.js @@ -364,21 +364,59 @@ loadFirmwareCheck(); setInterval(loadFirmwareCheck, 60000); // --- Button assignments ----------------------------------------------- -// {widgets: [{id, widget_type, actions: [{action, label}]}], next: [...], back: [...]} -// -- see api_frames.py's api_buttons_get. Each button's list is edited -// client-side (add/remove/reorder) then PUT as a whole -- simpler than -// separate reorder/add/remove endpoints for what's normally a handful of -// entries, and this file already has the full list in hand after any -// edit. +// {widgets: [{id, widget_type, x, y, w, h, actions: [{action, label}]}], +// grid: {cols, rows}, next: [...], back: [...]} -- see api_frames.py's +// api_buttons_get. Each button's list is edited client-side (add/ +// remove/reorder) then PUT as a whole -- simpler than separate reorder/ +// add/remove endpoints for what's normally a handful of entries, and +// this file already has the full list in hand after any edit. let buttonsData = null; +let widgetNames = {}; // widget id -> disambiguated display name, see buildWidgetNames const BUTTONS = ['next', 'back']; +// "top-left"/"bottom"/"center" etc. from a widget's grid rect vs the +// frame's grid dims -- the same rough position you'd read off the +// Layout canvas by eye, used to tell apart two widgets of the same type +// that would otherwise both just say "Photos". +function widgetPositionLabel(w, grid) { + const cx = w.x + w.w / 2; + const cy = w.y + w.h / 2; + const horiz = cx < grid.cols / 2 ? 'left' : (cx > grid.cols / 2 ? 'right' : ''); + const vert = cy < grid.rows / 2 ? 'top' : (cy > grid.rows / 2 ? 'bottom' : ''); + if (!horiz && !vert) return 'center'; + if (!vert) return horiz; + if (!horiz) return vert; + return `${vert}-${horiz}`; +} + +// A single widget of a given type keeps the plain type name ("Photos") +// -- the common case, no need to clutter it. Only widgets sharing a +// type with another widget on the same frame get a number + position +// suffix, numbered in reading order (top-to-bottom, left-to-right). +function buildWidgetNames(widgets, grid) { + const byType = {}; + widgets.forEach((w) => { (byType[w.widget_type] = byType[w.widget_type] || []).push(w); }); + const names = {}; + Object.values(byType).forEach((group) => { + if (group.length === 1) { + names[group[0].id] = WIDGET_LABELS[group[0].widget_type] || group[0].widget_type; + return; + } + const ordered = [...group].sort((a, b) => (a.y - b.y) || (a.x - b.x)); + ordered.forEach((w, i) => { + const base = WIDGET_LABELS[w.widget_type] || w.widget_type; + names[w.id] = `${base} ${i + 1} (${widgetPositionLabel(w, grid)})`; + }); + }); + return names; +} + function widgetActionLabel(widgetId, action) { const w = buttonsData.widgets.find((w) => w.id === widgetId); if (!w) return `(deleted widget): ${action}`; const found = w.actions.find((a) => a.action === action); const actionLabel = found ? found.label : action; - return `${WIDGET_LABELS[w.widget_type] || w.widget_type}: ${actionLabel}`; + return `${widgetNames[widgetId] || WIDGET_LABELS[w.widget_type] || w.widget_type}: ${actionLabel}`; } function renderButtonList(button) { @@ -451,7 +489,7 @@ function populateWidgetSelect(button) { buttonsData.widgets.forEach((w) => { const opt = document.createElement('option'); opt.value = w.id; - opt.textContent = WIDGET_LABELS[w.widget_type] || w.widget_type; + opt.textContent = widgetNames[w.id] || WIDGET_LABELS[w.widget_type] || w.widget_type; widgetSel.appendChild(opt); }); populateActionSelect(button); @@ -503,6 +541,7 @@ async function loadButtons() { const resp = await fetch(`${window.FRAME_BASE_API}/buttons`); if (!resp.ok) throw new Error(await apiError(resp)); buttonsData = await resp.json(); + widgetNames = buildWidgetNames(buttonsData.widgets, buttonsData.grid); document.getElementById('button-assign-groups').style.display = buttonsData.widgets.length ? '' : 'none'; document.getElementById('button-assign-empty-hint').style.display = diff --git a/server/tests/test_button_actions.py b/server/tests/test_button_actions.py index 309fabd..77a1795 100644 --- a/server/tests/test_button_actions.py +++ b/server/tests/test_button_actions.py @@ -46,6 +46,35 @@ def test_get_buttons_reflects_the_default_migration_mapping(client, db_session): assert data["back"] == [{"id": data["back"][0]["id"], "widget_id": photo_widget.id, "action": "back"}] +def test_get_buttons_includes_placement_and_grid_dims(client, db_session): + """Two widgets of the same type otherwise look identical in the + assignment UI ("Photos" / "Photos") -- the client tells them apart + using x/y/w/h against the frame's grid dims (see + static/frame_config.js's buildWidgetNames), so the API needs to + actually hand those over.""" + client.post("/setup", data={"username": "alice", "password": "hunter22"}) + frame = db_session.get(Frame, 1) + photo_widget = db_session.query(Widget).filter_by(frame_id=frame.id, widget_type="photos").one() + photo_widget.x, photo_widget.y, photo_widget.w, photo_widget.h = 0, 0, 4, 5 + db_session.commit() + second = Widget(frame_id=frame.id, widget_type="photos", x=4, y=0, w=4, h=5, + sort_order=1, created_at=time.time()) + db_session.add(second) + db_session.flush() + from app.models import PhotoWidgetConfig + db_session.add(PhotoWidgetConfig(widget_id=second.id)) + db_session.commit() + + resp = client.get("/api/frames/1/buttons") + assert resp.status_code == 200 + data = resp.json() + + assert data["grid"] == {"cols": 8, "rows": 5} + by_id = {w["id"]: w for w in data["widgets"]} + assert by_id[photo_widget.id]["x"] == 0 and by_id[photo_widget.id]["w"] == 4 + assert by_id[second.id]["x"] == 4 and by_id[second.id]["w"] == 4 + + def test_put_replaces_the_whole_list_in_order(client, db_session): client.post("/setup", data={"username": "alice", "password": "hunter22"}) frame = db_session.get(Frame, 1)