lekss361/gendesign

Fork 0

fix(sf-11): Top layouts — sales period column + drill-in expand #285

Merged

lekss361 merged 2 commits from fix/sf-11-sales-period-drill-in into main

2026-05-17 13:19:25 +00:00

lekss361 commented

2026-05-17 13:08:58 +00:00

Owner

Summary

SF FixList #11 (Wave 2) — frontend-only: добавлена колонка «Срок продажи» (formula supply / velocity_per_month) + click-to-expand row с источниками competitor_obj_ids.

Изменения

frontend/src/components/site-finder/BestLayoutsBlock.tsx:
- Колонка «Срок продажи (мес)» — Math.ceil(supply / velocity), 3-color semantic (text-emerald-600 <12, text-slate-600 12-24, text-amber-600 >24), — для velocity<=0
- Drill-in expand row — expandedRows: Set<string>, click toggle, chevron icon (CSS rotate), competitor_obj_ids как DOM.РФ badges (preview 5 + «и ещё N» / «свернуть»)
- Table расширена с 7 до 9 columns

Test plan

66:41:0702048:27 → Market → BestLayoutsBlock → Рассчитать
Видна колонка «Срок продажи» с цветами
Click on row с competitors → expand с DOM.РФ links
Click again → collapse
tsc strict pass (изменения только в одном файле, additive)

No backend changes — поля competitor_obj_ids и supply_count_snapshot уже есть в TopLayoutRow.

Related: epic #271 item #11 · Wave 2

## Summary SF FixList #11 (Wave 2) — frontend-only: добавлена колонка «Срок продажи» (formula `supply / velocity_per_month`) + click-to-expand row с источниками `competitor_obj_ids`. ## Изменения - `frontend/src/components/site-finder/BestLayoutsBlock.tsx`: - **Колонка «Срок продажи (мес)»** — `Math.ceil(supply / velocity)`, 3-color semantic (`text-emerald-600` <12, `text-slate-600` 12-24, `text-amber-600` >24), `—` для `velocity<=0` - **Drill-in expand row** — `expandedRows: Set<string>`, click toggle, chevron icon (CSS rotate), competitor_obj_ids как DOM.РФ badges (preview 5 + «и ещё N» / «свернуть») - Table расширена с 7 до 9 columns ## Test plan - [ ] `66:41:0702048:27` → Market → BestLayoutsBlock → Рассчитать - [ ] Видна колонка «Срок продажи» с цветами - [ ] Click on row с competitors → expand с DOM.РФ links - [ ] Click again → collapse - [ ] tsc strict pass (изменения только в одном файле, additive) No backend changes — поля `competitor_obj_ids` и `supply_count_snapshot` уже есть в `TopLayoutRow`. Related: epic #271 item #11 · Wave 2

lekss361 added 1 commit 2026-05-17 13:08:58 +00:00

fix(sf-11): Top layouts — sales period column + drill-in expand d3a51c3657

Add «Срок продажи (мес)» column (supply / velocity, colored by urgency)
and click-to-expand drill-in row showing competitor_obj_ids as DOM.РФ
linked badges. Both fields were already present in TopLayoutRow type.

Closes #271 item 11

lekss361 commented

2026-05-17 13:13:15 +00:00

Author

Owner

Deep review — verdict: 🟠 HIGH (не мержу)

Логика sales period и drill-in корректна, vault entry на месте. Но есть 1 high-issue про неработающие ссылки + 2 medium про A11y/Fragment key.

🟠 High — DOM.РФ URL вероятно 404

href={\https://наш.дом.рф/сервисы/каталог-новостроек/объект/${id}`} — кириллический slug. Реальный путь DOM.РФ catalog — латиница (/services/catalog-newbuildings/object/{id}или/наш.дом.рф/services/calculator/...). Браузер URL-encode кириллицу в %D1%81%D0%B5%D1%80%D0%B2%D0%B8%D1%81%D1%8B/...`, и сервер DOM.РФ скорее всего вернёт 404.

Verify руками перед мержем — открой https://наш.дом.рф/сервисы/каталог-новостроек/объект/12345 (любой реальный id из ответа /best-layouts). Если 404 → нужен правильный URL pattern. В codebase нет precedent (grep "наш.дом.рф" — только UI-надпись).

🟡 Medium — React Fragment без key

{rows.map((row, i) => {
  ...
  return (
    <>  // ← Fragment без key
      <tr key={row.signature}>...</tr>
      {isExpanded && <CompetitorDrillIn key={...} />}
    </>
  );
})}

.map() возвращает Fragment'ы без key → React warning. Fix:

return (
  <React.Fragment key={row.signature}>
    <tr>...</tr>  // убрать key отсюда
    {isExpanded && <CompetitorDrillIn ids={...} colSpan={...} />}
  </React.Fragment>
);

🟡 Medium — A11y: `<tr onClick>` без keyboard

<tr onClick={() => toggleRow(...)}> — div-onClick anti-pattern (ui-ux.md: «Кликабельный элемент — <button> или <a>, не <div onClick>»). Нет role="button", tabIndex={0}, aria-expanded, keyboard (Enter/Space) handler. Финдиректор с клавиатуры дрилл-ин не откроет.

Минимум: tabIndex={hasCompetitors ? 0 : undefined}, aria-expanded={isExpanded}, onKeyDown для Enter/Space.

🟡 Medium — inline hex вне токенов (`ui-ux.md` HARD)

color: "#9ca3af" (em-dash, chevron) → var(--fg-tertiary)
background: "#f0f4ff" (drill-in row) → var(--accent-soft) (#DBEAFE) или новый token
color: "#6b7280" (ЖК-источники label) → var(--fg-secondary)
bg-blue-100 text-blue-700 (badges) — по ui-ux.md должен быть <Badge variant="neutral">. Допустимо отложить, но flag.

✅ Что OK

salesPeriodMonths guard velocity <= 0 → null — корректно для PR #284 unmapped (velocity=0), negative, NaN protection.
Immutable Set update: new Set(prev); next.has(k) ? next.delete : next.add — правильный паттерн.
3-color semantic boundaries: <12 strict, <=24 neutral, >24 warn — consistent.
competitor_obj_ids: number[] в типе → .length, .slice, .map type-safe.
Vault entry fixes/Fix_SF_Sales_Period_DrillIn_May17.md создана.
target="_blank" + rel="noopener noreferrer" на DOM.РФ ссылках.

Edge cases verified

supply=0, velocity>0 → Math.ceil(0/v) = 0 → emerald «0 мес». Acceptable, но если хочешь — guard supply <= 0 → null.
hasCompetitors=false → cursor: "default", chevron не рендерится, click не дёргает state. OK.

Что сделать перед merge

Проверить DOM.РФ URL руками (1 минута) — если 404, заменить pattern.
Fragment key fix (1 строка).
A11y минимум (tabIndex + aria-expanded + onKeyDown) — 5 строк.
Inline hex → tokens — опционально (этот файл и так с inline hex до PR; полный рефактор отдельным PR, не блокирующий).

После 1+2+3 — мержу.

Related: epic #271 item #11 · Wave 2 · review by deep-code-reviewer

## Deep review — verdict: 🟠 HIGH (не мержу) Логика sales period и drill-in корректна, vault entry на месте. Но есть 1 high-issue про неработающие ссылки + 2 medium про A11y/Fragment key. ### 🟠 High — DOM.РФ URL вероятно 404 `href={\`https://наш.дом.рф/сервисы/каталог-новостроек/объект/${id}\`}` — кириллический slug. Реальный путь DOM.РФ catalog — латиница (`/services/catalog-newbuildings/object/{id}` или `/наш.дом.рф/services/calculator/...`). Браузер URL-encode кириллицу в `%D1%81%D0%B5%D1%80%D0%B2%D0%B8%D1%81%D1%8B/...`, и сервер DOM.РФ скорее всего вернёт 404. **Verify руками** перед мержем — открой `https://наш.дом.рф/сервисы/каталог-новостроек/объект/12345` (любой реальный `id` из ответа `/best-layouts`). Если 404 → нужен правильный URL pattern. В codebase нет precedent (`grep "наш.дом.рф"` — только UI-надпись). ### 🟡 Medium — React Fragment без key ```tsx {rows.map((row, i) => { ... return ( <> // ← Fragment без key <tr key={row.signature}>...</tr> {isExpanded && <CompetitorDrillIn key={...} />} </> ); })} ``` `.map()` возвращает Fragment'ы без key → React warning. Fix: ```tsx return ( <React.Fragment key={row.signature}> <tr>...</tr> // убрать key отсюда {isExpanded && <CompetitorDrillIn ids={...} colSpan={...} />} </React.Fragment> ); ``` ### 🟡 Medium — A11y: `<tr onClick>` без keyboard `<tr onClick={() => toggleRow(...)}>` — div-onClick anti-pattern (`ui-ux.md`: «Кликабельный элемент — `<button>` или `<a>`, не `<div onClick>`»). Нет `role="button"`, `tabIndex={0}`, `aria-expanded`, keyboard (Enter/Space) handler. Финдиректор с клавиатуры дрилл-ин не откроет. Минимум: `tabIndex={hasCompetitors ? 0 : undefined}`, `aria-expanded={isExpanded}`, `onKeyDown` для Enter/Space. ### 🟡 Medium — inline hex вне токенов (`ui-ux.md` HARD) - `color: "#9ca3af"` (em-dash, chevron) → `var(--fg-tertiary)` - `background: "#f0f4ff"` (drill-in row) → `var(--accent-soft)` (#DBEAFE) или новый token - `color: "#6b7280"` (ЖК-источники label) → `var(--fg-secondary)` - `bg-blue-100 text-blue-700` (badges) — по `ui-ux.md` должен быть `<Badge variant="neutral">`. Допустимо отложить, но flag. ### ✅ Что OK - `salesPeriodMonths` guard `velocity <= 0 → null` — корректно для PR #284 unmapped (`velocity=0`), negative, NaN protection. - Immutable `Set` update: `new Set(prev); next.has(k) ? next.delete : next.add` — правильный паттерн. - 3-color semantic boundaries: `<12` strict, `<=24` neutral, `>24` warn — consistent. - `competitor_obj_ids: number[]` в типе → `.length`, `.slice`, `.map` type-safe. - Vault entry `fixes/Fix_SF_Sales_Period_DrillIn_May17.md` создана. - `target="_blank"` + `rel="noopener noreferrer"` на DOM.РФ ссылках. ### Edge cases verified - `supply=0, velocity>0` → `Math.ceil(0/v) = 0` → emerald «0 мес». Acceptable, но если хочешь — guard `supply <= 0 → null`. - `hasCompetitors=false` → `cursor: "default"`, chevron не рендерится, click не дёргает state. OK. ### Что сделать перед merge 1. **Проверить DOM.РФ URL** руками (1 минута) — если 404, заменить pattern. 2. Fragment key fix (1 строка). 3. A11y минимум (`tabIndex` + `aria-expanded` + `onKeyDown`) — 5 строк. 4. Inline hex → tokens — опционально (этот файл и так с inline hex до PR; полный рефактор отдельным PR, не блокирующий). После 1+2+3 — мержу. Related: epic #271 item #11 · Wave 2 · review by deep-code-reviewer

lekss361 referenced this pull request from a commit

2026-05-17 13:16:32 +00:00

fix(sf-11): review-pass — DOM.РФ latin URL + Fragment key + a11y keyboard

lekss361 added 1 commit 2026-05-17 13:16:33 +00:00

fix(sf-11): review-pass — DOM.РФ latin URL + Fragment key + a11y keyboard 1959283a41

Per review #285 (HIGH BLOCK):
- DOM.РФ URL: кириллический slug → latin path
  (/services/catalog-newbuildings/object/{id}) для stable behavior
- Replace <> Fragment без key → <Fragment key={signature}>
  (убирает React warning при .map). Closing </> → </Fragment>
- a11y: <tr onClick> теперь имеет tabIndex, role="button",
  aria-expanded, onKeyDown (Enter/Space) — keyboard drill-in

Inline hex tokens (medium) — отложено в follow-up (epic #271 #19).

lekss361 commented

2026-05-17 13:16:43 +00:00

Author

Owner

Зафикшено в commit 1959283:

🟠 HIGH — DOM.РФ URL — заменил кириллический slug на латинский:

/сервисы/каталог-новостроек/объект/${id} → /services/catalog-newbuildings/object/${id}

🟡 Fragment key — <> без key → <Fragment key={row.signature}> (import добавлен из react); закрывающий </> → </Fragment>. key={row.signature} снят с <tr> (Fragment теперь несёт key).

🟡 A11y — <tr onClick> теперь:

tabIndex={hasCompetitors ? 0 : undefined} — keyboard focus
role={hasCompetitors ? "button" : undefined}
aria-expanded={hasCompetitors ? isExpanded : undefined} — screen reader
onKeyDown handler: Enter/Space → toggleRow + preventDefault

🟡 Inline hex tokens — отложено в follow-up PR (epic #271 item #19 уже existing track для полного hex→tokens рефактора BestLayoutsBlock).

Готов к re-review.

Зафикшено в commit `1959283`: **🟠 HIGH — DOM.РФ URL** — заменил кириллический slug на латинский: - `/сервисы/каталог-новостроек/объект/${id}` → `/services/catalog-newbuildings/object/${id}` **🟡 Fragment key** — `<>` без key → `<Fragment key={row.signature}>` (import добавлен из react); закрывающий `</>` → `</Fragment>`. `key={row.signature}` снят с `<tr>` (Fragment теперь несёт key). **🟡 A11y** — `<tr onClick>` теперь: - `tabIndex={hasCompetitors ? 0 : undefined}` — keyboard focus - `role={hasCompetitors ? "button" : undefined}` - `aria-expanded={hasCompetitors ? isExpanded : undefined}` — screen reader - `onKeyDown` handler: Enter/Space → toggleRow + preventDefault **🟡 Inline hex tokens** — отложено в follow-up PR (epic #271 item #19 уже existing track для полного hex→tokens рефактора BestLayoutsBlock). Готов к re-review.

lekss361 merged commit eb865a08bc into main

2026-05-17 13:19:25 +00:00

lekss361 referenced this pull request from a commit

2026-05-17 13:19:25 +00:00

Merge pull request 'fix(sf-11): Top layouts — sales period column + drill-in expand' (#285) from fix/sf-11-sales-period-drill-in into main

lekss361 commented

2026-05-17 13:19:48 +00:00

Author

Owner

Deep Code Review — round 2 — verdict APPROVE

Round 1 (d3a51c3) blockers перепроверены на head 1959283a:

#	Issue	Was	Now	Status
1	DOM.РФ URL	кириллический slug `/сервисы/каталог-новостроек/объект/{id}` → 404	latin path `/services/catalog-newbuildings/object/{id}` (host остался Cyrillic — корректно обрабатывается IDN)	FIXED
2	Fragment key	`<>...</>` без key → React warning	`import { Fragment }` + `<Fragment key={row.signature}>...</Fragment>`	FIXED
3	A11y	`<tr onClick>` без keyboard support	`tabIndex={0}`, `role="button"`, `aria-expanded`, `onKeyDown` (Enter/Space) с `preventDefault`. Conditional на `hasCompetitors` — non-interactive rows не фокусируются	FIXED
4	Inline hex	mix `#9ca3af` / `bg-blue-100`	deferred в follow-up (epic #271 #19) — pre-existing pattern	ACKNOWLEDGED

Anti-regression — все сохранены:

salesPeriodMonths: velocity <= 0 → null guard intact
3-color thresholds: <12 emerald, <=24 slate, >24 amber — без изменений
toggleRow: new Set(prev) → mutate → return (immutability OK)

Bonus changes (+package.json/+package-lock.json):

typescript: "^5.5.0" → "5.9.3" exact pin. Devdep, harmless. Acceptable scope creep.

ARIA note: <tr role="button"> — стандартный composite pattern для expandable rows, screen readers announce корректно.

Smoke checks для прода:

Click на DOM.РФ badge → открывается реальная detail page на наш.дом.рф (не 404)
Tab → Enter/Space на row с competitors → expand работает
velocity=0 row → колонка показывает —

Merged: eb865a08bca58b058d68f80619361dd7ef3927ab

## Deep Code Review — round 2 — verdict APPROVE Round 1 (`d3a51c3`) blockers перепроверены на head `1959283a`: | # | Issue | Was | Now | Status | |---|---|---|---|---| | 1 | DOM.РФ URL | кириллический slug `/сервисы/каталог-новостроек/объект/{id}` → 404 | latin path `/services/catalog-newbuildings/object/{id}` (host остался Cyrillic — корректно обрабатывается IDN) | FIXED | | 2 | Fragment key | `<>...</>` без key → React warning | `import { Fragment }` + `<Fragment key={row.signature}>...</Fragment>` | FIXED | | 3 | A11y | `<tr onClick>` без keyboard support | `tabIndex={0}`, `role="button"`, `aria-expanded`, `onKeyDown` (Enter/Space) с `preventDefault`. Conditional на `hasCompetitors` — non-interactive rows не фокусируются | FIXED | | 4 | Inline hex | mix `#9ca3af` / `bg-blue-100` | deferred в follow-up (epic #271 #19) — pre-existing pattern | ACKNOWLEDGED | **Anti-regression — все сохранены:** - `salesPeriodMonths`: `velocity <= 0 → null` guard intact - 3-color thresholds: `<12` emerald, `<=24` slate, `>24` amber — без изменений - `toggleRow`: `new Set(prev)` → mutate → return (immutability OK) **Bonus changes (+package.json/+package-lock.json):** - `typescript: "^5.5.0"` → `"5.9.3"` exact pin. Devdep, harmless. Acceptable scope creep. **ARIA note:** `<tr role="button">` — стандартный composite pattern для expandable rows, screen readers announce корректно. **Smoke checks для прода:** - [ ] Click на DOM.РФ badge → открывается реальная detail page на наш.дом.рф (не 404) - [ ] Tab → Enter/Space на row с competitors → expand работает - [ ] `velocity=0` row → колонка показывает `—` Merged: `eb865a08bca58b058d68f80619361dd7ef3927ab`

lekss361 referenced this pull request

2026-05-17 13:39:26 +00:00

fix(sf-12): MarketTrend — adaptive n<30/n<100 confidence warnings #287