Honor UntilChanged in sync; let SearchSelect own its disabled look
Follow-ups on the add-form fixes: - syncSelectInputUntilChanged now actually stops mirroring once the user edits the target (the "UntilChanged" contract). The old focus-based stop was a no-op (wrong removeEventListener reference), so live sync kept clobbering a manually-edited Sort name. Track dirty targets in a Set keyed by syncData index; programmatic writes don't fire "input", so only real user edits mark a target dirty. Drops the dead focus listener. - SearchSelect now greys itself when disabled, via has-[:disabled]: utilities on its container class — the visible "box" is the wrapper <div>, so disabling the transparent inner input alone left it looking active. The component owns its disabled appearance; callers only toggle the inner control's `disabled`. - Document the composite-widget disabling approach in CLAUDE.md and the SearchSelect docstring. Extends the e2e tests: sync drops after a manual Sort name edit; disabled related-game wrapper computes opacity 0.5 (and 1 when re-enabled). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -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.
|
- **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`.
|
- **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.
|
- **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 `<div>`, 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/<slug>.html`. Add new ones there and reference them by slug in `Platform.icon`.
|
- **Platform icons** are SVG snippets in `games/templates/icons/<slug>.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.
|
- **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.
|
||||||
|
|||||||
@@ -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
|
``data-*`` attributes wired up by ``ts/search_select.ts`` (compiled to
|
||||||
``games/static/js/dist/search_select.js``).
|
``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``. 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
|
Option sourcing follows two axes. *Population*: options are either rendered
|
||||||
inline up front (``options=``, no ``search_url``) or fetched from ``search_url``.
|
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
|
*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
|
# 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
|
# 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.)
|
# 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 = (
|
_CONTAINER_CLASS = (
|
||||||
"relative flex flex-wrap items-center gap-1 p-2 "
|
"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"
|
_PILLS_CLASS = "contents"
|
||||||
_SEARCH_CLASS = (
|
_SEARCH_CLASS = (
|
||||||
|
|||||||
@@ -167,8 +167,39 @@ def test_add_purchase_type_game_disables_related_game_search(
|
|||||||
(a <div> ignores the disabled property)."""
|
(a <div> ignores the disabled property)."""
|
||||||
page = authenticated_page
|
page = authenticated_page
|
||||||
page.goto(f"{live_server.url}{reverse('games:add_purchase')}")
|
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]')
|
search = page.locator('#id_related_game [data-search-select-search]')
|
||||||
|
|
||||||
page.select_option("#id_type", "game")
|
page.select_option("#id_type", "game")
|
||||||
expect(search).to_be_disabled()
|
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")
|
page.select_option("#id_type", "dlc")
|
||||||
expect(search).to_be_enabled()
|
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
|
||||||
|
|||||||
@@ -3430,6 +3430,16 @@
|
|||||||
outline-style: none;
|
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\]\:bg-brand {
|
||||||
&[data-search-select-highlighted] {
|
&[data-search-select-highlighted] {
|
||||||
background-color: var(--color-brand);
|
background-color: var(--color-brand);
|
||||||
|
|||||||
+22
-38
@@ -38,9 +38,10 @@ function toISOUTCString(date: Date): string {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Mirrors each source element's value onto its target until the target is
|
* Mirrors each source element's value onto its target live as the user types,
|
||||||
* focused (manual edit wins). Each syncData entry maps a source selector and
|
* until the user edits the target directly — at which point that target is
|
||||||
* property onto a target selector and property.
|
* "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) {
|
function syncSelectInputUntilChanged(syncData: Array<{ source: string; target: string; source_value: string; target_value: string }>, parentSelector: string | Document = document) {
|
||||||
const parentElement =
|
const parentElement =
|
||||||
@@ -52,48 +53,31 @@ function syncSelectInputUntilChanged(syncData: Array<{ source: string; target: s
|
|||||||
console.error(`The parent selector "${parentSelector}" is not valid.`);
|
console.error(`The parent selector "${parentSelector}" is not valid.`);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
// One delegated "input" listener handles every source. "input" (not "change")
|
// One delegated "input" listener drives both directions per syncItem. "input"
|
||||||
// makes the mirror live as the user types, instead of only on blur.
|
// (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<number>();
|
||||||
parentElement.addEventListener("input", function (event) {
|
parentElement.addEventListener("input", function (event) {
|
||||||
// Loop through each sync configuration item
|
const eventTarget = event.target as HTMLElement;
|
||||||
syncData.forEach((syncItem: { source: string; target: string; source_value: string; target_value: string }) => {
|
syncData.forEach((syncItem, index) => {
|
||||||
// Check if the event target matches the source selector
|
// User edited the target directly → stop mirroring into it.
|
||||||
if ((event.target as HTMLElement).matches(syncItem.source)) {
|
if (eventTarget.matches(syncItem.target)) {
|
||||||
if (!event.target) return;
|
dirtyTargets.add(index);
|
||||||
const sourceElement = event.target;
|
return;
|
||||||
const valueToSync = getValueFromProperty(
|
}
|
||||||
sourceElement,
|
// Source changed → mirror into the target unless the user took it over.
|
||||||
syncItem.source_value
|
if (eventTarget.matches(syncItem.source) && !dirtyTargets.has(index)) {
|
||||||
);
|
const valueToSync = getValueFromProperty(eventTarget, syncItem.source_value);
|
||||||
const targetElement = document.querySelector<HTMLSelectElement>(syncItem.target);
|
const targetElement = document.querySelector<HTMLSelectElement>(syncItem.target);
|
||||||
|
|
||||||
if (targetElement && valueToSync !== null) {
|
if (targetElement && valueToSync !== null) {
|
||||||
console.log(`Changing value of ${syncItem.target} to ${valueToSync}`);
|
(targetElement as unknown as Record<string, unknown>)[syncItem.target_value] =
|
||||||
(targetElement as unknown as Record<string, unknown>)[syncItem.target_value] = valueToSync;
|
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
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
Reference in New Issue
Block a user