diff --git a/CLAUDE.md b/CLAUDE.md index b9a739c..e1d1e87 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -189,5 +189,6 @@ Pytest settings are in `pyproject.toml` under `[tool.pytest.ini_options]` (`DJAN - **Signals handle side-effects** — do not manually recalculate `Game.playtime` or `Purchase.num_purchases`; the signals in `games/signals.py` do this on save/delete. - **Button colors**: `blue` (primary action), `red` (destructive), `gray` (secondary), `green` (positive). Icon buttons use `icon=True`. - **Inline Alpine.js** is used for client-side reactivity in domain components (`GameStatusSelector`, `SessionDeviceSelector`). The pattern is `x-data="{...}"` with `fetchWithHtmxTriggers()` for PATCH API calls. +- **Disabling composite widgets**: a composite widget (e.g. `SearchSelect`) carries its `id` on a wrapper `
`, which has no `disabled` state — setting `.disabled` on it is a no-op. Disable the inner control (for `SearchSelect`, the `[data-search-select-search]` input); the **component owns its disabled *look*** via `has-[:disabled]:` utilities on its container class, so callers toggle only the control's `disabled`, never styles. - **Platform icons** are SVG snippets in `games/templates/icons/.html`. Add new ones there and reference them by slug in `Platform.icon`. - **Name compound types explicitly** — if a `tuple`, `dict`, or other compound value is passed between functions or appears in multiple signatures, give it a named type (`TypedDict`, `NamedTuple`, or a `type` alias) rather than repeating the structural annotation. This applies even to small types used in only a few places; the name carries intent that the structure cannot. Examples: `LabeledOption = tuple[str, str]` instead of repeating `tuple[str, str]` for (value, label) pairs; `RangeValues(min, max)` instead of `tuple[str, str]` for range bounds. diff --git a/common/components/search_select.py b/common/components/search_select.py index b289b05..450d552 100644 --- a/common/components/search_select.py +++ b/common/components/search_select.py @@ -9,6 +9,13 @@ 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 +``
``, which has no ``disabled`` state of its own. To disable it, set +``disabled`` on the inner search ```` (``[data-search-select-search]``); +the wrapper then greys itself via the ``has-[:disabled]:`` utilities in +``_CONTAINER_CLASS``. Callers toggle only the control's ``disabled`` — never +styles. (See ``ts/add_purchase.ts`` gating ``related_game`` on the type field.) + Option sourcing follows two axes. *Population*: options are either rendered inline up front (``options=``, no ``search_url``) or fetched from ``search_url``. *Completeness*: without a ``search_url`` the inline set is the whole dataset and @@ -47,9 +54,13 @@ LabeledOption = tuple[str, str] # so its pills/hidden inputs flow as direct participants of that row, inline # with the search input. The options panel is absolute, so it sits outside the # flex flow. (border omitted intentionally — see if it's needed later.) +# The widget owns its disabled appearance: when any control inside it is +# :disabled (e.g. add_purchase.ts disabling the search input), the wrapper greys +# itself via :has() — callers only toggle the control's `disabled`, never styles. _CONTAINER_CLASS = ( "relative flex flex-wrap items-center gap-1 p-2 " - "rounded-base bg-neutral-secondary-medium" + "rounded-base bg-neutral-secondary-medium " + "has-[:disabled]:opacity-50 has-[:disabled]:cursor-not-allowed" ) _PILLS_CLASS = "contents" _SEARCH_CLASS = ( diff --git a/e2e/test_widgets_e2e.py b/e2e/test_widgets_e2e.py index 19fece8..2cf1925 100644 --- a/e2e/test_widgets_e2e.py +++ b/e2e/test_widgets_e2e.py @@ -167,8 +167,39 @@ def test_add_purchase_type_game_disables_related_game_search( (a
ignores the disabled property).""" 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]') + page.select_option("#id_type", "game") expect(search).to_be_disabled() + # The component greys itself via has-[:disabled] when the input is disabled. + assert wrapper.evaluate("el => getComputedStyle(el).opacity") == "0.5" + page.select_option("#id_type", "dlc") expect(search).to_be_enabled() + assert wrapper.evaluate("el => getComputedStyle(el).opacity") == "1" + + +def test_add_game_sync_stops_once_sort_name_edited( + authenticated_page: Page, live_server +): + """Name → Sort name mirrors live, but stops the moment the user edits Sort + name directly (the 'UntilChanged' contract). Editing Name afterwards must + not clobber the user's manual Sort name.""" + page = authenticated_page + page.goto(f"{live_server.url}{reverse('games:add_game')}") + name = page.locator("#id_name") + sort = page.locator("#id_sort_name") + + name.click() + name.type("Halo") + expect(sort).to_have_value("Halo") # live mirror before any manual edit + + sort.fill("Custom Sort") # user takes over the target → sync drops + expect(sort).to_have_value("Custom Sort") + + name.click() + name.press("End") + name.type(" 2") + expect(name).to_have_value("Halo 2") + expect(sort).to_have_value("Custom Sort") # not clobbered diff --git a/games/static/base.css b/games/static/base.css index 4e1ec6a..e036247 100644 --- a/games/static/base.css +++ b/games/static/base.css @@ -3430,6 +3430,16 @@ outline-style: none; } } + .has-\[\:disabled\]\:cursor-not-allowed { + &:has(*:is(:disabled)) { + cursor: not-allowed; + } + } + .has-\[\:disabled\]\:opacity-50 { + &:has(*:is(:disabled)) { + opacity: 50%; + } + } .data-\[search-select-highlighted\]\:bg-brand { &[data-search-select-highlighted] { background-color: var(--color-brand); diff --git a/ts/utils.ts b/ts/utils.ts index 75c72ae..6c4a432 100644 --- a/ts/utils.ts +++ b/ts/utils.ts @@ -38,9 +38,10 @@ function toISOUTCString(date: Date): string { } /** - * Mirrors each source element's value onto its target until the target is - * focused (manual edit wins). Each syncData entry maps a source selector and - * property onto a target selector and property. + * Mirrors each source element's value onto its target live as the user types, + * until the user edits the target directly — at which point that target is + * "dirty" and the manual value wins (no more mirroring into it). Each syncData + * entry maps a source selector and property onto a target selector and property. */ function syncSelectInputUntilChanged(syncData: Array<{ source: string; target: string; source_value: string; target_value: string }>, parentSelector: string | Document = document) { const parentElement = @@ -52,48 +53,31 @@ function syncSelectInputUntilChanged(syncData: Array<{ source: string; target: s console.error(`The parent selector "${parentSelector}" is not valid.`); return; } - // One delegated "input" listener handles every source. "input" (not "change") - // makes the mirror live as the user types, instead of only on blur. + // One delegated "input" listener drives both directions per syncItem. "input" + // (not "change") makes the mirror live as the user types. A target the user + // edits is marked dirty so the mirror stops clobbering it — programmatically + // setting target.value does NOT fire "input", so our own writes never mark a + // target dirty; only real user edits do. + const dirtyTargets = new Set(); parentElement.addEventListener("input", function (event) { - // Loop through each sync configuration item - syncData.forEach((syncItem: { source: string; target: string; source_value: string; target_value: string }) => { - // Check if the event target matches the source selector - if ((event.target as HTMLElement).matches(syncItem.source)) { - if (!event.target) return; - const sourceElement = event.target; - const valueToSync = getValueFromProperty( - sourceElement, - syncItem.source_value - ); + const eventTarget = event.target as HTMLElement; + syncData.forEach((syncItem, index) => { + // User edited the target directly → stop mirroring into it. + if (eventTarget.matches(syncItem.target)) { + dirtyTargets.add(index); + return; + } + // Source changed → mirror into the target unless the user took it over. + if (eventTarget.matches(syncItem.source) && !dirtyTargets.has(index)) { + const valueToSync = getValueFromProperty(eventTarget, syncItem.source_value); const targetElement = document.querySelector(syncItem.target); - if (targetElement && valueToSync !== null) { - console.log(`Changing value of ${syncItem.target} to ${valueToSync}`); - (targetElement as unknown as Record)[syncItem.target_value] = valueToSync; + (targetElement as unknown as Record)[syncItem.target_value] = + valueToSync; } } }); }); - - // Set up a single focus event listener on the document for handling all target focuses - const syncListener = (event: Event) => { - // Loop through each sync configuration item - syncData.forEach((syncItem: { source: string; target: string; source_value: string; target_value: string }) => { - // Check if the focus event target matches the target selector - if ((event.target as HTMLElement).matches(syncItem.target)) { - // Remove the change event listener to stop syncing - // This assumes you want to stop syncing once any target receives focus - // You may need a more sophisticated way to remove listeners if you want to stop - // syncing selectively based on other conditions - document.removeEventListener("change", syncListener); - } - }); - } - parentElement.addEventListener( - "focus", - syncListener, - true - ); // Use capture phase to ensure the event is captured during focus, not bubble } /**