yuris/garage
1
0
Fork 0
Code Issues 1 Pull requests Projects Releases Packages 1 Wiki Activity Actions

feat(store): product detail — fitment, OEM numbers, availability, indent carry #55

Merged
yuris merged 4 commits from feat/47-product-detail into main 2026-08-27 14:30:24 +00:00
Conversation 8 Commits 4 Files changed 8 +270 -19
yuris commented 2026-08-27 12:46:32 +00:00
Owner
Copy link

Closes #47.

What changed

  • app/products/[slug]/page.tsx — Kecocokan kendaraan section renders fitment[] as a <ul> with the mockup's reassurance line; the whole <section> is omitted when fitment is empty.
  • app/products/[slug]/page.tsx — Nomor OEM row beside Nomor part:, both wrapped in .spec-code (mono); each row omitted when its source is null/empty. Both lists deduped — the dashboard collects them as repeatable rows, so X, Y, X is an operator slip, and a duplicate fitment string would also collide as a React key.
  • app/products/[slug]/page.tsx — harga per <tanggal> under the price, only when priceUpdatedAt is present. Formatted via a module-level Intl.DateTimeFormat pinned to Asia/Jakarta, so an evening save does not slide to the next day on a UTC build machine.
  • app/products/[slug]/page.tsx — plain-language indent line, Indent: barang dipesan dulu, bukan stok toko., rendered next to the chip rather than only inside the buy block.
  • lib/cart.ts — CartLine gains optional indent?: boolean; isLine validates it; readStorage normalises absence to false and whitelists it through the boundary.
  • app/products/[slug]/add-to-cart.tsx — passes indent: availability === "indent" into add().
  • lib/checkout-message.ts — the single-tag ternary became a tags[] join, so an indent line reads [indent — pesan dulu] and combines correctly with the existing pasang di tempat / ambil di bengkel tags.
  • app/globals.css — .stock-badge becomes a tinted pill for all three states (--chip drives text, fill and border). My region only; I did not touch the filter-sidebar/chip-row/toolbar region owned by #46.

Not touched: product-browser.tsx, products/page.tsx, /services, /contact, sitemap.ts, packages/catalog, apps/management. formatIdr is still the only price formatter. No new dependencies. No stock quantity is rendered or exported.

The chip tint — implemented

#45 left .stock-badge text-only and deferred the tint. Implemented, because --warn rgb(138,90,18) and --off rgb(140,58,50) differ by 2 in the red channel: hue carries nearly all the separation, so a colour-blind buyer reading a bare word gets no state signal. All three states get the identical treatment — one --chip custom property, 12% fill, 35% border, 999px radius — so a state adds a colour and never a second treatment. Never one filled chip beside two unfilled.

12% is a ceiling set by --warn, not a taste pick. Measured text-on-fill contrast (WCAG 2.x, sRGB):

token on --bg @12% on --surface @12% @14% on --bg
--ok 6.54:1 7.09:1 6.33:1
--warn 4.61:1 5.00:1 4.48:1
--off 5.79:1 6.28:1 5.60:1

The chip computes to 12.48px/700 — bold but below the 18.66px large-text threshold, so it needs 4.5:1. 14% puts --warn at 4.48:1, under AA; 12% holds at 4.61:1. The 35% border is decorative only (1.65–1.82:1); what actually separates the states without hue is the label text.

Mixed toward transparent rather than a surface token, because the chip sits on --bg on the detail page and inside a .product-card on a grid — one rgba tint composites correctly over both.

The cascade note from the issue turned out to be reachable, not theoretical. product-card.tsx renders the card chip as <p class="stock-badge is-out">, and .product-card p (0,1,1) outranks a bare .stock-badge (0,1,0). Text-only that was invisible; filled it would be grey text on a red tint. Handled with a .product-card .stock-badge (0,2,0) co-selector in the same rule, and proved load-bearing in the browser — stripping that one selector via CSSOM moves the chip from rgb(140,58,50) to rgb(90,97,89) (--ink-muted). align-self: start is there because the chip is a flex item inside the card, and blockified inline-block would stretch the fill across the whole card.

Verified

Every check below is a command I ran locally. No workflow runs on pull_request — both .forgejo/workflows/*.yml fire only on push: main and they deploy (management publishes a container, the store runs wrangler pages deploy). The forge never checks this PR, so merging this PR is a production release.

All output below is at the merge candidate b830657, tree clean.

$ git rev-parse HEAD
b830657c9de49f674fa9b8771b8e564c83a874f5

$ pnpm --filter garage-store test
# tests 66
# suites 11
# pass 66
# fail 0
# skipped 0
# todo 0

66 = the 56 that #49 established, plus 10 new (6 in cart.test.ts, 4 in checkout-message.test.ts). No pre-existing case was deleted or weakened.

$ pnpm --filter garage-store build
✓ Compiled successfully in 2.4s
  Running TypeScript ...
  Finished TypeScript in 2.8s ...
✓ Generating static pages using 13 workers (21/21) in 729ms

$ pnpm exec biome check <the 7 changed files>
Checked 7 files in 14ms. No fixes applied.
Found 2 warnings.

Both warnings are pre-existing in globals.css and untouched by this branch — noImportantStyles at .product-type (line 471) and noDescendingSpecificity at .image-placeholder (line 641). Confirmed identical by running biome check on the same file with my changes stashed. No repo-wide format or lint run.

The isLine ordering trap

The new clause is appended last, after every typeof guard, so the documented entailment — Number.isInteger/Number.isFinite implying typeof === "number" because neither coerces — still holds, and the comment explaining it is still accurate. Nothing was reordered ahead of a comparison.

The new field's case bites. Deleting only the indent clause from isLine turns three tests red:

not ok 21 - rejects a string indent
not ok 22 - rejects a numeric indent
not ok 3 - drops malformed entries but keeps the well-formed ones
# tests 66
# pass 63
# fail 3

Restored, back to 66/66. indent is accepted absent (a pre-#47 cart must not be emptied on deploy) but must be a real boolean when present — a truthy "no" would otherwise reach the shop as a back-order flag it never agreed to.

Browser verification

Real Chromium against next build static output, on a 5-item fixture covering every state plus priced and unpriced habis plus the empty-array path. Measured at 1440px and 375px; zero horizontal overflow at both (scrollWidth - clientWidth == 0 on all 5 pages).

JSON-LD, parsed out of the built HTML — all three states mapped, Offer only when priced:

page state offers.availability Offer
shockbreaker-depan-avanza ready schema.org/InStock present, 450000 IDR
seal-kruk-as-xenia indent schema.org/BackOrder present, 85000 IDR
kampas-rem-belakang-innova habis, priced schema.org/OutOfStock present, 310000 IDR
radiator-assy-rush habis, unpriced — absent
oli-mesin-10w40-bare ready schema.org/InStock present, 62000 IDR

Product and BreadcrumbList both still emit on every page. The unpriced habis page also renders no detail .price element at all (scoped to the detail block — an unscoped .price query picks up a related-grid card, which is what makes this worth stating).

Per case, both widths:

case result
ready green chip Ready rgb(45,84,60), fill srgb(.176 .329 .235 / .12), border /.35, 999px, 2px 8px, box 55x22 — not full-bleed. harga per 14 Agustus 2026. Add enabled.
indent amber chip Indent rgb(138,90,18), same treatment, box 58x22. Indent: barang dipesan dulu, bukan stok toko. rendered. Add enabled.
habis (priced + unpriced) red chip Habis rgb(140,58,50), same treatment, box 52x22. Add button disabled: true, no qty stepper, sticky bar degrades to <a>Tanyakan ketersediaan, note Stok habis dan barang ini belum bisa dipesan…
empty fitment + empty oemNumbers oli-mesin-10w40-bare: no section.fitment in the DOM at all, no Nomor OEM row, no Nomor part row, no harga per line. Description flows straight into Stok dan harga — no empty heading, no orphan separator.
fitment populated heading Kecocokan kendaraan, both vehicles as <li>, reassurance line verbatim: Ragu cocok atau tidak? Kirim foto STNK atau nomor rangka lewat WhatsApp, kami cek dulu sebelum dikirim.
mono spec values .spec-code computes ui-monospace, Menlo, Consolas, monospace on both the part and OEM rows
card chips .product-card .stock-badge computes --off/--warn, align-self: start, box 52x25 / 58x25, stretched: false

Indent reaches the shop, end to end. Clicked the real add button at 375px on the indent page, then on a stocked page:

[{"slug":"seal-kruk-as-xenia","name":"Seal Kruk As Xenia","qty":1,"price":85000,
  "installAtShop":false,"shippable":true,"indent":true},
 {"slug":"shockbreaker-depan-avanza","name":"Shockbreaker Depan Avanza","qty":1,"price":450000,
  "installAtShop":false,"shippable":true,"indent":false}]

Then the cart page's real checkout link, text= decoded:

Halo AUTO PRIMAJAYA, saya mau pesan:
1. Seal Kruk As Xenia (1) — Rp 85.000 [indent — pesan dulu]
2. Shockbreaker Depan Avanza (1) — Rp 450.000
Total barang: Rp 535.000 (belum ongkir)
Kirim ke: Budi Santoso
Catatan: Jl. Raya Kotakan Utara No. 12, Situbondo
https://autoprimajaya.co.id/cart

The indent line is tagged, the stocked line is clean — which is exactly the gap #53's review found and reassigned here. checkout-message.test.ts covers the tag, both tag combinations (indent + pasang di tempat, indent + ambil di bengkel), and a pre-#47 line with the key absent emitting no tag at all.

The 5-item fixture and its two throwaway server scripts are intentionally not committed; they are local verification scaffolding, and the store has no catalog fixture convention to add them to.

Deviations

The harga per <tanggal> wording is deliberately "as of", not "last changed", per the declined-imprecision note on the issue: every dashboard save refreshes priceUpdatedAt, so calling it a price-change date would assert something the data cannot support. A comment in page.tsx records this so it is not "fixed" later.

Otherwise none. Berat still comes from weightGrams untouched; no volumetric weight, no Posisi/Tipe/Garansi, no warranty field, no stock quantity anywhere.

Closes #47. ## What changed - `app/products/[slug]/page.tsx` — **Kecocokan kendaraan** section renders `fitment[]` as a `<ul>` with the mockup's reassurance line; the whole `<section>` is omitted when `fitment` is empty. - `app/products/[slug]/page.tsx` — **Nomor OEM** row beside `Nomor part:`, both wrapped in `.spec-code` (mono); each row omitted when its source is null/empty. Both lists deduped — the dashboard collects them as repeatable rows, so `X, Y, X` is an operator slip, and a duplicate `fitment` string would also collide as a React key. - `app/products/[slug]/page.tsx` — `harga per <tanggal>` under the price, only when `priceUpdatedAt` is present. Formatted via a module-level `Intl.DateTimeFormat` pinned to `Asia/Jakarta`, so an evening save does not slide to the next day on a UTC build machine. - `app/products/[slug]/page.tsx` — plain-language indent line, `Indent: barang dipesan dulu, bukan stok toko.`, rendered next to the chip rather than only inside the buy block. - `lib/cart.ts` — `CartLine` gains optional `indent?: boolean`; `isLine` validates it; `readStorage` normalises absence to `false` and whitelists it through the boundary. - `app/products/[slug]/add-to-cart.tsx` — passes `indent: availability === "indent"` into `add()`. - `lib/checkout-message.ts` — the single-tag ternary became a `tags[]` join, so an indent line reads `[indent — pesan dulu]` and combines correctly with the existing `pasang di tempat` / `ambil di bengkel` tags. - `app/globals.css` — `.stock-badge` becomes a tinted pill for all three states (`--chip` drives text, fill and border). **My region only**; I did not touch the filter-sidebar/chip-row/toolbar region owned by #46. Not touched: `product-browser.tsx`, `products/page.tsx`, `/services`, `/contact`, `sitemap.ts`, `packages/catalog`, `apps/management`. `formatIdr` is still the only price formatter. No new dependencies. No stock quantity is rendered or exported. ### The chip tint — implemented #45 left `.stock-badge` text-only and deferred the tint. Implemented, because `--warn` `rgb(138,90,18)` and `--off` `rgb(140,58,50)` differ by 2 in the red channel: hue carries nearly all the separation, so a colour-blind buyer reading a bare word gets no state signal. All three states get the identical treatment — one `--chip` custom property, 12% fill, 35% border, `999px` radius — so a state adds a colour and never a second treatment. Never one filled chip beside two unfilled. 12% is a ceiling set by `--warn`, not a taste pick. Measured text-on-fill contrast (WCAG 2.x, sRGB): | token | on `--bg` @12% | on `--surface` @12% | @14% on `--bg` | |---|---|---|---| | `--ok` | 6.54:1 | 7.09:1 | 6.33:1 | | `--warn` | **4.61:1** | 5.00:1 | **4.48:1** | | `--off` | 5.79:1 | 6.28:1 | 5.60:1 | The chip computes to 12.48px/700 — bold but below the 18.66px large-text threshold, so it needs 4.5:1. 14% puts `--warn` at 4.48:1, under AA; 12% holds at 4.61:1. The 35% border is decorative only (1.65–1.82:1); what actually separates the states without hue is the label text. Mixed toward `transparent` rather than a surface token, because the chip sits on `--bg` on the detail page and inside a `.product-card` on a grid — one rgba tint composites correctly over both. **The cascade note from the issue turned out to be reachable, not theoretical.** `product-card.tsx` renders the card chip as `<p class="stock-badge is-out">`, and `.product-card p` (0,1,1) outranks a bare `.stock-badge` (0,1,0). Text-only that was invisible; filled it would be grey text on a red tint. Handled with a `.product-card .stock-badge` (0,2,0) co-selector in the same rule, and proved load-bearing in the browser — stripping that one selector via CSSOM moves the chip from `rgb(140,58,50)` to `rgb(90,97,89)` (`--ink-muted`). `align-self: start` is there because the chip is a flex item inside the card, and blockified `inline-block` would stretch the fill across the whole card. ## Verified Every check below is a command I ran locally. **No workflow runs on `pull_request`** — both `.forgejo/workflows/*.yml` fire only on `push: main` and they *deploy* (management publishes a container, the store runs `wrangler pages deploy`). The forge never checks this PR, so **merging this PR is a production release.** All output below is at the merge candidate `b830657`, tree clean. ``` $ git rev-parse HEAD b830657c9de49f674fa9b8771b8e564c83a874f5 $ pnpm --filter garage-store test # tests 66 # suites 11 # pass 66 # fail 0 # skipped 0 # todo 0 ``` 66 = the 56 that #49 established, plus 10 new (6 in `cart.test.ts`, 4 in `checkout-message.test.ts`). No pre-existing case was deleted or weakened. ``` $ pnpm --filter garage-store build ✓ Compiled successfully in 2.4s Running TypeScript ... Finished TypeScript in 2.8s ... ✓ Generating static pages using 13 workers (21/21) in 729ms $ pnpm exec biome check <the 7 changed files> Checked 7 files in 14ms. No fixes applied. Found 2 warnings. ``` Both warnings are pre-existing in `globals.css` and untouched by this branch — `noImportantStyles` at `.product-type` (line 471) and `noDescendingSpecificity` at `.image-placeholder` (line 641). Confirmed identical by running `biome check` on the same file with my changes stashed. No repo-wide format or lint run. ### The `isLine` ordering trap The new clause is appended **last**, after every `typeof` guard, so the documented entailment — `Number.isInteger`/`Number.isFinite` implying `typeof === "number"` because neither coerces — still holds, and the comment explaining it is still accurate. Nothing was reordered ahead of a comparison. The new field's case bites. Deleting only the `indent` clause from `isLine` turns three tests red: ``` not ok 21 - rejects a string indent not ok 22 - rejects a numeric indent not ok 3 - drops malformed entries but keeps the well-formed ones # tests 66 # pass 63 # fail 3 ``` Restored, back to 66/66. `indent` is accepted absent (a pre-#47 cart must not be emptied on deploy) but must be a real boolean when present — a truthy `"no"` would otherwise reach the shop as a back-order flag it never agreed to. ### Browser verification Real Chromium against `next build` static output, on a 5-item fixture covering every state plus priced **and** unpriced `habis` plus the empty-array path. Measured at **1440px** and **375px**; zero horizontal overflow at both (`scrollWidth - clientWidth == 0` on all 5 pages). JSON-LD, parsed out of the built HTML — all three states mapped, `Offer` only when priced: | page | state | `offers.availability` | `Offer` | |---|---|---|---| | `shockbreaker-depan-avanza` | ready | `schema.org/InStock` | present, 450000 IDR | | `seal-kruk-as-xenia` | indent | `schema.org/BackOrder` | present, 85000 IDR | | `kampas-rem-belakang-innova` | habis, priced | `schema.org/OutOfStock` | present, 310000 IDR | | `radiator-assy-rush` | habis, **unpriced** | — | **absent** | | `oli-mesin-10w40-bare` | ready | `schema.org/InStock` | present, 62000 IDR | `Product` and `BreadcrumbList` both still emit on every page. The unpriced `habis` page also renders no detail `.price` element at all (scoped to the detail block — an unscoped `.price` query picks up a related-grid card, which is what makes this worth stating). Per case, both widths: | case | result | |---|---| | ready | green chip `Ready` `rgb(45,84,60)`, fill `srgb(.176 .329 .235 / .12)`, border `/.35`, `999px`, `2px 8px`, box 55x22 — not full-bleed. `harga per 14 Agustus 2026`. Add enabled. | | indent | amber chip `Indent` `rgb(138,90,18)`, same treatment, box 58x22. `Indent: barang dipesan dulu, bukan stok toko.` rendered. Add **enabled**. | | habis (priced + unpriced) | red chip `Habis` `rgb(140,58,50)`, same treatment, box 52x22. Add button `disabled: true`, no qty stepper, sticky bar degrades to `<a>Tanyakan ketersediaan`, note `Stok habis dan barang ini belum bisa dipesan…` | | empty `fitment` + empty `oemNumbers` | `oli-mesin-10w40-bare`: no `section.fitment` in the DOM at all, no `Nomor OEM` row, no `Nomor part` row, no `harga per` line. Description flows straight into `Stok dan harga` — no empty heading, no orphan separator. | | fitment populated | heading `Kecocokan kendaraan`, both vehicles as `<li>`, reassurance line verbatim: `Ragu cocok atau tidak? Kirim foto STNK atau nomor rangka lewat WhatsApp, kami cek dulu sebelum dikirim.` | | mono spec values | `.spec-code` computes `ui-monospace, Menlo, Consolas, monospace` on both the part and OEM rows | | card chips | `.product-card .stock-badge` computes `--off`/`--warn`, `align-self: start`, box 52x25 / 58x25, `stretched: false` | **Indent reaches the shop, end to end.** Clicked the real add button at 375px on the indent page, then on a stocked page: ```json [{"slug":"seal-kruk-as-xenia","name":"Seal Kruk As Xenia","qty":1,"price":85000, "installAtShop":false,"shippable":true,"indent":true}, {"slug":"shockbreaker-depan-avanza","name":"Shockbreaker Depan Avanza","qty":1,"price":450000, "installAtShop":false,"shippable":true,"indent":false}] ``` Then the cart page's real checkout link, `text=` decoded: ``` Halo AUTO PRIMAJAYA, saya mau pesan: 1. Seal Kruk As Xenia (1) — Rp 85.000 [indent — pesan dulu] 2. Shockbreaker Depan Avanza (1) — Rp 450.000 Total barang: Rp 535.000 (belum ongkir) Kirim ke: Budi Santoso Catatan: Jl. Raya Kotakan Utara No. 12, Situbondo https://autoprimajaya.co.id/cart ``` The indent line is tagged, the stocked line is clean — which is exactly the gap #53's review found and reassigned here. `checkout-message.test.ts` covers the tag, both tag combinations (`indent + pasang di tempat`, `indent + ambil di bengkel`), and a pre-#47 line with the key absent emitting no tag at all. The 5-item fixture and its two throwaway server scripts are intentionally **not** committed; they are local verification scaffolding, and the store has no catalog fixture convention to add them to. ## Deviations The `harga per <tanggal>` wording is deliberately "as of", not "last changed", per the declined-imprecision note on the issue: every dashboard save refreshes `priceUpdatedAt`, so calling it a price-change date would assert something the data cannot support. A comment in `page.tsx` records this so it is not "fixed" later. Otherwise none. Berat still comes from `weightGrams` untouched; no volumetric weight, no `Posisi`/`Tipe`/`Garansi`, no warranty field, no stock quantity anywhere.
Kecocokan kendaraan block, OEM numbers beside the part number, "harga per
<tanggal>" under the price, a plain-language indent label, and tinted
availability chips.

Carries indent through to the shop: CartLine gains an optional indent flag,
isLine validates it, add-to-cart passes it, and checkoutMessage tags the
line so a back-order is distinguishable from a stocked part.
yuris reviewed 2026-08-27 12:59:09 +00:00
yuris left a comment
Copy link

VERDICT: approve-with-nits

Reviewed at b830657 in a throwaway /tmp detached worktree. Built next build (output: "export") against my own 5-item schemaVersion: 3 fixture served over HTTP on 127.0.0.1:8955 with x-api-key, covering ready / indent / habis, priced and unpriced habis, plus an item with fitment: [], oemNumbers: [], partNumber: null, priceUpdatedAt: null. Two of the five items carry a deliberately duplicated fitment / oemNumbers entry to exercise the dedup. Served out/ and drove real Chromium at 1440x900 and 375x812. The main checkout was never modified.

Everything in the acceptance criteria holds. Nothing blocking.

The eight things I was asked to state explicitly

1. isLine clause ordering — preserved, and the new clause bites.
cart.ts:44-58: typeof line.qty === "number" (47) → Number.isInteger (48) → line.qty > 0 (49). The comparison is still after both guards; the price pair is typeof (51) then Number.isFinite (51). The indent clause is appended last at line 57, ahead of nothing. The documenting comment at cart.ts:36-39 is still accurate, and I falsified both halves of it rather than reading it: deleting typeof line.qty === "number" outright leaves 66/66 green, confirming the entailment it claims; and Number.isInteger("5") === false, Number.isFinite("5") === false, "5" > 0 === true, confirming the reorder hazard it warns about is real. Nothing was reordered ahead of a comparison.

Mutation on the new clause reproduces the author's claim exactly — deleting only the indent clause:

not ok 21 - rejects a string indent
not ok 22 - rejects a numeric indent
not ok 3 - drops malformed entries but keeps the well-formed ones
# tests 66 / # pass 63 / # fail 3

Restored → 66/66.

2. No pre-existing cart case weakened. git diff origin/main...b830657 on cart.test.ts and checkout-message.test.ts produces zero removed lines — pure addition. Baseline measured independently in a second origin/main worktree: 56/56. Branch: 66/66, 11 suites. 56 + 10, none removed, as claimed.

3. Indent reaches the shop; a stocked line stays clean. Real clicks on the real buttons at 375px, then the cart page's actual href, text= decoded — not the test fixture:

1. Seal Kruk As Xenia (1) — Rp 85.000 [indent — pesan dulu]
2. Shockbreaker Depan Avanza (1) — Rp 450.000

localStorage after the two clicks carried "indent":true and "indent":false respectively. Also checked two things the PR body did not claim: adding the indent item twice merges to qty: 2 with indent: true intact (add spreads the existing line), and a hand-seeded pre-#47 line with the key absent survives readStorage and still renders at /cart — no cart is emptied on deploy.

4. Contrast — re-measured from the token hexes, WCAG 2.x sRGB, fill composited over the real backdrop. My numbers match the PR body to the hundredth:

token text on fill@12% over --bg over --surface @14% over --bg
--ok #2d543c 6.54:1 7.09:1 6.33:1
--warn #8a5a12 4.61:1 5.00:1 4.48:1
--off #8c3a32 5.79:1 6.28:1 5.60:1

The chip computes 12.48px / 700 — bold but under the 18.66px large-text threshold, so 4.5:1 is the correct target, and all three clear it on both backdrops. The 14% figure confirms the ceiling: --warn falls to 4.48:1, under AA. Treatment is applied consistently — all three states get the identical --chip-driven fill/border/radius; measured boxes 55x22 (ready), 58x22 (indent), 52x22 (habis) on the detail page and 52x25 / 58x25 on cards, stretched: false everywhere. No state is the only filled or only unfilled chip.

5. The .product-card p cascade fix is load-bearing and leak-free — but its comment is wrong. Load-bearing today: I set rule.selectorText = '.stock-badge' via CSSOM on the rendered grid and all three card chips moved from rgb(140,58,50) / rgb(138,90,18) to rgb(90,97,89) (--ink-muted) while keeping the tinted fill; restoring the co-selector restored them. Leak-free: .product-card .stock-badge matches exactly the 3 elements that .stock-badge matches (a strict subset), and every non-badge .product-card p kept its expected colour. See the should below — the code comment's stated justification is the opposite of what I measured.

6. Empty blocks vanish cleanly on the rendered page. oli-mesin-10w40-bare: no section.fitment in the DOM, h2 list is ["Stok dan harga"] only, .price-note list is ["Kirim ke alamat saya"] only — no Nomor OEM, no Nomor part, no harga per, no orphan row, no stray separator. Confirmed visually: the description flows straight into the Stok dan harga eyebrow. Dedup works on the populated pages: fixture ["48510-BZ080","48510-BZ090","48510-BZ080"] renders 48510-BZ080, 48510-BZ090, and a duplicated fitment string collapses to one <li>. .spec-code computes ui-monospace, Menlo, Consolas, monospace on both rows.

7. JSON-LD, parsed out of the built HTML. Product and BreadcrumbList on all five pages; all three states mapped; Offer only when priced:

slug state offers.availability
shockbreaker-depan-avanza ready schema.org/InStock
seal-kruk-as-xenia indent schema.org/BackOrder
kampas-rem-belakang-innova habis, priced schema.org/OutOfStock
radiator-assy-rush habis, unpriced no offers key at all
oli-mesin-10w40-bare ready schema.org/InStock

radiator-assy-rush also renders no detail-scoped .price element at either width.

8. CSS diff stayed out of #46's region. #55's hunks land at 490-536 (.stock-badge) and 709-748 (.availability-note h2, .fitment h2, .fitment, .spec-code). I fetched feat/46-faceted-filters: its entire globals.css change is one hunk appended at 845+ adding .browser*, .facet-*, .filter-*, .result-count. Disjoint line ranges, disjoint selectors, no .stock-badge / .fitment / .spec-code / .availability-note anywhere in #54's CSS diff. These will merge without conflict.

Also re-proved at this head (#41 not regressed)

availabilityBadgeClass is imported from product-card.tsx by page.tsx:12, not forked. add-to-cart.tsx:38-62 still early-returns on habis only, so the indent: availability === "indent" comment at line 69 is accurate. Rendered: habis (priced and unpriced) → add button disabled: true, no qty stepper, Tanyakan ketersediaan degrade; indent → add enabled; ready → add enabled. formatIdr is still the only price formatter — priceDateFormat is a date formatter, not a second money path.

harga per <tanggal> wording is "as of", not "last changed", matching the declined-imprecision note; the comment at page.tsx:165-167 records why. Rendered only where priceUpdatedAt is present (absent on both radiator-assy-rush and oli-mesin-10w40-bare). The Asia/Jakarta pin is doing real work: under TZ=UTC, 2026-08-14T13:30:00Z → 14 Agustus 2026 and 2026-08-14T18:00:00Z → 15 Agustus 2026.

Biome: apps/store/app/globals.css reports 2 warnings on this branch and 2 on origin/main (noImportantStyles at .product-type, noDescendingSpecificity at .image-placeholder) — measured in two separate worktrees, identical. All 7 changed files: 2 warnings, no new ones. Zero horizontal overflow on all 5 pages at both widths.

Findings

apps/store/app/globals.css:516: should: the comment says the .product-card .stock-badge co-selector is "Unreachable today — the card renders no ready chip", but I measured it load-bearing right now. product-card.tsx:52 renders <p class="stock-badge is-out"> / is-indent; those class pairs (0,2,0) do win the --chip cascade, but color: var(--chip) is declared only in the (0,1,0) selector list, so without the co-selector .product-card p (0,1,1) takes color and every card chip goes --ink-muted grey over its tint — reachable via is-out, not via a hypothetical ready. The shipped CSS is correct; only the justification is wrong, and "unreachable today" is precisely the reasoning that got this deferred once already. Replace that sentence with the measured reason — color is declared at (0,1,0) and loses to .product-card p, which the card renders today for habis and indent.

apps/store/app/cart/cart-view.tsx:68: nit: the cart line already annotates its other two flags inline (· Pasang di bengkel, · Ambil di bengkel) but not indent, so a buyer who added a back-order part days ago gets no reminder at the last screen before checkout. The shop is covered via the WhatsApp tag; this is the buyer half. One ternary in the same expression.

apps/store/app/globals.css:480: nit: pre-existing, not introduced here, but it is the same cascade defect one rule above the one you just fixed — .price (0,1,0) sets color: var(--ink) and loses to .product-card p (0,1,1), so every card price renders rgb(90,97,89) instead of --ink. Measured on the rendered grid. Same one-selector remedy if you want it in this pass; otherwise worth its own issue rather than leaving the chip fixed and the price not.

apps/store/app/globals.css:504: nit: the comment's contrast figures drift from the PR body's and from mine — 4.60/4.99/6.56/5.80 in the comment vs 4.61/5.00/6.54/5.79 measured. Rounding only, and the 12% conclusion is unaffected, but the comment is the copy a future maintainer will re-derive against.

VERDICT: approve-with-nits Reviewed at `b830657` in a throwaway `/tmp` detached worktree. Built `next build` (`output: "export"`) against my own 5-item `schemaVersion: 3` fixture served over HTTP on `127.0.0.1:8955` with `x-api-key`, covering `ready` / `indent` / `habis`, priced **and** unpriced `habis`, plus an item with `fitment: []`, `oemNumbers: []`, `partNumber: null`, `priceUpdatedAt: null`. Two of the five items carry a deliberately duplicated `fitment` / `oemNumbers` entry to exercise the dedup. Served `out/` and drove real Chromium at **1440x900** and **375x812**. The main checkout was never modified. Everything in the acceptance criteria holds. Nothing blocking. ### The eight things I was asked to state explicitly **1. `isLine` clause ordering — preserved, and the new clause bites.** `cart.ts:44-58`: `typeof line.qty === "number"` (47) → `Number.isInteger` (48) → `line.qty > 0` (49). The comparison is still *after* both guards; the price pair is `typeof` (51) then `Number.isFinite` (51). The `indent` clause is appended **last** at line 57, ahead of nothing. The documenting comment at `cart.ts:36-39` is still accurate, and I falsified both halves of it rather than reading it: deleting `typeof line.qty === "number"` outright leaves **66/66 green**, confirming the entailment it claims; and `Number.isInteger("5") === false`, `Number.isFinite("5") === false`, `"5" > 0 === true`, confirming the reorder hazard it warns about is real. Nothing was reordered ahead of a comparison. Mutation on the new clause reproduces the author's claim exactly — deleting only the `indent` clause: ``` not ok 21 - rejects a string indent not ok 22 - rejects a numeric indent not ok 3 - drops malformed entries but keeps the well-formed ones # tests 66 / # pass 63 / # fail 3 ``` Restored → 66/66. **2. No pre-existing cart case weakened.** `git diff origin/main...b830657` on `cart.test.ts` and `checkout-message.test.ts` produces **zero** removed lines — pure addition. Baseline measured independently in a second `origin/main` worktree: **56/56**. Branch: **66/66, 11 suites**. 56 + 10, none removed, as claimed. **3. Indent reaches the shop; a stocked line stays clean.** Real clicks on the real buttons at 375px, then the cart page's actual `href`, `text=` decoded — not the test fixture: ``` 1. Seal Kruk As Xenia (1) — Rp 85.000 [indent — pesan dulu] 2. Shockbreaker Depan Avanza (1) — Rp 450.000 ``` `localStorage` after the two clicks carried `"indent":true` and `"indent":false` respectively. Also checked two things the PR body did not claim: adding the indent item twice merges to `qty: 2` with `indent: true` intact (`add` spreads the existing line), and a hand-seeded pre-#47 line with the key **absent** survives `readStorage` and still renders at `/cart` — no cart is emptied on deploy. **4. Contrast — re-measured from the token hexes, WCAG 2.x sRGB, fill composited over the real backdrop.** My numbers match the PR body to the hundredth: | token | text on fill@12% over `--bg` | over `--surface` | @14% over `--bg` | |---|---|---|---| | `--ok` `#2d543c` | **6.54:1** | 7.09:1 | 6.33:1 | | `--warn` `#8a5a12` | **4.61:1** | 5.00:1 | 4.48:1 | | `--off` `#8c3a32` | **5.79:1** | 6.28:1 | 5.60:1 | The chip computes **12.48px / 700** — bold but under the 18.66px large-text threshold, so 4.5:1 is the correct target, and all three clear it on both backdrops. The 14% figure confirms the ceiling: `--warn` falls to 4.48:1, under AA. Treatment is applied consistently — all three states get the identical `--chip`-driven fill/border/radius; measured boxes 55x22 (ready), 58x22 (indent), 52x22 (habis) on the detail page and 52x25 / 58x25 on cards, `stretched: false` everywhere. No state is the only filled or only unfilled chip. **5. The `.product-card p` cascade fix is load-bearing and leak-free — but its comment is wrong.** Load-bearing *today*: I set `rule.selectorText = '.stock-badge'` via CSSOM on the rendered grid and all three card chips moved from `rgb(140,58,50)` / `rgb(138,90,18)` to `rgb(90,97,89)` (`--ink-muted`) while keeping the tinted fill; restoring the co-selector restored them. Leak-free: `.product-card .stock-badge` matches exactly the 3 elements that `.stock-badge` matches (a strict subset), and every non-badge `.product-card p` kept its expected colour. See the `should` below — the code comment's stated justification is the opposite of what I measured. **6. Empty blocks vanish cleanly on the rendered page.** `oli-mesin-10w40-bare`: no `section.fitment` in the DOM, `h2` list is `["Stok dan harga"]` only, `.price-note` list is `["Kirim ke alamat saya"]` only — no `Nomor OEM`, no `Nomor part`, no `harga per`, no orphan row, no stray separator. Confirmed visually: the description flows straight into the `Stok dan harga` eyebrow. Dedup works on the populated pages: fixture `["48510-BZ080","48510-BZ090","48510-BZ080"]` renders `48510-BZ080, 48510-BZ090`, and a duplicated fitment string collapses to one `<li>`. `.spec-code` computes `ui-monospace, Menlo, Consolas, monospace` on both rows. **7. JSON-LD, parsed out of the built HTML.** `Product` and `BreadcrumbList` on all five pages; all three states mapped; `Offer` only when priced: | slug | state | `offers.availability` | |---|---|---| | `shockbreaker-depan-avanza` | ready | `schema.org/InStock` | | `seal-kruk-as-xenia` | indent | `schema.org/BackOrder` | | `kampas-rem-belakang-innova` | habis, priced | `schema.org/OutOfStock` | | `radiator-assy-rush` | habis, **unpriced** | no `offers` key at all | | `oli-mesin-10w40-bare` | ready | `schema.org/InStock` | `radiator-assy-rush` also renders no detail-scoped `.price` element at either width. **8. CSS diff stayed out of #46's region.** #55's hunks land at 490-536 (`.stock-badge`) and 709-748 (`.availability-note h2, .fitment h2`, `.fitment`, `.spec-code`). I fetched `feat/46-faceted-filters`: its entire `globals.css` change is one hunk appended at **845+** adding `.browser*`, `.facet-*`, `.filter-*`, `.result-count`. Disjoint line ranges, disjoint selectors, no `.stock-badge` / `.fitment` / `.spec-code` / `.availability-note` anywhere in #54's CSS diff. These will merge without conflict. ### Also re-proved at this head (#41 not regressed) `availabilityBadgeClass` is **imported** from `product-card.tsx` by `page.tsx:12`, not forked. `add-to-cart.tsx:38-62` still early-returns on `habis` only, so the `indent: availability === "indent"` comment at line 69 is accurate. Rendered: `habis` (priced and unpriced) → add button `disabled: true`, no qty stepper, `Tanyakan ketersediaan` degrade; `indent` → add **enabled**; `ready` → add enabled. `formatIdr` is still the only price formatter — `priceDateFormat` is a date formatter, not a second money path. `harga per <tanggal>` wording is "as of", not "last changed", matching the declined-imprecision note; the comment at `page.tsx:165-167` records why. Rendered only where `priceUpdatedAt` is present (absent on both `radiator-assy-rush` and `oli-mesin-10w40-bare`). The `Asia/Jakarta` pin is doing real work: under `TZ=UTC`, `2026-08-14T13:30:00Z` → `14 Agustus 2026` and `2026-08-14T18:00:00Z` → `15 Agustus 2026`. Biome: `apps/store/app/globals.css` reports **2 warnings on this branch and 2 on `origin/main`** (`noImportantStyles` at `.product-type`, `noDescendingSpecificity` at `.image-placeholder`) — measured in two separate worktrees, identical. All 7 changed files: 2 warnings, no new ones. Zero horizontal overflow on all 5 pages at both widths. ### Findings apps/store/app/globals.css:516: should: the comment says the `.product-card .stock-badge` co-selector is "Unreachable today — the card renders no `ready` chip", but I measured it load-bearing *right now*. `product-card.tsx:52` renders `<p class="stock-badge is-out">` / `is-indent`; those class pairs (0,2,0) do win the `--chip` cascade, but `color: var(--chip)` is declared only in the (0,1,0) selector list, so without the co-selector `.product-card p` (0,1,1) takes `color` and every card chip goes `--ink-muted` grey over its tint — reachable via `is-out`, not via a hypothetical `ready`. The shipped CSS is correct; only the justification is wrong, and "unreachable today" is precisely the reasoning that got this deferred once already. Replace that sentence with the measured reason — `color` is declared at (0,1,0) and loses to `.product-card p`, which the card renders today for `habis` and `indent`. apps/store/app/cart/cart-view.tsx:68: nit: the cart line already annotates its other two flags inline (`· Pasang di bengkel`, `· Ambil di bengkel`) but not `indent`, so a buyer who added a back-order part days ago gets no reminder at the last screen before checkout. The shop is covered via the WhatsApp tag; this is the buyer half. One ternary in the same expression. apps/store/app/globals.css:480: nit: pre-existing, not introduced here, but it is the *same* cascade defect one rule above the one you just fixed — `.price` (0,1,0) sets `color: var(--ink)` and loses to `.product-card p` (0,1,1), so every card price renders `rgb(90,97,89)` instead of `--ink`. Measured on the rendered grid. Same one-selector remedy if you want it in this pass; otherwise worth its own issue rather than leaving the chip fixed and the price not. apps/store/app/globals.css:504: nit: the comment's contrast figures drift from the PR body's and from mine — `4.60`/`4.99`/`6.56`/`5.80` in the comment vs `4.61`/`5.00`/`6.54`/`5.79` measured. Rounding only, and the 12% conclusion is unaffected, but the comment is the copy a future maintainer will re-derive against.
Review findings on #55:

- globals.css: the .product-card .stock-badge co-selector was justified as
  guarding an unreachable case. It is load-bearing today — the card renders
  its chip as a <p class="stock-badge is-out">/is-indent, and those pairs win
  --chip without declaring color. Comment corrected; "unreachable today" is
  the reasoning that deferred this once already.
- globals.css: contrast figures re-derived from the token hexes — 4.61/5.00
  (--warn), 6.54 (--ok), 5.79 (--off), 4.48 at 14%. Also states why the AA
  target is 4.5:1 rather than 3:1.
- cart-view.tsx: annotate an indent line for the buyer, matching the two
  fulfilment flags already shown there. The WhatsApp tag covers the shop;
  this is the last screen before checkout for the buyer.
yuris commented 2026-08-27 13:13:23 +00:00
Author
Owner
Copy link

Thanks — the review's own measurements caught something my PR body got wrong, and I've taken the should plus two of the three nits. New head is 3daea4b.

Note this push voids review 14's verdict. Re-review requested against 3daea4b.

should — globals.css:516 cascade justification: FIXED

You are right, and this is the one finding I'd have missed by reading my own comment instead of the DOM. My own composition audit had already measured that card chips are <p class="stock-badge is-out"> and that the co-selector is load-bearing now — so my comment and my evidence disagreed and I shipped the comment. Your diagnosis of the mechanism is more precise than mine was: the .is-* pairs are (0,2,0) and do win --chip, but they never declare color, so color is only ever set by the (0,1,0) selector list and that is what .product-card p (0,1,1) beats. Reached through is-out/is-indent, never through a ready chip the card deliberately never renders.

Rewritten to say exactly that, and to drop "unreachable today" — which, as you note, is the reasoning that got this deferred once already.

nit — globals.css:504 contrast figures: FIXED

Corrected to the measured values. Your numbers, mine, and the PR body's table now all agree: --warn 4.61:1 on --bg / 5.00:1 on --surface, --ok 6.54:1, --off 5.79:1, and 4.48:1 at 14%. The comment now also records why the target is 4.5:1 and not 3:1 — the chip computes to 12.48px/700, bold but under the 18.66px large-text threshold — since that is the load-bearing part of the 12% decision and the old comment left it implicit.

nit — cart-view.tsx:68 buyer-side indent annotation: TAKEN

Fair, and it closes the buyer half of the same criterion. One ternary in the same expression, ordered ahead of the fulfilment flags to match the WhatsApp tag order:

{line.indent ? " · Indent, dipesan dulu" : null}

Verified in a real browser at 375px, driving the actual add buttons and then reading the rendered cart:

cart line rendered meta
indent, shippable Rp 85.000 · Indent, dipesan dulu
indent + installAtShop Rp 85.000 · Indent, dipesan dulu · Pasang di bengkel
indent + pickup-only Rp 310.000 · Indent, dipesan dulu · Ambil di bengkel
stocked Rp 450.000
pre-#47 line, key absent Rp 62.000 — no annotation, and the line still renders

The same three carts produce the matching WhatsApp text, so the two surfaces agree rather than drifting:

1. Indent + pasang (1) — Rp 85.000 [indent — pesan dulu, pasang di tempat]
2. Indent + ambil (2) — Rp 310.000 [indent — pesan dulu, ambil di bengkel]
3. Stok, kirim (1) — Rp 450.000

nit — globals.css:480 .price in cards renders --ink-muted: DECLINED, filed instead

Confirmed real — I reproduced it, and it is genuinely the same defect class one rule above the one I fixed. Declining it in this pass anyway, for two reasons rather than convenience:

  1. It is pre-existing on 55f5d72 and outside #47. Every card price in the store changes colour, on /, /products, /services/<slug> and the related grid — a store-wide visual change landing in a PR titled "product detail", where nobody reviewing #47's acceptance criteria would think to look for it.
  2. /products is owned by #46 (PR #54) right now, and it is the page where this is most visible. A store-wide card restyle from me, mid-flight, is exactly the composition hazard Main had us cross-check for.

Your framing — "worth its own issue rather than leaving the chip fixed and the price not" — is the right one, so I'm flagging it to Main to file rather than silently dropping it. It is a one-selector fix (.price, .product-card .price) with the identical shape to mine, and it should be measured on /products after both #54 and #55 land.

Re-verification at 3daea4b

$ git rev-parse HEAD
3daea4b2f5fc551fac6e247d3132fe198074577d

$ pnpm --filter garage-store test
# tests 66
# suites 11
# pass 66
# fail 0
# skipped 0
# todo 0

$ pnpm --filter garage-store build
✓ Compiled successfully in 4.6s
  Finished TypeScript in 2.4s ...
✓ Generating static pages using 13 workers (21/21) in 685ms

$ pnpm exec biome check <the 8 changed files>
Checked 8 files in 14ms. No fixes applied.
Found 2 warnings.

Still the same 2 pre-existing globals.css warnings (noImportantStyles at .product-type, noDescendingSpecificity at .image-placeholder), unchanged in count from origin/main. Diff is now 8 files, +236/−19. Nothing else moved: the CSS change is still confined to .stock-badge / .fitment / .spec-code / .availability-note h2, still disjoint from #54's appended .browser* / .facet-* / .filter-* block.

Nothing in this push touches isLine, checkout-message.ts, add-to-cart.tsx or page.tsx, so your items 1, 2, 3, 6 and 7 are unaffected by it. Item 5's code is unchanged too — only its comment.

Composition cross-check with #54, since it post-dates the review

Main asked for this separately and it is worth recording on the PR. I composed b830657 + #54's 7b7a7e5 in a throwaway local branch (never pushed, since deleted), built, and measured:

  • Merge auto-resolved, no conflict. Composed suite 90/90 (my 66 + #54's 24). TypeScript clean, 21/21 pages.
  • Enumerated every selector in the composed sheet, took the 22 rules in #54's region, and ran element.matches() against all 138–158 elements on each of my four detail pages: zero matches, all four.
  • All my computed values are byte-identical composed vs. branch-alone at both widths. Critically is-indent resolves --warn rgb(138,90,18), not --off — the #52/#53 wave-1 bug does not recur.
  • Walking up from each chip to its first non-transparent ancestor found two backdrops, not one: .product-card.is-out { background: transparent } (pre-existing on 55f5d72, byte-identical on all three refs) leaves a Habis card unfilled, so its chip composites over --bg, while a normal card's chip composites over --surface. Both were already in the measured set — --off on --bg 5.79:1, --warn on --surface 5.00:1. #54 introduces no third surface.

Closes #47 is unchanged, and merging this PR deploys to production — no workflow runs on pull_request; both .forgejo/workflows/*.yml fire only on push: main and they deploy.

Thanks — the review's own measurements caught something my PR body got wrong, and I've taken the `should` plus two of the three nits. New head is `3daea4b`. **Note this push voids review 14's verdict.** Re-review requested against `3daea4b`. ## `should` — globals.css:516 cascade justification: FIXED You are right, and this is the one finding I'd have missed by reading my own comment instead of the DOM. My own composition audit had already measured that card chips are `<p class="stock-badge is-out">` and that the co-selector is load-bearing *now* — so my comment and my evidence disagreed and I shipped the comment. Your diagnosis of the mechanism is more precise than mine was: the `.is-*` pairs are (0,2,0) and do win `--chip`, but they never declare `color`, so `color` is only ever set by the (0,1,0) selector list and that is what `.product-card p` (0,1,1) beats. Reached through `is-out`/`is-indent`, never through a `ready` chip the card deliberately never renders. Rewritten to say exactly that, and to drop "unreachable today" — which, as you note, is the reasoning that got this deferred once already. ## nit — globals.css:504 contrast figures: FIXED Corrected to the measured values. Your numbers, mine, and the PR body's table now all agree: `--warn` 4.61:1 on `--bg` / 5.00:1 on `--surface`, `--ok` 6.54:1, `--off` 5.79:1, and 4.48:1 at 14%. The comment now also records *why* the target is 4.5:1 and not 3:1 — the chip computes to 12.48px/700, bold but under the 18.66px large-text threshold — since that is the load-bearing part of the 12% decision and the old comment left it implicit. ## nit — cart-view.tsx:68 buyer-side indent annotation: TAKEN Fair, and it closes the buyer half of the same criterion. One ternary in the same expression, ordered ahead of the fulfilment flags to match the WhatsApp tag order: ``` {line.indent ? " · Indent, dipesan dulu" : null} ``` Verified in a real browser at 375px, driving the actual add buttons and then reading the rendered cart: | cart line | rendered meta | |---|---| | indent, shippable | `Rp 85.000 · Indent, dipesan dulu` | | indent + `installAtShop` | `Rp 85.000 · Indent, dipesan dulu · Pasang di bengkel` | | indent + pickup-only | `Rp 310.000 · Indent, dipesan dulu · Ambil di bengkel` | | stocked | `Rp 450.000` | | **pre-#47 line, key absent** | `Rp 62.000` — no annotation, and the line still renders | The same three carts produce the matching WhatsApp text, so the two surfaces agree rather than drifting: ``` 1. Indent + pasang (1) — Rp 85.000 [indent — pesan dulu, pasang di tempat] 2. Indent + ambil (2) — Rp 310.000 [indent — pesan dulu, ambil di bengkel] 3. Stok, kirim (1) — Rp 450.000 ``` ## nit — globals.css:480 `.price` in cards renders `--ink-muted`: DECLINED, filed instead Confirmed real — I reproduced it, and it is genuinely the same defect class one rule above the one I fixed. Declining it in this pass anyway, for two reasons rather than convenience: 1. It is **pre-existing on `55f5d72`** and outside #47. Every card price in the store changes colour, on `/`, `/products`, `/services/<slug>` and the related grid — a store-wide visual change landing in a PR titled "product detail", where nobody reviewing #47's acceptance criteria would think to look for it. 2. `/products` is **owned by #46 (PR #54) right now**, and it is the page where this is most visible. A store-wide card restyle from me, mid-flight, is exactly the composition hazard Main had us cross-check for. Your framing — "worth its own issue rather than leaving the chip fixed and the price not" — is the right one, so I'm flagging it to Main to file rather than silently dropping it. It is a one-selector fix (`.price, .product-card .price`) with the identical shape to mine, and it should be measured on `/products` after both #54 and #55 land. ## Re-verification at `3daea4b` ``` $ git rev-parse HEAD 3daea4b2f5fc551fac6e247d3132fe198074577d $ pnpm --filter garage-store test # tests 66 # suites 11 # pass 66 # fail 0 # skipped 0 # todo 0 $ pnpm --filter garage-store build ✓ Compiled successfully in 4.6s Finished TypeScript in 2.4s ... ✓ Generating static pages using 13 workers (21/21) in 685ms $ pnpm exec biome check <the 8 changed files> Checked 8 files in 14ms. No fixes applied. Found 2 warnings. ``` Still the same 2 pre-existing `globals.css` warnings (`noImportantStyles` at `.product-type`, `noDescendingSpecificity` at `.image-placeholder`), unchanged in count from `origin/main`. Diff is now 8 files, +236/−19. Nothing else moved: the CSS change is still confined to `.stock-badge` / `.fitment` / `.spec-code` / `.availability-note h2`, still disjoint from #54's appended `.browser*` / `.facet-*` / `.filter-*` block. Nothing in this push touches `isLine`, `checkout-message.ts`, `add-to-cart.tsx` or `page.tsx`, so your items 1, 2, 3, 6 and 7 are unaffected by it. Item 5's *code* is unchanged too — only its comment. ### Composition cross-check with #54, since it post-dates the review Main asked for this separately and it is worth recording on the PR. I composed `b830657` + #54's `7b7a7e5` in a throwaway local branch (never pushed, since deleted), built, and measured: - Merge auto-resolved, no conflict. Composed suite **90/90** (my 66 + #54's 24). TypeScript clean, 21/21 pages. - Enumerated every selector in the composed sheet, took the 22 rules in #54's region, and ran `element.matches()` against all 138–158 elements on each of my four detail pages: **zero matches**, all four. - All my computed values are byte-identical composed vs. branch-alone at both widths. Critically `is-indent` resolves `--warn` `rgb(138,90,18)`, **not** `--off` — the #52/#53 wave-1 bug does not recur. - Walking up from each chip to its first non-transparent ancestor found **two** backdrops, not one: `.product-card.is-out { background: transparent }` (pre-existing on `55f5d72`, byte-identical on all three refs) leaves a Habis card unfilled, so its chip composites over `--bg`, while a normal card's chip composites over `--surface`. Both were already in the measured set — `--off` on `--bg` 5.79:1, `--warn` on `--surface` 5.00:1. #54 introduces no third surface. `Closes #47` is unchanged, and **merging this PR deploys to production** — no workflow runs on `pull_request`; both `.forgejo/workflows/*.yml` fire only on `push: main` and they deploy.
yuris reviewed 2026-08-27 14:05:35 +00:00
yuris left a comment
Copy link

VERDICT: approve-with-nits

Round-2 review at 3daea4b, fresh throwaway /tmp worktree, rebuilt and re-driven. The two items you took are correct and I verified both independently rather than reading the diff. The decline is fair and I accept it. One new finding, on the surface this push added — and it is one my round-1 review should have caught and did not, because I verified the cart's data and its WhatsApp output but never screenshotted the cart page itself at 375px.

The delta, verified independently

globals.css is comment-only. I stripped every /* … */ block from b830657:apps/store/app/globals.css and 3daea4b:… and compared the remainder: byte-identical, 823 rule lines each (raw 901 → 906). Same result Author46b reported, reached separately. Zero selectors, zero declarations changed.

The seven files I based items 1, 2, 3, 5, 6 and 7 on are byte-identical, checked by git object hash, not by taking your word: lib/cart.ts, lib/cart.test.ts, lib/checkout-message.ts, lib/checkout-message.test.ts, [slug]/add-to-cart.tsx, [slug]/page.tsx, products/product-card.tsx. Re-ran the isLine mutation at this head anyway — deleting only the indent clause still turns exactly rejects a string indent, rejects a numeric indent and drops malformed entries but keeps the well-formed ones red, 63/66, restored to 66/66.

should at :516 — fixed, and the new text matches what I measured. Re-proved load-bearing at this head on a 7-item grid: stripping the co-selector via CSSOM moved all five card chips from rgb(138,90,18)/rgb(140,58,50) to rgb(90,97,89), restoring brought them back; .product-type unaffected, so still leak-free. The rewritten mechanism is right, including the part I did not spell out: the .is-* pairs win --chip but never declare color. "Unreachable today" is gone.

nit at :504 — fixed, and every number now reconciles. Independently recomputed: --warn 4.61 / 5.00, 4.48 at 14%, --ok 6.54, --off 5.79. All four match. I also checked the border claim you did not change — 1.65-1.82:1 is border@35% against the backdrop (min --warn over --bg 1.65, max --ok over --surface 1.82), which is derivable and correct; it is not the border-vs-fill figure (1.40-1.51), so the wording "decorative at 35%" is the right reading. The added 12.48px/700 vs 18.66px rationale is confirmed on the rendered chip: every chip on all 7 pages at both widths computes exactly 12.48px/700, 999px.

nit at :68 — taken, and it behaves. Built a 7-item fixture including a new indent+installService: "kaki-kaki" item and a new indent+shippable: false item, so all three tag combinations come from real add-button clicks and the real "Pasang di bengkel" radio, not from seeding. Then appended a pre-#47 line with the key absent (the only case seeding is required for). Rendered cart:

line rendered meta
indent, shippable Rp 85.000 · Indent, dipesan dulu
indent + installAtShop Rp 275.000 · Indent, dipesan dulu · Pasang di bengkel
indent + pickup-only Rp 310.000 · Indent, dipesan dulu · Ambil di bengkel
stocked Rp 450.000
pre-#47, key absent Rp 62.000 — no annotation, line not dropped

and the same cart's real wa.me link agrees line for line, indent-first in both:

1. Seal Kruk As Xenia (1) — Rp 85.000 [indent — pesan dulu]
2. Per Keong Indent Pasang (1) — Rp 275.000 [indent — pesan dulu, pasang di tempat]
3. Bumper Depan Indent Ambil (1) — Rp 310.000 [indent — pesan dulu, ambil di bengkel]
4. Shockbreaker Depan Avanza (1) — Rp 450.000
5. Oli Mesin 10W-40 (1) — Rp 62.000

nit at :480 — decline accepted. Your two reasons are the right ones and the second is stronger than my framing was: /products is where it is most visible and it is #46's page mid-flight. I confirmed the defect is still there (.price in a card computes rgb(90,97,89)) and I am not re-raising it. Please make sure it is actually filed, together with the new one below — it is more urgent than this one.

Nothing else regressed at this head. 66/66, 11 suites. Build compiles, TypeScript clean, 23/23 pages on my 7-item fixture. JSON-LD across all 7: ready→InStock, all three indent→BackOrder, habis priced→OutOfStock, habis unpriced→no offers key; Product + BreadcrumbList on every page. Empty-state still clean on oli-mesin-10w40-bare (no section.fitment, h2 list is ["Stok dan harga"] alone, no OEM/part/harga per). Dedup still collapses the duplicated OEM and fitment entries. Biome on the now-8 changed files: the same 2 pre-existing globals.css warnings. Zero horizontal overflow on 7 pages × 2 widths.

Merge safety re-checked against #54's current head, not the one you composed with. #54 has moved to 6fece92. git merge-tree --write-tree 3daea4b 6fece92 exits 0, no conflict. In the composed tree: all 6 of #47's selectors survive intact, all 22 of #54's region selectors present, zero overlap between the two selector sets.

Findings

apps/store/app/cart/cart-view.tsx:69: should: the annotation is unreadable at every common phone width, because .cart-line's middle column collapses. .cart-line is grid-template-columns: var(--s16) minmax(0, 1fr) auto and the auto controls column takes max-content, so the name/meta column measures 875px at 1440, 91px at 480, 25px at 414, and 0px at 390, 375, 360 and 320. At 375px the new text does not merely wrap — it stacks one fragment per line under the qty stepper and the Hapus button, and the row grows 123px → 195px with one tag and 267px with two, while a stocked row stays 118px. Screenshotted. The overlap itself is pre-existing: I rendered a stocked-only line, whose DOM is identical to origin/main, and the product name already collides with the − button and Hapus already sits on top of Rp 85.000 — that ships in production today and this PR did not cause it. But this PR is what puts buyer-facing reassurance copy into that cell, so as shipped the annotation is legible only above ~480px, and the store's buyers are on phones. Your call which way to go, and both are defensible: fix .cart-line, or accept the annotation as desktop-only for now. What is not defensible is dropping it silently. If you fix it, I verified this remedy at 375px — name column 0px → 263px, meta back to a single 18px line, all rows uniform at 157px, no overflow: @media (max-width: 480px) { .cart-line { grid-template-columns: var(--s16) minmax(0, 1fr); } .cart-line > div:last-child { grid-column: 1 / -1; } }. Unlike the .price nit there is no collision hazard here — #49 is merged rather than in flight, and #46 owns the filter sidebar and /products, not the cart. If you would rather not widen this PR, file it and say so on the PR; I will not hold the verdict over it.

VERDICT: approve-with-nits Round-2 review at `3daea4b`, fresh throwaway `/tmp` worktree, rebuilt and re-driven. The two items you took are correct and I verified both independently rather than reading the diff. The decline is fair and I accept it. One **new** finding, on the surface this push added — and it is one my round-1 review should have caught and did not, because I verified the cart's *data* and its WhatsApp output but never screenshotted the cart page itself at 375px. ### The delta, verified independently **globals.css is comment-only.** I stripped every `/* … */` block from `b830657:apps/store/app/globals.css` and `3daea4b:…` and compared the remainder: **byte-identical, 823 rule lines each** (raw 901 → 906). Same result Author46b reported, reached separately. Zero selectors, zero declarations changed. **The seven files I based items 1, 2, 3, 5, 6 and 7 on are byte-identical**, checked by git object hash, not by taking your word: `lib/cart.ts`, `lib/cart.test.ts`, `lib/checkout-message.ts`, `lib/checkout-message.test.ts`, `[slug]/add-to-cart.tsx`, `[slug]/page.tsx`, `products/product-card.tsx`. Re-ran the `isLine` mutation at this head anyway — deleting only the `indent` clause still turns exactly `rejects a string indent`, `rejects a numeric indent` and `drops malformed entries but keeps the well-formed ones` red, 63/66, restored to 66/66. **`should` at :516 — fixed, and the new text matches what I measured.** Re-proved load-bearing at this head on a 7-item grid: stripping the co-selector via CSSOM moved all five card chips from `rgb(138,90,18)`/`rgb(140,58,50)` to `rgb(90,97,89)`, restoring brought them back; `.product-type` unaffected, so still leak-free. The rewritten mechanism is right, including the part I did not spell out: the `.is-*` pairs win `--chip` but never declare `color`. "Unreachable today" is gone. **nit at :504 — fixed, and every number now reconciles.** Independently recomputed: `--warn` 4.61 / 5.00, 4.48 at 14%, `--ok` 6.54, `--off` 5.79. All four match. I also checked the border claim you did not change — `1.65-1.82:1` is border@35% against the *backdrop* (min `--warn` over `--bg` 1.65, max `--ok` over `--surface` 1.82), which is derivable and correct; it is not the border-vs-fill figure (1.40-1.51), so the wording "decorative at 35%" is the right reading. The added 12.48px/700 vs 18.66px rationale is confirmed on the rendered chip: **every** chip on all 7 pages at both widths computes exactly `12.48px/700`, `999px`. **nit at :68 — taken, and it behaves.** Built a 7-item fixture including a new indent+`installService: "kaki-kaki"` item and a new indent+`shippable: false` item, so all three tag combinations come from **real add-button clicks** and the real "Pasang di bengkel" radio, not from seeding. Then appended a pre-#47 line with the key absent (the only case seeding is required for). Rendered cart: | line | rendered meta | |---|---| | indent, shippable | `Rp 85.000 · Indent, dipesan dulu` | | indent + installAtShop | `Rp 275.000 · Indent, dipesan dulu · Pasang di bengkel` | | indent + pickup-only | `Rp 310.000 · Indent, dipesan dulu · Ambil di bengkel` | | stocked | `Rp 450.000` | | pre-#47, key absent | `Rp 62.000` — no annotation, line not dropped | and the same cart's real `wa.me` link agrees line for line, indent-first in both: ``` 1. Seal Kruk As Xenia (1) — Rp 85.000 [indent — pesan dulu] 2. Per Keong Indent Pasang (1) — Rp 275.000 [indent — pesan dulu, pasang di tempat] 3. Bumper Depan Indent Ambil (1) — Rp 310.000 [indent — pesan dulu, ambil di bengkel] 4. Shockbreaker Depan Avanza (1) — Rp 450.000 5. Oli Mesin 10W-40 (1) — Rp 62.000 ``` **nit at :480 — decline accepted.** Your two reasons are the right ones and the second is stronger than my framing was: `/products` is where it is most visible and it is #46's page mid-flight. I confirmed the defect is still there (`.price` in a card computes `rgb(90,97,89)`) and I am not re-raising it. Please make sure it is actually filed, together with the new one below — it is more urgent than this one. **Nothing else regressed at this head.** 66/66, 11 suites. Build compiles, TypeScript clean, 23/23 pages on my 7-item fixture. JSON-LD across all 7: `ready`→`InStock`, all three `indent`→`BackOrder`, `habis` priced→`OutOfStock`, `habis` unpriced→**no `offers` key**; `Product` + `BreadcrumbList` on every page. Empty-state still clean on `oli-mesin-10w40-bare` (no `section.fitment`, `h2` list is `["Stok dan harga"]` alone, no OEM/part/`harga per`). Dedup still collapses the duplicated OEM and fitment entries. Biome on the now-8 changed files: the same 2 pre-existing `globals.css` warnings. Zero horizontal overflow on 7 pages × 2 widths. **Merge safety re-checked against #54's *current* head, not the one you composed with.** #54 has moved to `6fece92`. `git merge-tree --write-tree 3daea4b 6fece92` exits 0, no conflict. In the composed tree: all 6 of #47's selectors survive intact, all 22 of #54's region selectors present, **zero overlap** between the two selector sets. ### Findings apps/store/app/cart/cart-view.tsx:69: should: the annotation is unreadable at every common phone width, because `.cart-line`'s middle column collapses. `.cart-line` is `grid-template-columns: var(--s16) minmax(0, 1fr) auto` and the `auto` controls column takes max-content, so the name/meta column measures 875px at 1440, 91px at 480, 25px at 414, and **0px at 390, 375, 360 and 320**. At 375px the new text does not merely wrap — it stacks one fragment per line under the qty stepper and the `Hapus` button, and the row grows 123px → 195px with one tag and 267px with two, while a stocked row stays 118px. Screenshotted. **The overlap itself is pre-existing**: I rendered a stocked-only line, whose DOM is identical to `origin/main`, and the product name already collides with the `−` button and `Hapus` already sits on top of `Rp 85.000` — that ships in production today and this PR did not cause it. But this PR is what puts buyer-facing reassurance copy into that cell, so as shipped the annotation is legible only above ~480px, and the store's buyers are on phones. Your call which way to go, and both are defensible: fix `.cart-line`, or accept the annotation as desktop-only for now. What is not defensible is dropping it silently. If you fix it, I verified this remedy at 375px — name column 0px → 263px, meta back to a single 18px line, all rows uniform at 157px, no overflow: `@media (max-width: 480px) { .cart-line { grid-template-columns: var(--s16) minmax(0, 1fr); } .cart-line > div:last-child { grid-column: 1 / -1; } }`. Unlike the `.price` nit there is no collision hazard here — #49 is merged rather than in flight, and #46 owns the filter sidebar and `/products`, not the cart. If you would rather not widen this PR, file it and say so on the PR; I will not hold the verdict over it.
The cart line is a three-column grid whose controls column is `auto`, so its
max-content width holds ~261px regardless of viewport and squeezes name and
meta to 25px at 414px and 0px at 390px and below. The text then overflows its
own zero-width column onto the "-" button.

The collapse predates this issue — a stocked line already overlaps at 375px —
but the indent annotation is what made it matter, since the point of that line
is that a buyer reads it before checkout.

Below 600px the thumb and text keep row one and the controls span row two.
600px is where the three-column layout runs out of room, measured: it needs
~604px to hold the meta on one line.
yuris commented 2026-08-27 14:11:11 +00:00
Author
Owner
Copy link

Round-2 should taken. New head 8f160d1. This push voids review 15's verdict; re-review requested against 8f160d1.

The cart-line collapse: FIXED, not filed

I reproduced it before deciding, and it is worse than the summary sounds — so thank you for going back and screenshotting the page you'd only measured the data for. At 375px the middle column computes to exactly 0px, and the name and meta then overflow their own zero-width column on top of the − button. Measured on the pre-fix build:

viewport middle column meta lines row heights
1440px 875px 1 97 / 97 / 97
620px 231px 1 97 / 97 / 97
580px 191px 2 97 / 107 / 97
480px 91px 3 125 / 195 / 106
414px 25px 5 195 / 267 / 106
390px 0.75px 5 195 / 267 / 106
375px 0px 5 195 / 267 / 118

Your read on ownership is right — .cart-line is pre-existing, #49 is merged rather than in flight, and #46 owns /products, not the cart — so there is no collision hazard.

I fixed rather than filed, and the reason matters: unlike the .price nit I declined, this one makes the line I just added unreadable. The annotation exists so a buyer sees the back-order before checking out; at 375px, which for this shop is the dominant width, it rendered as one word per line underneath the Hapus button. A criterion that is satisfied only above 480px is not satisfied. The pre-existing overlap getting fixed alongside it is a consequence of the same one media query, not scope I went looking for.

I moved the breakpoint from your 480px to 600px, on measurement. 480px is below the point where this actually breaks: the meta already wraps to two lines at 580px and to three at 480px. The three-column layout needs roughly 604px to keep the meta on one line — controls ~261px, meta ~215px, thumb 64px, gaps 32px, page padding 32px — and measurement matches, one line down to 620px, wrapping at 580px. So 600px is where the layout runs out of room to do its job, rather than where it becomes actively illegible. The comment records that arithmetic.

@media (max-width: 600px) {
  .cart-line {
    grid-template-columns: var(--s16) minmax(0, 1fr);
  }
  .cart-line > :last-child {
    grid-column: 1 / -1;
  }
}

After, same rig

viewport middle column meta lines name overflows meta overflows rows
1440px 875px 1 no no 97 / 97 / 97
720px 315px 1 no no 97 / 97 / 97
620px 231px 1 no no 97 / 97 / 97
600px 488px 1 no no 157 / 157 / 157
480px 368px 1 no no 157 / 157 / 157
414px 302px 1 no no 157 / 157 / 157
390px 278px 1 no no 157 / 157 / 157
375px 263px 1 no no 157 / 157 / 157
320px 208px 1 no no 157 / 167 / 157

The meta holds one line at every width tested, 320px to 1440px. Zero horizontal overflow throughout. Nothing at or above 620px changed — same columns, same 97px rows — so the desktop layout you approved is untouched, confirmed visually at 1440px.

Rp 85.000 · Indent, dipesan dulu now reads as one line at 375px, and the two-tag line reads Rp 310.000 · Indent, dipesan dulu · Pasang di bengkel across two clean lines instead of five fragments. The pre-existing name/− collision is gone as a side effect.

Re-verification at 8f160d1

$ git rev-parse HEAD
8f160d18b69deed25b2f2ab42d9a61d65744671d

$ pnpm --filter garage-store test
# tests 66
# suites 11
# pass 66
# fail 0

$ pnpm --filter garage-store build
✓ Compiled successfully in 2.8s
  Finished TypeScript in 2.5s ...
✓ Generating static pages using 13 workers (21/21) in 690ms

$ pnpm exec biome check <the 8 changed files>
Checked 8 files in 18ms. No fixes applied.
Found 2 warnings.

Still only the 2 pre-existing globals.css warnings, unchanged in count from origin/main. Diff is 8 files, +262/−19.

This push is one appended media query in globals.css and nothing else — no existing rule, selector or declaration was modified, and no other file was touched. So every item you verified in rounds 1 and 2 stands: isLine ordering and its mutation, the seven byte-identical files, the co-selector, the contrast figures, the chip treatment, the empty-block behaviour, the JSON-LD table, and the cart's data and WhatsApp output. The only new surface is the sub-600px cart layout.

The .price decline stands, and I've asked Main again to file it — flagged in three separate messages now, so it is on the record rather than dropped.

Merging this PR deploys to production: no workflow runs on pull_request, and both .forgejo/workflows/*.yml fire only on push: main and deploy.

Round-2 `should` taken. New head `8f160d1`. **This push voids review 15's verdict; re-review requested against `8f160d1`.** ## The cart-line collapse: FIXED, not filed I reproduced it before deciding, and it is worse than the summary sounds — so thank you for going back and screenshotting the page you'd only measured the data for. At 375px the middle column computes to **exactly 0px**, and the name and meta then overflow their own zero-width column on top of the `−` button. Measured on the pre-fix build: | viewport | middle column | meta lines | row heights | |---|---|---|---| | 1440px | 875px | 1 | 97 / 97 / 97 | | 620px | 231px | 1 | 97 / 97 / 97 | | 580px | 191px | 2 | 97 / 107 / 97 | | 480px | 91px | 3 | 125 / 195 / 106 | | 414px | 25px | 5 | 195 / 267 / 106 | | 390px | **0.75px** | 5 | 195 / 267 / 106 | | 375px | **0px** | 5 | 195 / 267 / 118 | Your read on ownership is right — `.cart-line` is pre-existing, #49 is merged rather than in flight, and #46 owns `/products`, not the cart — so there is no collision hazard. I fixed rather than filed, and the reason matters: unlike the `.price` nit I declined, this one makes **the line I just added** unreadable. The annotation exists so a buyer sees the back-order before checking out; at 375px, which for this shop is the dominant width, it rendered as one word per line underneath the Hapus button. A criterion that is satisfied only above 480px is not satisfied. The pre-existing overlap getting fixed alongside it is a consequence of the same one media query, not scope I went looking for. **I moved the breakpoint from your 480px to 600px, on measurement.** 480px is below the point where this actually breaks: the meta already wraps to two lines at 580px and to three at 480px. The three-column layout needs roughly 604px to keep the meta on one line — controls ~261px, meta ~215px, thumb 64px, gaps 32px, page padding 32px — and measurement matches, one line down to 620px, wrapping at 580px. So 600px is where the layout runs out of room to do its job, rather than where it becomes actively illegible. The comment records that arithmetic. ```css @media (max-width: 600px) { .cart-line { grid-template-columns: var(--s16) minmax(0, 1fr); } .cart-line > :last-child { grid-column: 1 / -1; } } ``` ### After, same rig | viewport | middle column | meta lines | name overflows | meta overflows | rows | |---|---|---|---|---|---| | 1440px | 875px | 1 | no | no | 97 / 97 / 97 | | 720px | 315px | 1 | no | no | 97 / 97 / 97 | | 620px | 231px | 1 | no | no | 97 / 97 / 97 | | 600px | 488px | 1 | no | no | 157 / 157 / 157 | | 480px | 368px | 1 | no | no | 157 / 157 / 157 | | 414px | 302px | 1 | no | no | 157 / 157 / 157 | | 390px | 278px | 1 | no | no | 157 / 157 / 157 | | 375px | **263px** | 1 | no | no | 157 / 157 / 157 | | 320px | 208px | 1 | no | no | 157 / 167 / 157 | The meta holds one line at **every** width tested, 320px to 1440px. Zero horizontal overflow throughout. Nothing at or above 620px changed — same columns, same 97px rows — so the desktop layout you approved is untouched, confirmed visually at 1440px. `Rp 85.000 · Indent, dipesan dulu` now reads as one line at 375px, and the two-tag line reads `Rp 310.000 · Indent, dipesan dulu · Pasang di bengkel` across two clean lines instead of five fragments. The pre-existing name/`−` collision is gone as a side effect. ## Re-verification at `8f160d1` ``` $ git rev-parse HEAD 8f160d18b69deed25b2f2ab42d9a61d65744671d $ pnpm --filter garage-store test # tests 66 # suites 11 # pass 66 # fail 0 $ pnpm --filter garage-store build ✓ Compiled successfully in 2.8s Finished TypeScript in 2.5s ... ✓ Generating static pages using 13 workers (21/21) in 690ms $ pnpm exec biome check <the 8 changed files> Checked 8 files in 18ms. No fixes applied. Found 2 warnings. ``` Still only the 2 pre-existing `globals.css` warnings, unchanged in count from `origin/main`. Diff is 8 files, +262/−19. This push is **one appended media query in `globals.css`** and nothing else — no existing rule, selector or declaration was modified, and no other file was touched. So every item you verified in rounds 1 and 2 stands: `isLine` ordering and its mutation, the seven byte-identical files, the co-selector, the contrast figures, the chip treatment, the empty-block behaviour, the JSON-LD table, and the cart's data and WhatsApp output. The only new surface is the sub-600px cart layout. The `.price` decline stands, and I've asked Main again to file it — flagged in three separate messages now, so it is on the record rather than dropped. Merging this PR deploys to production: no workflow runs on `pull_request`, and both `.forgejo/workflows/*.yml` fire only on `push: main` and deploy.
yuris reviewed 2026-08-27 14:18:02 +00:00
yuris left a comment
Copy link

VERDICT: request-changes

Round-3 review at 8f160d1. The layout fix is correct and your 600px reasoning is better than my 480px was — I checked it and you are right, so I am not arguing the breakpoint. But this push moved #47's CSS to the one line in the file it could not safely occupy, and it turned a merge I verified clean in round 2 into a conflict whose obvious resolution silently breaks both regions. That is the whole of the blocker; the fix is a relocation with zero behavioural change and I have already proven it merges clean.

The blocker, measured

Round 2 I reported git merge-tree --write-tree 3daea4b 6fece92 → exit 0, no conflict. At this head, against the same #54 commit:

merge 3daea4b + 6fece92 -> exit=0
merge 8f160d1 + 6fece92 -> exit=1   CONFLICT (content): apps/store/app/globals.css

Conflicting in both orders, so whichever of #55/#54 merges second hits it. Cause: main's globals.css ends at line 847 with the @media (max-width: 700px) block, and both PRs now append a brand-new region at exactly that anchor — #54 its .browser* / .facet-* / .filter-* block, #55 this @media (max-width: 600px) at 906-931. Git cannot order two pure additions sharing one anchor. Your selectors are entirely #47's own, so this is not the selector-ownership problem I checked for in rounds 1 and 2 — it is the merge collision that rule exists to prevent, arriving by position instead.

Why this is a blocker rather than a routine conflict. Look at the shape git produces. The two sides share the file's trailing braces: <<<<<<< opens right after your comment, ======= falls between your grid-column: 1 / -1; and #54's region comment, and >>>>>>> lands inside #54's last rule — with } } sitting outside the markers. So the natural resolution, keep both sides and delete the three marker lines, yields:

naive keep-both -> final brace depth: 2 | min: 0 (UNBALANCED)
nesting depth at .facet-panel: 2 -> #46 region is NESTED inside #47 rule/media

I ran that resolution. It nests #54's entire filter-sidebar region inside .cart-line > :last-child { … }, where browsers discard every one of those declarations, and leaves the stylesheet two braces open at EOF. Both regions break and nothing tells you: the file still parses, 139 top-level-looking class rules either way, and the two brace counts are the only signal. With no workflow on pull_request and merge running wrangler pages deploy, a hand resolution that misses the shared braces ships a store with no filter styling and a malformed cart rule straight to production.

Remedy, verified. Move the 26-line block off the end-of-file anchor into #47's own cart region — immediately after .cart-line p (before .cart-summary, line 622 at this head). Nothing else changes; a media query's position among sibling top-level rules does not affect the cascade here, since no other rule sets grid-template-columns or grid-column on .cart-line. I built exactly that file and merged it:

merge-file exit=0 (0 = clean, 1 = conflict)   conflict markers: 0
final brace depth: 0 | min depth: 0
has #47 cart media: true | has #46 facet-panel: true | has #47 stock-badge co-selector: true

Clean merge, balanced braces, both regions intact. Any anchor other than EOF works; this one keeps the rule beside the .cart-line rules it overrides.

The layout fix itself — verified, and your breakpoint call is right

Rebuilt at 8f160d1 on a 7-item fixture and swept the cart across 18 viewports, 320px to 1440px, on a worst-case cart: indent + installAtShop, indent + pickup-only, indent alone, a stocked line, and a pre-#47 line with the key absent.

  • Zero name/meta overlap with the controls at every width. Tested by rect intersection, not by eye. Pre-fix I had recorded overlap at every width ≤414px, including on a stocked line.
  • Zero horizontal overflow at all 18 widths.
  • Rows uniform 157px below the breakpoint; 97px at and above 620px with columns 64px 230.75px 261.25px — the desktop layout I approved in round 2 is untouched, which a max-width query guarantees and the measurements confirm.
  • Screenshotted at 375px: full product name on one line, meta wrapping cleanly to two, controls on their own row. Compare the round-2 shot where Hapus sat on top of Rp 85.000.

600 over 480 is the better number and your arithmetic checks out for the single-tag meta. I measured intrinsic single-line widths with a nowrap probe in the rendered font: Rp 85.000 · Indent, dipesan dulu needs 202px, close to your ~215px. At 620px the middle column is 230.75px so it holds one line; pre-fix at 580px it was 191px and wrapped. Your 620/580 observations reproduce exactly.

Everything from rounds 1 and 2 re-checked at this head and unaffected, as you said: 66/66 / 11 suites; build compiles, TypeScript clean, 23/23 pages; biome on the 8 changed files reports only the 2 pre-existing globals.css warnings; the co-selector still load-bearing (stripping it moves all 5 card chips to rgb(90,97,89), restore returns them); chips still 12.48px/700, 999px, correct --ok/--warn/--off; indent add enabled and habis disabled; unpriced habis still renders no detail .price; empty fitment/oemNumbers still vanish with h2 list ["Stok dan harga"] alone.

The .price decline still stands with me. Three unanswered filing requests to Main is not on you.

Findings

apps/store/app/globals.css:906: blocker: appending this region at end-of-file puts it on the same anchor #54 appends its filter-sidebar region to, turning merge-tree 3daea4b + 6fece92 (exit 0 in round 2) into 8f160d1 + 6fece92 exit 1, conflicting in both orders. The two sides share the file's trailing braces, so the natural keep-both resolution nests #54's entire .facet-*/.filter-* region inside .cart-line > :last-child { … } and leaves the sheet two braces open — I ran it and measured brace depth 2, .facet-panel at nesting depth 2. No CI runs on pull requests and merge deploys via wrangler pages deploy, so that resolution error reaches production silently. Move the 26-line block from 906-931 to just after .cart-line p (before .cart-summary, line 622); no cascade change, since nothing else sets grid-template-columns or grid-column on .cart-line. I verified the relocated file merges with 6fece92 at exit 0, 0 markers, balanced braces, both regions intact.

apps/store/app/globals.css:917: nit: the recorded arithmetic is right for the single-tag meta but does not hold for the two-tag one, so say which it is for or the next person re-deriving it will move the breakpoint. Intrinsic single-line widths in the rendered font: Rp 85.000 · Indent, dipesan dulu 202px (your ~215px, fine), but Rp 275.000 · Indent, dipesan dulu · Pasang di bengkel needs 331px and the Ambil di bengkel variant 323px. Those wrap to two lines in the three-column layout from 700px all the way down to 601px — above your breakpoint — so "600px is where the three-column layout stops being able to do its job" is true at 202px and would be ~740-760px at 331px. Relatedly, the PR comment's "meta holds ONE line at every width, 320px to 1440px" is not what I measure: the two-tag meta takes two lines at 601-700px and at ≤414px. Neither is a defect — the wrapping is legible, rows stay uniform, no overlap, no overflow — but the numbers in the comment should name their case.

VERDICT: request-changes Round-3 review at `8f160d1`. **The layout fix is correct and your 600px reasoning is better than my 480px was** — I checked it and you are right, so I am not arguing the breakpoint. But this push moved #47's CSS to the one line in the file it could not safely occupy, and it turned a merge I verified clean in round 2 into a conflict whose obvious resolution silently breaks both regions. That is the whole of the blocker; the fix is a relocation with zero behavioural change and I have already proven it merges clean. ### The blocker, measured Round 2 I reported `git merge-tree --write-tree 3daea4b 6fece92` → **exit 0, no conflict**. At this head, against the *same* #54 commit: ``` merge 3daea4b + 6fece92 -> exit=0 merge 8f160d1 + 6fece92 -> exit=1 CONFLICT (content): apps/store/app/globals.css ``` Conflicting in both orders, so whichever of #55/#54 merges second hits it. Cause: `main`'s `globals.css` ends at line 847 with the `@media (max-width: 700px)` block, and **both PRs now append a brand-new region at exactly that anchor** — #54 its `.browser*` / `.facet-*` / `.filter-*` block, #55 this `@media (max-width: 600px)` at 906-931. Git cannot order two pure additions sharing one anchor. Your selectors are entirely #47's own, so this is not the selector-ownership problem I checked for in rounds 1 and 2 — it is the merge collision that rule exists to prevent, arriving by position instead. **Why this is a blocker rather than a routine conflict.** Look at the shape git produces. The two sides share the file's trailing braces: `<<<<<<<` opens right after your comment, `=======` falls between your `grid-column: 1 / -1;` and #54's region comment, and `>>>>>>>` lands inside #54's last rule — with `}` `}` sitting *outside* the markers. So the natural resolution, keep both sides and delete the three marker lines, yields: ``` naive keep-both -> final brace depth: 2 | min: 0 (UNBALANCED) nesting depth at .facet-panel: 2 -> #46 region is NESTED inside #47 rule/media ``` I ran that resolution. It nests #54's **entire** filter-sidebar region inside `.cart-line > :last-child { … }`, where browsers discard every one of those declarations, and leaves the stylesheet two braces open at EOF. Both regions break and nothing tells you: the file still parses, `139 top-level-looking class rules` either way, and the two brace counts are the only signal. With **no workflow on `pull_request`** and merge running `wrangler pages deploy`, a hand resolution that misses the shared braces ships a store with no filter styling and a malformed cart rule straight to production. **Remedy, verified.** Move the 26-line block off the end-of-file anchor into #47's own cart region — immediately after `.cart-line p` (before `.cart-summary`, line 622 at this head). Nothing else changes; a media query's position among sibling top-level rules does not affect the cascade here, since no other rule sets `grid-template-columns` or `grid-column` on `.cart-line`. I built exactly that file and merged it: ``` merge-file exit=0 (0 = clean, 1 = conflict) conflict markers: 0 final brace depth: 0 | min depth: 0 has #47 cart media: true | has #46 facet-panel: true | has #47 stock-badge co-selector: true ``` Clean merge, balanced braces, both regions intact. Any anchor other than EOF works; this one keeps the rule beside the `.cart-line` rules it overrides. ### The layout fix itself — verified, and your breakpoint call is right Rebuilt at `8f160d1` on a 7-item fixture and swept the cart across **18 viewports, 320px to 1440px**, on a worst-case cart: `indent + installAtShop`, `indent + pickup-only`, `indent` alone, a stocked line, and a pre-#47 line with the key absent. - **Zero name/meta overlap with the controls at every width.** Tested by rect intersection, not by eye. Pre-fix I had recorded overlap at every width ≤414px, including on a stocked line. - **Zero horizontal overflow at all 18 widths.** - Rows uniform **157px** below the breakpoint; **97px** at and above 620px with columns `64px 230.75px 261.25px` — the desktop layout I approved in round 2 is untouched, which a `max-width` query guarantees and the measurements confirm. - Screenshotted at 375px: full product name on one line, meta wrapping cleanly to two, controls on their own row. Compare the round-2 shot where `Hapus` sat on top of `Rp 85.000`. **600 over 480 is the better number and your arithmetic checks out for the single-tag meta.** I measured intrinsic single-line widths with a nowrap probe in the rendered font: `Rp 85.000 · Indent, dipesan dulu` needs **202px**, close to your ~215px. At 620px the middle column is 230.75px so it holds one line; pre-fix at 580px it was 191px and wrapped. Your 620/580 observations reproduce exactly. Everything from rounds 1 and 2 re-checked at this head and unaffected, as you said: 66/66 / 11 suites; build compiles, TypeScript clean, 23/23 pages; biome on the 8 changed files reports only the 2 pre-existing `globals.css` warnings; the co-selector still load-bearing (stripping it moves all 5 card chips to `rgb(90,97,89)`, restore returns them); chips still `12.48px/700`, `999px`, correct `--ok`/`--warn`/`--off`; `indent` add enabled and `habis` disabled; unpriced `habis` still renders no detail `.price`; empty `fitment`/`oemNumbers` still vanish with `h2` list `["Stok dan harga"]` alone. The `.price` decline still stands with me. Three unanswered filing requests to Main is not on you. ### Findings apps/store/app/globals.css:906: blocker: appending this region at end-of-file puts it on the same anchor #54 appends its filter-sidebar region to, turning `merge-tree 3daea4b + 6fece92` (exit 0 in round 2) into `8f160d1 + 6fece92` exit 1, conflicting in both orders. The two sides share the file's trailing braces, so the natural keep-both resolution nests #54's entire `.facet-*`/`.filter-*` region inside `.cart-line > :last-child { … }` and leaves the sheet two braces open — I ran it and measured brace depth 2, `.facet-panel` at nesting depth 2. No CI runs on pull requests and merge deploys via `wrangler pages deploy`, so that resolution error reaches production silently. Move the 26-line block from 906-931 to just after `.cart-line p` (before `.cart-summary`, line 622); no cascade change, since nothing else sets `grid-template-columns` or `grid-column` on `.cart-line`. I verified the relocated file merges with `6fece92` at exit 0, 0 markers, balanced braces, both regions intact. apps/store/app/globals.css:917: nit: the recorded arithmetic is right for the single-tag meta but does not hold for the two-tag one, so say which it is for or the next person re-deriving it will move the breakpoint. Intrinsic single-line widths in the rendered font: `Rp 85.000 · Indent, dipesan dulu` 202px (your ~215px, fine), but `Rp 275.000 · Indent, dipesan dulu · Pasang di bengkel` needs **331px** and the `Ambil di bengkel` variant **323px**. Those wrap to two lines in the three-column layout from 700px all the way down to 601px — above your breakpoint — so "600px is where the three-column layout stops being able to do its job" is true at 202px and would be ~740-760px at 331px. Relatedly, the PR comment's "meta holds ONE line at every width, 320px to 1440px" is not what I measure: the two-tag meta takes two lines at 601-700px and at ≤414px. Neither is a defect — the wrapping is legible, rows stay uniform, no overlap, no overflow — but the numbers in the comment should name their case.
The block was appended at end of file, which is the exact anchor #46 appends
its filter-sidebar region to. merge-tree against feat/46-faceted-filters went
from exit 0 to exit 1 in both orders, and the natural keep-both resolution
nests #46 entire facet region inside `.cart-line > :last-child { … }`, where
browsers discard it, leaving the sheet two braces open. The file still parses
and the rule count is unchanged, so nothing warns about it — and with no CI on
pull_request that resolution would ship a store with no filter styling.

Moved beside the .cart-line rules it overrides. No cascade change: nothing else
sets grid-template-columns or grid-column on .cart-line, and the query still
follows the base rule it overrides.

Also names which meta the 600px arithmetic is sized for (the single-tag line,
202px intrinsic) and records that two-tag metas measure 331px/323px and wrap to
two legible lines above the breakpoint and at <=414px.
yuris commented 2026-08-27 14:21:48 +00:00
Author
Owner
Copy link

Blocker fixed. New head c20ae14. Re-review requested against c20ae14.

Blocker — EOF anchor colliding with #46's region: FIXED

This is a good catch and I want to be clear that I would not have found it: I checked composition twice and both times measured the wrong thing. At 3daea4b I verified merge-tree exit 0 and reported the pair composition-safe — then appended a region at EOF and never re-ran it, because I'd mentally filed "composition" as settled. The regression came from where I put the code, not what it does, which is precisely the class of thing that survives a selector-by-selector audit.

Reproduced your finding before fixing:

$ git merge-tree --write-tree 8f160d1 origin/feat/46-faceted-filters ; echo $?
1

And the reason it is a blocker rather than a routine conflict is the part I want on the record: the two regions share the file's trailing braces, so the natural keep-both resolution nests #46's entire facet region inside .cart-line > :last-child { … }, where browsers discard all of it, and leaves the sheet two braces open. The file still parses, the rule count is unchanged, biome stays quiet, and no workflow runs on pull_request — so that resolution would wrangler pages deploy a store with a filter sidebar and no filter styling, silently.

Moved the 26-line block from EOF to immediately after .cart-line p, before .cart-summary, as you suggested. No cascade change: nothing else sets grid-template-columns or grid-column on .cart-line, and the query still follows the base rule it overrides. The comment now records why it is anchored there, so it does not drift back to EOF next time someone appends to this file.

Verified after the move

$ git merge-tree --write-tree c20ae14 origin/feat/46-faceted-filters ; echo $?
0
$ git merge-tree --write-tree origin/feat/46-faceted-filters c20ae14 ; echo $?
0

Exit 0 in both orders. Then I composed it for real rather than trusting the exit code — merged c20ae14 + 6fece92 in a throwaway branch, built, and rendered:

  • 0 conflict markers in globals.css.
  • Comment-stripped brace walk: final depth 0, minimum depth 0 — balanced, and never dips negative.
  • Nesting depth of every selector from both regions: .facet-panel, .facet-group, .filter-chip, .filter-toolbar, .browser-results, .stock-badge, .fitment, .spec-code — all at depth 0. The depth-2 burial you measured is gone.
  • Composed suite 90/90. TypeScript clean, 21/21 pages.
  • The decisive check, since balanced braces alone would not prove the styles apply: in the composed build .facet-group computes background rgb(255,255,255), border 1px solid, border-radius 12px, padding 8px 16px 12px, and .browser computes display: grid with grid-template-columns: 240px 968px. #46's sidebar is styled and laid out, not discarded.
  • Cart layout still correct in the composed build: 875px middle column and 97px rows at 1440px and 620px, stacked with 488px at 600px and 263px at 375px, meta unwrapped and no overflow at all four.

Throwaway branch deleted; 0 tmp branches on origin.

Nit — which meta the arithmetic is for: FIXED

Fair, and my "one line at every width 320–1440" was simply wrong as written — it was true of the single-tag meta I measured and I generalised it. Your numbers reproduce: single-tag intrinsic 202px, two-tag 331px (pasang di bengkel) and 323px (ambil di bengkel).

The comment now names the case it is sized for and states the two-tag behaviour explicitly: sized on the single-tag meta at 202px, so ~590–600px total; two-tag metas wrap to two lines from 700px down to 601px, above the breakpoint, and again at ≤414px below it — two legible lines in a uniform row, not a collapse, and not what the breakpoint is sized for. Corrected in the PR record here too rather than left in a stale table.

Also noted: you measured 202px against my ~215px estimate and I've adopted your figure.

Re-verification at c20ae14

$ git rev-parse HEAD
c20ae1415b194b7a5b58dab2bee70842e255f6ec

$ pnpm --filter garage-store test
# tests 66
# suites 11
# pass 66
# fail 0

$ pnpm --filter garage-store build
✓ Compiled successfully in 2.7s
  Finished TypeScript in 2.4s ...
✓ Generating static pages using 13 workers (21/21) in 677ms

$ pnpm exec biome check <the 8 changed files>
Checked 8 files in 15ms. No fixes applied.
Found 2 warnings.

Same 2 pre-existing globals.css warnings, unchanged from origin/main.

This push moves one block and edits its comment — no declaration or selector changed, no other file touched, and the rendered result at every viewport is identical to 8f160d1 (which you already verified across 18 viewports). So your rounds 1–3 findings all stand, including the 600px breakpoint you told me to keep.

Thanks for holding this one — a silent conflict resolution that ships unstyled CSS with no CI to catch it is exactly the failure that would have been found in production.

The .price decline stands and is flagged to Main for its own issue. Merging this PR deploys to production.

Blocker fixed. New head `c20ae14`. **Re-review requested against `c20ae14`.** ## Blocker — EOF anchor colliding with #46's region: FIXED This is a good catch and I want to be clear that I would not have found it: I checked composition *twice* and both times measured the wrong thing. At `3daea4b` I verified merge-tree exit 0 and reported the pair composition-safe — then appended a region at EOF and never re-ran it, because I'd mentally filed "composition" as settled. The regression came from where I put the code, not what it does, which is precisely the class of thing that survives a selector-by-selector audit. Reproduced your finding before fixing: ``` $ git merge-tree --write-tree 8f160d1 origin/feat/46-faceted-filters ; echo $? 1 ``` And the reason it is a blocker rather than a routine conflict is the part I want on the record: the two regions share the file's trailing braces, so the natural keep-both resolution nests #46's **entire** facet region inside `.cart-line > :last-child { … }`, where browsers discard all of it, and leaves the sheet two braces open. The file still parses, the rule count is unchanged, biome stays quiet, and no workflow runs on `pull_request` — so that resolution would `wrangler pages deploy` a store with a filter sidebar and no filter styling, silently. Moved the 26-line block from EOF to immediately after `.cart-line p`, before `.cart-summary`, as you suggested. No cascade change: nothing else sets `grid-template-columns` or `grid-column` on `.cart-line`, and the query still follows the base rule it overrides. The comment now records *why* it is anchored there, so it does not drift back to EOF next time someone appends to this file. ### Verified after the move ``` $ git merge-tree --write-tree c20ae14 origin/feat/46-faceted-filters ; echo $? 0 $ git merge-tree --write-tree origin/feat/46-faceted-filters c20ae14 ; echo $? 0 ``` Exit 0 in **both** orders. Then I composed it for real rather than trusting the exit code — merged `c20ae14` + `6fece92` in a throwaway branch, built, and rendered: - 0 conflict markers in `globals.css`. - Comment-stripped brace walk: final depth **0**, minimum depth **0** — balanced, and never dips negative. - Nesting depth of every selector from **both** regions: `.facet-panel`, `.facet-group`, `.filter-chip`, `.filter-toolbar`, `.browser-results`, `.stock-badge`, `.fitment`, `.spec-code` — all at depth **0**. The depth-2 burial you measured is gone. - Composed suite **90/90**. TypeScript clean, 21/21 pages. - The decisive check, since balanced braces alone would not prove the styles *apply*: in the composed build `.facet-group` computes `background rgb(255,255,255)`, `border 1px solid`, `border-radius 12px`, `padding 8px 16px 12px`, and `.browser` computes `display: grid` with `grid-template-columns: 240px 968px`. #46's sidebar is styled and laid out, not discarded. - Cart layout still correct in the composed build: 875px middle column and 97px rows at 1440px and 620px, stacked with 488px at 600px and 263px at 375px, meta unwrapped and no overflow at all four. Throwaway branch deleted; 0 `tmp` branches on origin. ## Nit — which meta the arithmetic is for: FIXED Fair, and my "one line at every width 320–1440" was simply wrong as written — it was true of the single-tag meta I measured and I generalised it. Your numbers reproduce: single-tag intrinsic 202px, two-tag 331px (`pasang di bengkel`) and 323px (`ambil di bengkel`). The comment now names the case it is sized for and states the two-tag behaviour explicitly: sized on the single-tag meta at 202px, so ~590–600px total; two-tag metas wrap to two lines from 700px down to 601px, above the breakpoint, and again at ≤414px below it — two legible lines in a uniform row, not a collapse, and not what the breakpoint is sized for. Corrected in the PR record here too rather than left in a stale table. Also noted: you measured 202px against my ~215px estimate and I've adopted your figure. ## Re-verification at `c20ae14` ``` $ git rev-parse HEAD c20ae1415b194b7a5b58dab2bee70842e255f6ec $ pnpm --filter garage-store test # tests 66 # suites 11 # pass 66 # fail 0 $ pnpm --filter garage-store build ✓ Compiled successfully in 2.7s Finished TypeScript in 2.4s ... ✓ Generating static pages using 13 workers (21/21) in 677ms $ pnpm exec biome check <the 8 changed files> Checked 8 files in 15ms. No fixes applied. Found 2 warnings. ``` Same 2 pre-existing `globals.css` warnings, unchanged from `origin/main`. This push **moves one block and edits its comment** — no declaration or selector changed, no other file touched, and the rendered result at every viewport is identical to `8f160d1` (which you already verified across 18 viewports). So your rounds 1–3 findings all stand, including the 600px breakpoint you told me to keep. Thanks for holding this one — a silent conflict resolution that ships unstyled CSS with no CI to catch it is exactly the failure that would have been found in production. The `.price` decline stands and is flagged to Main for its own issue. Merging this PR deploys to production.
yuris reviewed 2026-08-27 14:27:21 +00:00
yuris left a comment
Copy link

VERDICT: approve

Round-4 review at c20ae14. Blocker fixed, nit fixed, and I re-derived both independently rather than reading your table. One nit below, minor and optional. This is good to merge once Main confirms order.

The blocker is gone, and I proved the move is behaviourally inert

Pure relocation, established structurally rather than by eye. I stripped every comment from globals.css at 8f160d1 and c20ae14 and compared: 831 rule lines each, and the sorted line multisets are identical — nothing added, removed or altered, so the only change is ordering. A longest-common-prefix/suffix walk then isolates the change to exactly one contiguous block: common prefix runs 553 stripped lines, through .cart-line p { … }, after which c20ae14 has the @media (max-width: 600px) block and 8f160d1 does not. One block moved, nothing else touched. Raw file 932 → 940 lines, all eight from the comment.

Cascade-safe at the new anchor, checked rather than assumed. .cart-line appears in the file only at 597-617 (base rules) and 649/652 (inside the moved query). Every grid-template-columns / grid-column after line 648 belongs to .product-detail (737), .breadcrumb (838), or the .contact-grid/.product-detail/.hero/.product-grid/.category-tiles/.photo-strip media queries (902-917). Nothing later can override it, and the query still follows the base rule it overrides.

Merge clean, and I composed for real.

merge-tree c20ae14 + 6fece92 -> exit=0
merge-tree 6fece92 + c20ae14 -> exit=0

Then an actual git merge in a throwaway detached worktree: no conflict markers, git diff --check clean, comment-stripped brace walk final depth 0, min depth 0 — never dips negative. And the depth-2 burial that made this a blocker is gone: first-occurrence nesting depth is 0 for .facet-panel, .facet-group, .filter-chip, .filter-toolbar, .browser-results, .browser, .stock-badge, .fitment, .spec-code and .cart-line alike.

You are right that balanced braces would not prove the styles apply, so I measured that too — in my own composed build, not from your numbers. #46's region is styled and laid out:

selector computed
.browser display: grid, grid-template-columns: 240px 968px
.facet-group background rgb(255,255,255), border 1px solid rgb(227,222,211), border-radius 12px, padding 8px 16px 12px
.filter-toolbar display: flex, align-items: center, gap: 12px
.result-count styled, reads 1 produk after filtering

.filter-chip needs an active filter, so I clicked a real facet input: the chip row appears, display: flex, border-radius: 999px, background rgb(228,243,232), 1px solid, inside a wrapping .filter-chips with gap: 8px. #46's sidebar, groups, toolbar, chip row and result count all apply. In the same sheet, #47's card chip still computes rgb(138,90,18), 12% tint, 999px, 12.48px/700, align-self: start.

Composed suite 90/90, 16 suites. Composed build: TypeScript clean, 23/23 pages on my 7-item fixture. Cross-region isolation re-derived at this composed head: all 13 of #46's region selectors match zero elements on each of four detail pages (104-117 elements each).

Cart layout identical to the head I already verified, swept across 12 viewports in the composed build: zero rows with name/meta overlapping the controls at any width (rect intersection), zero horizontal overflow, 64px 874.75px 261.25px with 97px rows at 1440 down through 601px, stacking to 64px 488px at 600 and 64px 263px at 375 with uniform 157px rows. Screenshotted at 375px: name on one line, meta on two, controls on their own row, grid-column: 1 / -1 resolving on the third child.

JSON-LD re-verified in the composed build across all 7: ready→InStock, all three indent→BackOrder, habis priced→OutOfStock, habis unpriced→no offers key, BreadcrumbList everywhere. indent add enabled, both habis disabled, unpriced habis renders no detail .price, empty fitment/oemNumbers still vanish with the h2 list ["Stok dan harga"] alone, .spec-code mono on every spec row. Biome: the same 2 pre-existing warnings.

The nit is fixed accurately

The comment now names its case — single-tag meta, "Rp 85.000 · Indent, dipesan dulu", intrinsic 202px — adopts the measured 202px over the ~215px estimate, and its arithmetic reconciles: 261 + 202 + 64 + 32 + 32 = 591, matching the stated ~590-600px. It records that two-tag metas at 331px and 323px wrap to two lines from 700px down and again at ≤414px, and that this is two legible lines in a uniform row rather than a collapse. That is exactly what I measured. The anchor rationale is recorded too, which is the part that stops this drifting back to EOF the next time someone appends.

On your note about how the regression happened: filing composition as settled and not re-running it after moving code is the failure mode worth remembering, not the conflict itself. A selector audit could not have caught it, because nothing about the selectors changed.

Findings

apps/store/app/cart/cart-view.tsx:76: nit: .cart-line > :last-child is positional, and the element it targets is an unclassed <div>, so the rule silently retargets if anything is ever appended after the controls — a wrapper, a sticky note, a per-line warning. I confirmed the DOM is three children with the controls last and grid-column: 1 / -1 resolving on child 3, so it is correct today. A class hook on that div, matched as .cart-line > .cart-line-actions, would make the coupling explicit and cost one attribute. Not worth another round on its own — fold it in only if you touch this file again.

VERDICT: approve Round-4 review at `c20ae14`. Blocker fixed, nit fixed, and I re-derived both independently rather than reading your table. One nit below, minor and optional. This is good to merge once Main confirms order. ### The blocker is gone, and I proved the move is behaviourally inert **Pure relocation, established structurally rather than by eye.** I stripped every comment from `globals.css` at `8f160d1` and `c20ae14` and compared: **831 rule lines each, and the sorted line multisets are identical** — nothing added, removed or altered, so the only change is ordering. A longest-common-prefix/suffix walk then isolates the change to exactly one contiguous block: common prefix runs 553 stripped lines, through `.cart-line p { … }`, after which `c20ae14` has the `@media (max-width: 600px)` block and `8f160d1` does not. One block moved, nothing else touched. Raw file 932 → 940 lines, all eight from the comment. **Cascade-safe at the new anchor, checked rather than assumed.** `.cart-line` appears in the file only at 597-617 (base rules) and 649/652 (inside the moved query). Every `grid-template-columns` / `grid-column` after line 648 belongs to `.product-detail` (737), `.breadcrumb` (838), or the `.contact-grid`/`.product-detail`/`.hero`/`.product-grid`/`.category-tiles`/`.photo-strip` media queries (902-917). Nothing later can override it, and the query still follows the base rule it overrides. **Merge clean, and I composed for real.** ``` merge-tree c20ae14 + 6fece92 -> exit=0 merge-tree 6fece92 + c20ae14 -> exit=0 ``` Then an actual `git merge` in a throwaway detached worktree: no conflict markers, `git diff --check` clean, comment-stripped brace walk **final depth 0, min depth 0** — never dips negative. And the depth-2 burial that made this a blocker is gone: first-occurrence nesting depth is **0** for `.facet-panel`, `.facet-group`, `.filter-chip`, `.filter-toolbar`, `.browser-results`, `.browser`, `.stock-badge`, `.fitment`, `.spec-code` and `.cart-line` alike. **You are right that balanced braces would not prove the styles apply, so I measured that too** — in my own composed build, not from your numbers. #46's region is styled *and* laid out: | selector | computed | |---|---| | `.browser` | `display: grid`, `grid-template-columns: 240px 968px` | | `.facet-group` | `background rgb(255,255,255)`, `border 1px solid rgb(227,222,211)`, `border-radius 12px`, `padding 8px 16px 12px` | | `.filter-toolbar` | `display: flex`, `align-items: center`, `gap: 12px` | | `.result-count` | styled, reads `1 produk` after filtering | `.filter-chip` needs an active filter, so I clicked a real facet input: the chip row appears, `display: flex`, `border-radius: 999px`, `background rgb(228,243,232)`, `1px solid`, inside a wrapping `.filter-chips` with `gap: 8px`. #46's sidebar, groups, toolbar, chip row and result count all apply. In the same sheet, #47's card chip still computes `rgb(138,90,18)`, 12% tint, `999px`, `12.48px/700`, `align-self: start`. Composed suite **90/90, 16 suites**. Composed build: TypeScript clean, 23/23 pages on my 7-item fixture. Cross-region isolation re-derived at this composed head: all 13 of #46's region selectors match **zero** elements on each of four detail pages (104-117 elements each). **Cart layout identical to the head I already verified**, swept across 12 viewports in the composed build: zero rows with name/meta overlapping the controls at any width (rect intersection), zero horizontal overflow, `64px 874.75px 261.25px` with 97px rows at 1440 down through 601px, stacking to `64px 488px` at 600 and `64px 263px` at 375 with uniform 157px rows. Screenshotted at 375px: name on one line, meta on two, controls on their own row, `grid-column: 1 / -1` resolving on the third child. JSON-LD re-verified in the composed build across all 7: `ready`→`InStock`, all three `indent`→`BackOrder`, `habis` priced→`OutOfStock`, `habis` unpriced→**no `offers` key**, `BreadcrumbList` everywhere. `indent` add enabled, both `habis` disabled, unpriced `habis` renders no detail `.price`, empty `fitment`/`oemNumbers` still vanish with the `h2` list `["Stok dan harga"]` alone, `.spec-code` mono on every spec row. Biome: the same 2 pre-existing warnings. ### The nit is fixed accurately The comment now names its case — single-tag meta, `"Rp 85.000 · Indent, dipesan dulu"`, intrinsic 202px — adopts the measured 202px over the ~215px estimate, and its arithmetic reconciles: 261 + 202 + 64 + 32 + 32 = 591, matching the stated ~590-600px. It records that two-tag metas at 331px and 323px wrap to two lines from 700px down and again at ≤414px, and that this is two legible lines in a uniform row rather than a collapse. That is exactly what I measured. The anchor rationale is recorded too, which is the part that stops this drifting back to EOF the next time someone appends. On your note about how the regression happened: filing composition as settled and not re-running it after moving code is the failure mode worth remembering, not the conflict itself. A selector audit could not have caught it, because nothing about the selectors changed. ### Findings apps/store/app/cart/cart-view.tsx:76: nit: `.cart-line > :last-child` is positional, and the element it targets is an unclassed `<div>`, so the rule silently retargets if anything is ever appended after the controls — a wrapper, a sticky note, a per-line warning. I confirmed the DOM is three children with the controls last and `grid-column: 1 / -1` resolving on child 3, so it is correct today. A class hook on that div, matched as `.cart-line > .cart-line-actions`, would make the coupling explicit and cost one attribute. Not worth another round on its own — fold it in only if you touch this file again.
yuris commented 2026-08-27 14:30:11 +00:00
Author
Owner
Copy link

Pre-merge verification at the exact merge SHA

Main cleared this PR to merge first in the order. Gate re-checked at c20ae14, the SHA being merged, with a clean worktree.

$ git rev-parse HEAD
c20ae1415b194b7a5b58dab2bee70842e255f6ec

$ git status --porcelain
(empty)

$ pnpm --filter garage-store test
# tests 66
# suites 11
# pass 66
# fail 0
# skipped 0
# todo 0

$ pnpm --filter garage-store build
✓ Compiled successfully in 2.5s
  Finished TypeScript in 2.3s ...
✓ Generating static pages using 13 workers (21/21) in 669ms

$ pnpm exec biome check <the 8 changed files>
Checked 8 files in 17ms. No fixes applied.
Found 2 warnings.

$ git merge-tree --write-tree HEAD origin/feat/46-faceted-filters ; echo $?
0

The 2 biome warnings are the pre-existing globals.css pair (noImportantStyles at .product-type, noDescendingSpecificity at .image-placeholder), unchanged in count from origin/main.

Gate:

  • Newest verdict is review id 17 at 2026-08-27T14:27:21Z, commit_id pinned to c20ae14 — VERDICT: approve, 0 blockers, 0 shoulds. It post-dates the head, so no push has voided it.
  • mergeable: true, fetched now. Base is 55f5d72, which is current main — 0 commits behind, so this merges without a rebase and the verdict stands.
  • All three earlier rounds' findings resolved: the round-3 blocker (EOF anchor) fixed by relocation, both round-2 nits fixed, the round-1 should fixed. The one remaining nit is marked optional with an explicit "do NOT re-push for it".
  • Diff is 8 files, +262/−19, confined to apps/store. Nothing unrelated.
  • One optional nit deliberately not actioned, on the reviewer's instruction: .cart-line > :last-child is positional against an unclassed div. Correct today (three children, controls last); a .cart-line-actions hook is worth folding in only if that file is touched again for another reason.

Merging deploys to production

Recording this once more at the point of merge, because it is the consequence of this button and not a formality. No workflow runs on pull_request — both .forgejo/workflows/*.yml trigger only on push: main, and both deploy: management publishes a container, and the store runs wrangler pages deploy. So nothing in this PR was ever checked by the forge; every green light above is a command run locally and pasted here, and squashing this to main ships the storefront to production.

Deviations

Unchanged from the original body: the harga per <tanggal> wording is deliberately "as of" rather than "last changed", per the declined-imprecision note on #47, because every dashboard save refreshes priceUpdatedAt. A comment in page.tsx records that so it is not "corrected" later.

Two items found during review are out of scope here and filed separately by Main rather than folded in: the pre-existing .price-inside-.product-card cascade defect (store-wide, --ink-muted instead of --ink), and the .cart-line sub-600px collapse that this PR incidentally fixes for the cart.

Closes #47.

## Pre-merge verification at the exact merge SHA Main cleared this PR to merge first in the order. Gate re-checked at `c20ae14`, the SHA being merged, with a clean worktree. ``` $ git rev-parse HEAD c20ae1415b194b7a5b58dab2bee70842e255f6ec $ git status --porcelain (empty) $ pnpm --filter garage-store test # tests 66 # suites 11 # pass 66 # fail 0 # skipped 0 # todo 0 $ pnpm --filter garage-store build ✓ Compiled successfully in 2.5s Finished TypeScript in 2.3s ... ✓ Generating static pages using 13 workers (21/21) in 669ms $ pnpm exec biome check <the 8 changed files> Checked 8 files in 17ms. No fixes applied. Found 2 warnings. $ git merge-tree --write-tree HEAD origin/feat/46-faceted-filters ; echo $? 0 ``` The 2 biome warnings are the pre-existing `globals.css` pair (`noImportantStyles` at `.product-type`, `noDescendingSpecificity` at `.image-placeholder`), unchanged in count from `origin/main`. Gate: - Newest verdict is review **id 17** at `2026-08-27T14:27:21Z`, `commit_id` pinned to `c20ae14` — **`VERDICT: approve`**, 0 blockers, 0 shoulds. It post-dates the head, so no push has voided it. - `mergeable: true`, fetched now. Base is `55f5d72`, which is current `main` — 0 commits behind, so this merges without a rebase and the verdict stands. - All three earlier rounds' findings resolved: the round-3 blocker (EOF anchor) fixed by relocation, both round-2 nits fixed, the round-1 `should` fixed. The one remaining nit is marked optional with an explicit "do NOT re-push for it". - Diff is 8 files, +262/−19, confined to `apps/store`. Nothing unrelated. - One optional nit deliberately not actioned, on the reviewer's instruction: `.cart-line > :last-child` is positional against an unclassed div. Correct today (three children, controls last); a `.cart-line-actions` hook is worth folding in only if that file is touched again for another reason. ## Merging deploys to production Recording this once more at the point of merge, because it is the consequence of this button and not a formality. **No workflow runs on `pull_request`** — both `.forgejo/workflows/*.yml` trigger only on `push: main`, and both *deploy*: management publishes a container, and the store runs `wrangler pages deploy`. So nothing in this PR was ever checked by the forge; every green light above is a command run locally and pasted here, and squashing this to `main` ships the storefront to production. ## Deviations Unchanged from the original body: the `harga per <tanggal>` wording is deliberately "as of" rather than "last changed", per the declined-imprecision note on #47, because every dashboard save refreshes `priceUpdatedAt`. A comment in `page.tsx` records that so it is not "corrected" later. Two items found during review are **out of scope here and filed separately by Main** rather than folded in: the pre-existing `.price`-inside-`.product-card` cascade defect (store-wide, `--ink-muted` instead of `--ink`), and the `.cart-line` sub-600px collapse that this PR incidentally fixes for the cart. Closes #47.
yuris merged commit c4172b3877 into main 2026-08-27 14:30:24 +00:00
yuris deleted branch feat/47-product-detail 2026-08-27 14:30:24 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
yuris/garage!55
No description provided.