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
This commit is contained in:
@@ -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]
|
||||
|
||||
|
||||
|
||||
+29
-12
@@ -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
|
||||
):
|
||||
|
||||
+1
-6
@@ -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) => {
|
||||
|
||||
Reference in New Issue
Block a user