---
type: review
work_item_id: ET-002
title: "Code Review: Чекбокс показа/скрытия POI в попапе рельефа"
version: 2
status: approved
verdict: APPROVED
created_at: 2026-05-21
authors:
  - "agent:reviewer"
---

# Code Review — ET-002

## Вердикт

**APPROVED** — найдены только замечания уровня P3 (nice-to-have).
Блокеров (P0) и must-fix (P1) нет.

## Объём ревью

Проверены изменения в `src/web/app.js` и `src/web/index.html` против:

- `docs/work-items/ET-002/02-trz.md` (ТЗ, v1, approved)
- `docs/work-items/ET-002/03-acceptance-criteria.md` (AC, v1, approved)
- `docs/work-items/ET-002/06-adr/adr-0001-poi-visibility-client-side.md`
- `docs/work-items/ET-002/04-test-plan.yaml`
- `CLAUDE.md`

Дополнительно учтён юнит-тест `tests/unit/poi_toggle.test.js`.

---

## 1. Соответствие ТЗ

| Требование | Статус | Где реализовано |
|------------|--------|-----------------|
| REQ-F-01 — чекбокс «POI» в `terrain-popup` после `trails-path-cb`, отделён `<hr>` | ✅ | `index.html:56-60` |
| REQ-F-02 — состояние по умолчанию: checked, POI видимы | ✅ | `index.html:58` (атрибут `checked`); `app.js:2848-2852` (`stored === null → poiOn = true`) |
| REQ-F-03 — снятие чекбокса → `visibility: 'none'` для `poi-circles`, `poi-labels` | ✅ | `app.js:2833-2837` → `applyPoiVisibility(false)` → `app.js:2815-2825` |
| REQ-F-04 — установка чекбокса → `visibility: 'visible'` | ✅ | те же функции, ветка `visible` |
| REQ-F-05 — сохранение в `localStorage['poi-visible']` (`'1'`/`'0'`) | ✅ | `app.js:2835` |
| REQ-F-06 — восстановление при загрузке (`'0'` скрыть / `'1'`\|null показать) | ✅ | `app.js:2847-2853` (`restorePoiState`) |
| REQ-F-07 — синхронизация `layerState.poi` | ✅ | `app.js:2816` (`layerState.poi = visible` в общем хелпере) |
| REQ-NF-01 — переключение < 50 мс, без перезагрузки тайлов | ✅ | используется `setLayoutProperty`, источник слоёв не трогается |
| REQ-NF-02 — совместимость с MapLibre GL JS | ✅ | штатные API MapLibre, без экзотики |
| REQ-NF-03 — мобильный touch target ≥ 44px | ⚠️ см. P3-2 | переиспользован класс `terrain-checkbox` (как предписано ТЗ §3) |
| REQ-NF-04 — отсутствие регрессий | ✅ | существующие чекбоксы и их обработчики не затронуты |

UI-спецификация ТЗ §3 (разметка, `id="poi-visible-cb"`, `onchange="onPoiCheckbox()"`,
класс `terrain-checkbox`) воспроизведена в `index.html` дословно.

**Вывод:** все функциональные требования выполнены.

## 2. Соответствие ADR (adr-0001)

| Пункт решения | Статус | Комментарий |
|---------------|--------|-------------|
| 1. Видимость через `map.setLayoutProperty(... 'visibility' ...)` | ✅ | `app.js:2822` |
| 2. Персистентность — `localStorage['poi-visible']`, `'1'`/`'0'` | ✅ | соответствует конвенции проекта (`trails-track`, `terrain-*`) |
| 3. Источник истины — `layerState.poi`; `onPoiCheckbox()` синхронно обновляет `layerState`, `localStorage`, `layout.visibility` | ✅ | `onPoiCheckbox` → `setItem` + `applyPoiVisibility` (правит `layerState` и оба слоя) |
| 4. Без дублирования: общий приватный хелпер, переиспользование `layerGroups.poi` | ✅ | выделен `applyPoiVisibility(visible)`, итерирует `layerGroups.poi`; используется и `onPoiCheckbox`, и `restorePoiState` — ровно как предлагает ADR |
| 5. Backend/БД/API/инфраструктура без изменений | ✅ | изменения чисто клиентские |

Выбран и реализован Вариант A. Решение полностью соответствует ADR.
Замечание по консистентности с `toggleLayer('poi')` — см. P3-1.

## 3. Качество кода

Сильные стороны:

- Изменения локализованы в явно размеченном блоке
  `>>> ET-002 POI visibility block ... <<<` (`app.js:2800-2854`) — удобно
  для ревью и для тест-харнеса.
- JSDoc на всех трёх функциях, есть ссылка на ADR в шапке блока.
- `applyPoiVisibility` имеет guard `if (!map) return` и проверку
  `map.getLayer(id)` перед `setLayoutProperty` — устойчиво к раннему вызову
  и к смене стиля.
- `restorePoiState` корректно интегрирован в существующую цепочку
  восстановления: `style.load → onMapStyleLoad → rebuildMapOverlays`
  (`app.js:131`) — POI восстанавливается и при первой загрузке, и при
  переключении темы. Паттерн идентичен `restoreTrailsState`.
- `restorePoiState` не пишет в `localStorage` (восстановление не должно
  иметь побочных эффектов) — поведение задокументировано в JSDoc и
  покрыто тестом TP-03.
- Разделение ответственности: персистентность — только в `onPoiCheckbox`,
  применение видимости — в общем хелпере.

Замечаний P0/P1/P2 нет.

## 4. Качество тестов

`tests/unit/poi_toggle.test.js` исполняет **реальный** код из `app.js`
(блок извлекается по маркерам ET-002 и оборачивается через `new Function`
с мок-зависимостями) — это покрывает риск рассинхрона тестов и продакшн-кода.

- TP-01 — снятие чекбокса: скрытие слоёв + `setItem('poi-visible','0')` + `layerState.poi=false` ✅
- TP-02 — установка чекбокса: показ слоёв + `setItem('1')` + `layerState.poi=true` ✅
- TP-03 — `restorePoiState()` при `'0'`: скрытие + чекбокс снят + без записи в `localStorage` ✅
- TP-04 — `restorePoiState()` без ключа: дефолт «видимы» ✅
- Доп.: значение `'1'`, изоляция чужих слоёв (дух TP-08), синхронизация
  `layerState` без слоёв на карте.

TP-05..TP-07, TP-09 (e2e) и TP-08 (integration) по своей природе не
покрываются юнит-тестом — это ожидаемо и не является замечанием к данному PR.

## Findings

### P3-1 (nice-to-have) — рассинхрон `toggleLayer('poi')` с чекбоксом

`toggleLayer(group)` (`app.js:386-396`) меняет `layerState.poi`, но не
обновляет ни чекбокс `poi-visible-cb`, ни `localStorage`. ADR-0001 п.4
указывает, что состояние кнопки и чекбокса не должно расходиться.

Смягчающие факты: `toggleLayer` не имеет ни одного вызова в кодовой базе,
элемента `btn-poi` в `index.html` нет (вызов `toggleLayer('poi')` упал бы
на `btn.classList`). Фактически это недостижимый код, расхождение
пользователю не наблюдаемо. ADR сам относит унификацию тулбар-кнопок и
popup-чекбоксов к будущему техдолгу (раздел «Технический долг»).

Рекомендация (вне scope ET-002): удалить мёртвый `toggleLayer` либо
оформить unified-контроллер слоёв отдельной задачей.

### P3-2 (nice-to-have) — REQ-NF-03 не проверяется по диффу

Touch target ≥ 44px зависит от CSS-класса `terrain-checkbox` в `app.css`,
который в рамках ET-002 не менялся (ТЗ §3 предписывает переиспользовать
существующий класс). Замечания к коду нет — отметка для приёмки:
подтвердить REQ-NF-03 прогоном e2e-теста TP-09.

### P3-3 (nice-to-have) — неровный отступ в IIFE `initTerrain`

В `app.js:2949-2950` и `2962-2963` вызовы `restoreTrailsState()` /
`restorePoiState()` имеют отступ, не совпадающий с окружающим блоком.
Это предсуществующая стилевая мелочь, которую ET-002 лишь продолжил.
Косметика; рекомендуется выровнять при ближайшем касании файла.

## Итог

Реализация полностью соответствует ТЗ и ADR-0001, код аккуратен, юнит-тесты
исполняют реальный код и покрывают TP-01..TP-04. Найдены только три
замечания уровня P3, ни одно из которых не блокирует мерж.

**Вердикт: APPROVED.**