From b816c68cb8630122225cf24020b9988c97894dab Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Kucharczyk?= <lukas@kucharczyk.xyz>
Date: Sat, 20 Jun 2026 18:34:07 +0200
Subject: [PATCH] fix(search-select): move field id to inner search input
 (issue #30)

The id (e.g. id_related_game) sat on the <search-select> wrapper, a
non-labelable custom element. Consequences:
- <label for="id_X"> focused nothing (a11y gap)
- .disabled / .focus() on #id_X silently no-oped
- add_purchase.ts needed a [data-search-select-search] descendant
  workaround to gate related_game on the type field

id is now on the [data-search-select-search] <input>, making it a real
labelable, disableable control. add_purchase.ts drops the workaround
and gates via #id_related_game directly. E2e tests updated; new test
asserts label-click focuses the search box.

Closes #30
---
 common/components/search_select.py | 23 +++++++++--------
 e2e/test_widgets_e2e.py            | 41 +++++++++++++++++++++---------
 ts/add_purchase.ts                 |  7 +----
 3 files changed, 43 insertions(+), 28 deletions(-)

diff --git a/common/components/search_select.py b/common/components/search_select.py
index c2ee25b..c7fe4aa 100644
--- a/common/components/search_select.py
+++ b/common/components/search_select.py
@@ -9,15 +9,17 @@ This module imports only from ``common.components`` — it has no Django-forms o
 ``data-*`` attributes wired up by ``ts/search_select.ts`` (compiled to
 ``games/static/js/dist/search_select.js``).
 
-**Disabling**: this is a composite widget — its ``id`` sits on the wrapper
-``<div>``, which has no ``disabled`` state of its own. To disable it, set
-``disabled`` on the inner search ``<input>`` (``[data-search-select-search]``);
-the wrapper then greys itself via the ``has-[:disabled]:`` utilities in
-``_CONTAINER_CLASS``. The inner input is excluded from the global
-disabled-input surface (``common/input.css``) so it stays transparent — the
-widget reads as one faded element, not a nested box. Callers toggle only the
-control's ``disabled`` — never styles. (See ``ts/add_purchase.ts`` gating
-``related_game`` on the type field.)
+**Field id / label association**: when ``SearchSelect`` is used as a Django form
+widget, the field ``id`` (e.g. ``id_related_game``) is placed on the inner
+search ``<input>`` (``[data-search-select-search]``), making it a real labelable
+control. ``<label for="id_X">`` therefore focuses the search box, and
+``document.querySelector('#id_X').disabled`` behaves as for a native input.
+
+**Disabling**: set ``disabled`` directly on the field id (or on the inner
+``[data-search-select-search]`` input). The wrapper greys itself via the
+``has-[:disabled]:`` utilities in ``_CONTAINER_CLASS``. The inner input stays
+transparent — the widget reads as one faded element, not a nested box. Callers
+toggle only the control's ``disabled`` — never styles.
 
 Option sourcing follows two axes. *Population*: options are either rendered
 inline up front (``options=``, no ``search_url``) or fetched from ``search_url``.
@@ -301,6 +303,8 @@ def SearchSelect(
         ("autocomplete", "off"),
         ("class", _SEARCH_CLASS),
     ]
+    if id:
+        search_attrs.append(("id", id))
     if autofocus:
         search_attrs.append(("autofocus", ""))
     if search_value:
@@ -345,7 +349,6 @@ def SearchSelect(
         prefetch=prefetch,
         sync_url="true" if sync_url else "false",
         class_=_CONTAINER_CLASS,
-        id_=id or None,
     )[*children]
 
 
diff --git a/e2e/test_widgets_e2e.py b/e2e/test_widgets_e2e.py
index 2891e62..2009214 100644
--- a/e2e/test_widgets_e2e.py
+++ b/e2e/test_widgets_e2e.py
@@ -161,14 +161,15 @@ def test_searchselect_border_matches_native_input(
     page = authenticated_page
     page.goto(f"{live_server.url}{reverse('games:add_purchase')}")
     price = page.locator("#id_price")  # always-enabled native input
-    wrapper = page.locator("#id_platform")
-    search = page.locator("#id_platform [data-search-select-search]")
+    # #id_platform is now on the inner <input>; find the wrapper by name attr.
+    wrapper = page.locator("search-select[name='platform']")
+    search_input = page.locator("#id_platform")
     border = "el => getComputedStyle(el).borderColor"
 
     rest = price.evaluate(border)
     assert wrapper.evaluate(border) == rest  # same border at rest
 
-    search.focus()
+    search_input.focus()
     focused_wrapper = wrapper.evaluate(border)
     price.focus()
     focused_input = price.evaluate(border)
@@ -189,19 +190,21 @@ def test_add_game_syncs_sort_name_from_name(authenticated_page: Page, live_serve
 def test_add_purchase_type_game_disables_related_game_search(
     authenticated_page: Page, live_server
 ):
-    """When Type is 'game', the related-game SearchSelect is disabled — the
-    real disable target is the inner search input, not the wrapper <div>
-    (a <div> ignores the disabled property)."""
+    """When Type is 'game', the related-game SearchSelect is disabled.
+    #id_related_game is the inner search <input> (the real labelable control),
+    and the <search-select> wrapper fades via has-[:disabled]:opacity-50."""
     page = authenticated_page
     page.goto(f"{live_server.url}{reverse('games:add_purchase')}")
-    wrapper = page.locator("#id_related_game")
-    search = page.locator("#id_related_game [data-search-select-search]")
+    # #id_related_game is now on the inner <input data-search-select-search>
+    search_input = page.locator("#id_related_game")
+    # The wrapper has no id; find it by the stable `name` attribute.
+    wrapper = page.locator("search-select[name='related_game']")
     name = page.locator("#id_name")
     opacity = "el => getComputedStyle(el).opacity"
     bg = "el => getComputedStyle(el).backgroundColor"
 
     page.select_option("#id_type", "game")
-    expect(search).to_be_disabled()
+    expect(search_input).to_be_disabled()
     # A disabled SearchSelect must look identical to a disabled native input:
     # both fade (opacity-50) over the same surface.
     assert wrapper.evaluate(opacity) == "0.5"
@@ -209,16 +212,30 @@ def test_add_purchase_type_game_disables_related_game_search(
     assert wrapper.evaluate(bg) == name.evaluate(bg)
     # The inner input stays transparent (no nested box) with the same not-allowed
     # cursor (no flicker across the widget).
-    assert search.evaluate(bg) == "rgba(0, 0, 0, 0)"
-    assert search.evaluate("el => getComputedStyle(el).cursor") == "not-allowed"
+    assert search_input.evaluate(bg) == "rgba(0, 0, 0, 0)"
+    assert search_input.evaluate("el => getComputedStyle(el).cursor") == "not-allowed"
 
     page.select_option("#id_type", "dlc")
-    expect(search).to_be_enabled()
+    expect(search_input).to_be_enabled()
     # Enabled, both return to full opacity.
     assert wrapper.evaluate(opacity) == "1"
     assert name.evaluate(opacity) == "1"
 
 
+def test_label_click_focuses_search_select(authenticated_page: Page, live_server):
+    """Clicking a <label for="id_X"> on a SearchSelect field must focus the
+    search input — confirmed now that id is on the real <input> control."""
+    page = authenticated_page
+    page.goto(f"{live_server.url}{reverse('games:add_purchase')}")
+    # related_game is disabled when type is "game" (the default); switch so it
+    # is enabled, otherwise clicking the label for a disabled control fails.
+    page.select_option("#id_type", "dlc")
+    label = page.locator("label[for='id_related_game']")
+    search_input = page.locator("#id_related_game")
+    label.click()
+    expect(search_input).to_be_focused()
+
+
 def test_add_game_sync_stops_once_sort_name_edited(
     authenticated_page: Page, live_server
 ):
diff --git a/ts/add_purchase.ts b/ts/add_purchase.ts
index 5aa64bd..76ba68e 100644
--- a/ts/add_purchase.ts
+++ b/ts/add_purchase.ts
@@ -53,12 +53,7 @@ onSwap("#id_separate_prices", (checkbox) => {
 });
 
 function setupElementHandlers(): void {
-  // related_game is a SearchSelect: its #id_related_game wrapper is a <div>
-  // (ignores `disabled`), so target the inner search <input> instead.
-  disableElementsWhenTrue("#id_type", "game", [
-    "#id_name",
-    "#id_related_game [data-search-select-search]",
-  ]);
+  disableElementsWhenTrue("#id_type", "game", ["#id_name", "#id_related_game"]);
 }
 
 onSwap("#id_type", (typeSelect) => {