feat(web): list filters submit once via „Търси" button instead of per-toggle

[DiyanaDimitrova]

Проблем

Списъчните филтри (FilterRail) събмитваха формата при всяка промяна на чекбокс (<Form method="get" onChange={submitForm}>) — по една пълна Worker заявка + цял loader D1 pass на всяко цъкване; избор на N филтъра = N навигации (N× D1 read cost — същата повърхност като Denial-of-Wallet риска в #122). Освен това чекбоксовете бяха controlled (checked=… + no-op onChange), та състоянието им се притежаваше от server-rendered loader данните — цъкването не се отразяваше визуално докато навигацията не приключи (усещане за „заковани"/неотзивчиви на мобилен/бавна връзка).

Closes #181

Промени

• Един submit. Махнат auto-submit-ът; видим бутон „Търси" прави една заявка за N филтъра. Приоритет (по тикета): точност, цена и производителност над instant-filter UX.
• Uncontrolled чекбокси (defaultChecked) — реагират мигновено и работят напълно без JS (прогресивно подобрение към native SSR форма).
• Запазване на URL параметри. Native GET сериализира само полетата на формата, затова всеки текущ non-filter параметър (q от търсенето, authority/bidder scope, …) се пренася през hidden inputs (preservedParamInputs), а cursor/page нямат поле → keyset курсорът се ресетва „безплатно".
• Канонични URL-и. „Всички" radio-тата подават value= (празно); onSubmit прунва само празните group-key полета, за да не изтича ?value=&eu= (без риск за sort/q).
• Плаващ „Търси" pill. Fixed floating компонент, който стои над скролиращия списък и не покрива footer-а (лек scroll effect го повдига над него).
• Без промени по routes или lib/filters.ts — компонентът е споделен от contracts / companies / authorities.
• Тестове (липсваха). Добавени unit тестове за извлечената чиста логика (categorySelectionState, preservedParamInputs, shouldPruneField, filterFormKey) + round-trip тест, който доказва, че native GET submit минава коректно през loader parser-а.

Запазват се споделяемите URL-и: при „Търси" GET формата сериализира избора в query string-а; при зареждане от споделен линк loader-ът го чете и чекбоксовете се рендерират defaultChecked. URL-ът остава източникът на истина.

Test plan

• pnpm --filter @sigma/web test — web тестове (вкл. новите за FilterRail/filters)
• pnpm typecheck — 7 successful
• Ръчна проверка на /contracts, /companies, /authorities:
  • цъкане по няколко филтъра → 0 навигации; чекбоксите реагират мигновено
  • „Търси" → една заявка, URL съдържа всички избрани филтри и без cursor/page
  • „Изчисти филтрите" чисти чекбоксите и URL-а
  • споделен линк (напр. ?sector=45&year=2026) рендерира съответните чекбокси отметнати
  • select-all в категория превключва децата без да събмитва (до „Търси")
  • работи и с изключен JavaScript (native submit)

---

Забележка: клонът изостава от main с 2 commit-а (amendment history #165, in-table search #204). Мога да merge-на main при нужда — вероятно има конфликт в FilterRail.tsx / filters.test.ts.

[nedda76]

Ревю на PR #228 — филтрите се подават наведнъж през бутон „Търси"

Общ преглед

FilterRail минава от контролирани чекчета, които се изпращат при всяко превключване, към uncontrolled полета (defaultChecked) в нативна <Form method="get">, която прилага цялата селекция с една навигация при натискане на „Търси" (issue #181 — така отпада заявката към Worker-а / D1 при всяко чекче). Чистата логика е изнесена в filterRail.logic.ts с unit тестове; формата е с key според приложените филтри, за да се remount-не и да презаредят defaultChecked при „Изчисти", back/forward и споделен линк. Добре обмислена промяна.

Коректност

🔴 СРЕДЕН — onSubmit изключва (disabled) радио бутоните „Всички" и те остават изключени, когато формата не се remount-ва. В onSubmit полетата с празна стойност се маркират disabled, за да не влязат ?value=&eu= в GET заявката. За самото изпращане това е коректно, но полетата никога не се връщат обратно — възстановяват се единствено при remount на <Form key={formKey}>, а filterFormKey изключва cursor/page/sort. Заради това:

• Възпроизвеждане: на страница ≥2 (или състояние с cursor) се натиска „Търси" без промяна по филтрите. Нативното изпращане маха cursor/page → новият URL дава същия formKey → няма remount → всички радио бутони „Всички" остават disabled и не реагират, докато не се смени филтър или не се презареди страницата. (Същото и при всяко повторно изпращане на идентична селекция.)
• React няма как да ги върне: disabled е зададено императивно и никога не е било в JSX-а, така че следващ render не го нулира.
• Поправка: връщане на полетата в активно състояние след изпращането, напр. queueMicrotask(() => elements.forEach((el) => (el.disabled = false))) в onSubmit — React Router чете FormData синхронно в същото събитие, така че microtask-ът връща контролите, без да засяга току-що подадените данни. (Алтернатива: „Всички" да е радио без name, като се разчита на „няма избрана опция ⇒ параметърът липсва ⇒ всички" — тогава изключването изобщо не трябва.)

🟡 НИСЪК — при недовършена селекция състоянието на uncontrolled полетата изостава (заложено в дизайна). Тъй като нищо не се пре-render-ва до изпращане: броячът appliedCount на бутона отразява приложения URL, а не текущо избраното, и „избери всички" за категория не обновява checked/indeterminate/aria-checked, докато посетителят маркира отделните чекчета. Всичко това е съзнателно и приемливо, но брояч точно върху бутона за изпращане, показващ старата стойност, може леко да подведе („Търси · 2", докато са маркирани 5). Струва си да се потвърди дизайнерски.

Качество на кода и конвенции — силно

• Изнасянето на чистата логика в filterRail.logic.ts (без React/DOM) е точно по конвенцията за тестваемост; функциите са малки, чисти и всяка носи коментар „защо".
• Добро внимание към достъпността: indeterminate се задава през ref, има aria-checked="mixed" и aria-label на категорийния бутон.
• Работи и без JS — нативна форма + defaultChecked + бутон; изчистването на празните параметри и повдигането над футъра са само надграждане при наличен JS, а loader-ът чете празната стойност като „без филтър" (покрито от round-trip теста).
• CSS ползва токените на дизайн системата; useEffect с dependency formKey коректно се закача за новия node на плаващата лента след remount (обяснено в коментара), с rAF throttling, passive listener-и и пълно почистване.

Производителност

• Основното — една навигация вместо по една при всяко чекче — е ясна печалба (няма извикване на Worker / D1 при всяко превключване).
• Handler-ът за скрол е с rAF и passive; цената е пренебрежима.

Тестове

• filterRail.logic.test.ts е изчерпателен (състояние на категория при празно/частично/надмножество, preservedParamInputs с q/bids/scope/повтарящи се стойности, shouldPruneField, стабилност на filterFormKey при страниране и сортиране).
• filters.test.ts добавя реално покритие за широко ползвания, но досега нетестван withParams, плюс round-trip през loader-а за формата на нативния GET. Хубаво.
• Липсва: интеграционен / e2e тест за самото DOM поведение — remount при смяна на formKey, изчистването при изпращане, находката с disabled бутоните. Точно този клас регресия unit тест няма да хване; тест с Playwright/RTL за „маркиране на филтър → Търси → URL се сменя веднъж; страниране → Търси → бутоните пак работят" щеше да покаже СРЕДНАТА находка.

Сигурност

Без забележки. Стойностите на скритите полета идват от URLSearchParams и се екранират от React (няма инжекция); няма нови заявки, тайни или промяна в автентикацията. Повърхността на атака е същата.

Дребни

• Плаващата лента (position: fixed, z-index: 30) стои над съдържанието в долния край; повдигането гледа футъра, но струва си да се провери дали не покрива последния ред / контролите за страниране на десктоп и дали z-index: 30 стои под евентуален header / модал.
• @media (min-width: 960.02px) — коментарът казва „no dead zone", но всъщност само свива мъртвата зона до (960, 960.02]; на практика нула, а базовите стилове се държат коректно, така че е ок — само коментарът е леко пресилен.

Заключение

Добре направена промяна, по конвенция, с добри тестове и реална печалба в производителността. Една СРЕДНА находка за оправяне преди merge (изключените „Всички" радио бутони след изпращане без remount — лесна поправка с microtask) плюс предложение за e2e тест на основния поток. Останалото е по избор.

[nedda76]

🔴 СРЕДЕН (находката от ревюто по-горе): тук disabled се задава, но полетата никога не се връщат активни. Възстановяват се единствено при remount на <Form key={formKey}>, а filterFormKey изключва cursor/page/sort — значи „Търси" от страница ≥2 (или повторно изпращане на идентична селекция) дава същия formKey, няма remount и радио бутоните „Всички" остават disabled и не реагират до смяна на филтър / презареждане.

Поправка — връщане на полетата активни веднага след изпращането:

const onSubmit = (e: FormEvent<HTMLFormElement>) => {
  const pruned: HTMLInputElement[] = [];
  for (const el of Array.from(e.currentTarget.elements)) {
    const input = el as HTMLInputElement;
    if (shouldPruneField(input, groupKeys)) {
      input.disabled = true;
      pruned.push(input);
    }
  }
  queueMicrotask(() => pruned.forEach((el) => (el.disabled = false)));
};

React Router чете FormData синхронно в същото събитие, така че microtask-ът връща контролите, без да засяга вече подадените данни. (Алтернатива: „Всички" да е радио без name — тогава изключването изобщо не трябва.)

[nedda76]

Повторно ревю — последните промени (33cbf22..34381c4)

Блокиращата находка (MEDIUM) е решена коректно. 5a8e70a събира изключените полета и ги връща активни през queueMicrotask след submit-а (React Router вече е прочел FormData синхронно), с guard на pruned.length — точно поправката от коментара. „Всички" радио бутоните вече не могат да останат заключени след повторно изпращане без remount.

CSS-промените също са добри и обосновани:

• chrome.css — overflow-x: clip на main спира широка таблица без пренасяне да избутва страницата и да поражда хоризонтален скрол (който отрязваше левия край и правеше бутона „Търси" да изглежда по-широк). clip, не hidden — не създава скрол контейнер, така че sticky и fixed позиционирането остават невредими.
• layout.css — grid-template-columns: 220px minmax(0, 1fr) (базово + мобилно) оправя класическото разливане на 1fr грида; box-sizing: border-box на .filter-apply спира бутона да излиза 66px извън колоната си на мобилно.
• Коментарът за 960.02 вече е точен (свива, а не премахва мъртвата зона) — добре уловено.

Без регресии: тестовете за чистата логика не са засегнати (filterRail.logic.ts е непроменен), а TSX промяната е малка и типизирана.

Остават две незадължителни неща (не блокират): броячът и indeterminate при недовършена селекция остават спрямо приложеното (заложено в uncontrolled дизайна), а самата re-enable поправка няма регресионен тест (DOM + microtask поведение, трудно за unit тест; логиката shouldPruneField вече е покрита). Playwright/RTL тест за потока „маркиране → Търси → странициране → Търси → бутоните пак работят" би затворил и това, но за после.

Одобрявам след зелена CI. Чиста, добре тествана промяна с реална печалба в производителността.

[lyubomir-bozhinov]

Прегледах на връх 34381c4 — sound, без блокери. Auto-submit-ът е напълно махнат; submit-ва се веднъж през „Търси“. Три дребни (non-blocking) наблюдения:

1. „Избери всички“ остава stale след взаимодействие с child-ите (FilterRail.tsx ~190). Чекбоксът е uncontrolled (defaultChecked={allSelected}); inline ref-ът обновява само indeterminate на всеки render, но checked не се пресинхронизира. След ръчно махане на всички child-и в категорията someSelected=false → indeterminate=false, а checked остава true от предишното състояние → „избери всички“ се вижда отметнато при нула избрани. Козметично и се нулира при следващ submit/навигация (form-ът е keyed), но между два submit-а подвежда. Fix: пресметни checked/indeterminate и в onCategoryChange.

2. filterFormKey е чувствителен на реда на параметрите (filterRail.logic.ts:58-63). Връща next.toString(), който пази insertion order на sp. Два URL-а със същия filter set, но разбъркан ред (споделен линк, ръчно редактиран URL, back/forward), дават различен key → form-ът се remount-ва и колапсва <details>/фокуса — точно това, което key-ът съществува да ИЗБЕГНЕ (виж собствения docstring). Fix: sort-ни параметрите преди toString() (напр. по канонична подредба à la #222) за стабилен key.

3. Няма aria-busy/aria-live на rail-а по време на навигация (a11y enhancement). Докато loader-ът върви след „Търси“, screen-reader потребител няма сигнал. Вържи aria-busy на <aside className="filter-rail"> към useNavigation().state !== 'idle'.

Нищо от трите не блокира merge.

[nikimilenkov]

Благодаря за PR-а — работата е солидна: чисто минаване към uncontrolled inputs, изнесена и тествана логика, работи без JS, и Medium-ът на @nedda76 (заключени „Всички" радио бутони) е коректно оправен на HEAD. Остават обаче два проблема — и двата на основния път — които блокират merge.

🔴 Висок — сигурност: preservedParamInputs връща #197 cache poisoning (CWE-349)

filterRail.logic.ts:28-38 пренася всеки URL параметър освен собствените на формата — включително непознати. Затова GET /contracts?zzz=<payload> рендерира <input type="hidden" name="zzz" value="…"> в SSR тялото; cache-key.ts маха zzz от ключа → отровеното тяло се кешира под чистия /contracts (contracts.tsx е publicCache(1800)) и се сервира на всички посетители.

• Това е регресия от този PR — старият код пренасяше само q/authority/bidder (всички в cache allow-list-а). „Пренасяй всичко" чупи инварианта на #197 (нищо unkeyed да не влиза в кешираното тяло).
• Не е XSS (React escape-ва и името, и стойността; input-ът е инертен) — а инжектиране на unkeyed съдържание / cache poisoning, на трите кеширани списъчни route-а.
• Drift guard-ът в cache-key.test.ts не го хваща — сканира routes/** + lib/filters.ts за литерални четения, а тук параметрите се четат генерично през sp.entries() в components/.
• Поправка: ограничи пренасянето до allow-list-а — if (CACHE_QUERY_PARAMS.has(key) && !owned.has(key)) — така непознатите падат. Плюс тест: preservedParamInputs(sp('zzz=poison'), …) → [].

🔴 Висок — достъпност: фокусът се губи при всяко „Търси" (WCAG 2.4.3)

<Form key={filterFormKey(sp)}> — при „Търси" след смяна на филтър ключът се сменя → React remount-ва <Form>-а и размонтира бутона, върху който е бил клавиатурният фокус → фокусът пада на <body>, безшумно, без нищо да се премести към заглавие на резултатите / live region. Това е основният сценарий, който PR-ът въвежда (един бутон за прилагане): клавиатурният потребител трябва да табва от началото, а screen reader не получава съобщение какво се е случило.

• Поправка: след навигацията премести фокуса умишлено (към заглавие на резултатите или aria-live статус), или не remount-вай целия <Form> (нулирай само input-ите).

🟡 Среден — достъпност: няма обратна връзка за неприложена селекция (WCAG 4.1.3)

Натискането на чекбокси вече е безшумно (нито визуално „N избрани, неприложени", нито aria-live) до „Търси" — SR потребител няма как да разбере, че отметките му са регистрирани. Шаблонът вече го има в кода (ListControls.tsx ползва role="status"/aria-live); приложи го и тук.

🔵 Ниски

• Достъпност (възможен): императивният disabled на „Всички" радио бутони при submit може да blur-не фокуса към <body>, ако някой е фокусиран — зависи от браузъра, вероятно недостижимо днес, но лесно се чупи при бъдеща промяна на кода.
• Тест-пропуск: няма тест с непознат параметър, който да заключи allow-list инварианта.
• React: неприложени отметки оцеляват при paging навигация със същия formKey; „избери всички" чекбоксът застоява след ръчна смяна на дъщерна отметка (и двете са присъщи на модела „натрупай и приложи" — продуктово решение).

✅ Проверено и чисто

Uncontrolled defaultChecked (без hydration mismatch); re-enable на радио бутоните (queueMicrotask е timing-safe, RR чете FormData синхронно преди него); native GET без JS; cursor/page reset към страница 1; multi-value/encoding; label/форма семантика, клавиатура, contrast, видим фокус, prefers-reduced-motion. Medium-ът на @nedda76 е оправен и пълен на HEAD.

---

Вердикт: Изисквам промени — блокиращи са двата „Висок" (сигурност + достъпност/фокус). Останалото е дребно.

[lyubomir-bozhinov]

Одобрявам на връх 34381c4. Auto-submit-ът е напълно махнат; submit-ва се веднъж през „Търси“, логиката е чиста. Трите бележки в предходния ми коментар (stale „избери всички“, order-sensitive filterFormKey, липсващ aria-busy) са non-blocking — не са условие за merge; хубаво е да се адресират в follow-up. mergeable, остава required CI да мине зелено.

[cefothe]

Ревю на PR #228 — преглед по dimensions

Прегледах кода спрямо действителното състояние на клона (c3e1800), workers/cache-key.ts, loader-а/lib/filters.ts и merge-статуса. Обобщение по-долу.

🔒 Сигурност — ЧИСТО

Няма твърдо кодирани тайни, нови зависимости или подозрителни URL-и. Ключовата промяна тук всъщност затваря регресия за cache poisoning (CWE-349, #197): preservedParamInputs вече пренася само параметри от CACHE_QUERY_PARAMS, внесен като единствен източник на истина от cache-key.ts. Разсъждението е коректно — drift guard-ът в cache-key.test.ts пази това множество изчерпателно, така че всеки пренесен hidden input задължително е част от cache ключа. ✅

✅ Коректност и дизайн — силно

• Native, прогресивно подобрена GET форма; uncontrolled defaultChecked; cursor/page без поле → keyset reset „безплатно".
• filterFormKey е order-insensitive (сортиране) — споделен/пренареден URL не remount-ва формата и не срутва <details>/фокуса.
• Чистата логика е изнесена и покрита с unit тестове + round-trip тест през реалния loader parser.
• withParams още се ползва от routes (contracts/companies/authorities) — не е dead code.
• Няма риск от hydration mismatch: всички derivations са детерминистични от URL-а.

📋 Забележки

1. Дребно / локализация — противоречиво съобщение за екранни четци. FilterRail.tsx:159:

„Има непроменени филтри. Натиснете „Търси", за да ги приложите."

„непроменени" противоречи на „приложите ги". Предлагам напр. „Има неприложени промени по филтрите." Само за screen reader, но това е точно съобщението за pending промени.

2. Дребно / покритие. Тествана е само чистата логика. React-логиката, която регресира два пъти по време на ревюто — възстановяване на фокуса след keyed remount (WCAG 2.4.3), syncSelectAll, onSubmit prune + queueMicrotask re-enable — няма jsdom/RTL тест. Точно тези части носят риск.

3. Инфо / визуална проверка. main { overflow-x: clip } (chrome.css) заедно с position: fixed за .filter-apply-bar, който е DOM наследник на main. Разсъждението е вярно (main не е containing block за fixed → не се clip-ва), но това поведение е чувствително към браузъра (исторически Safari). Струва си един ръчен cross-browser smoke (особено Safari/iOS), че pill-ът не се отрязва.

4. Дребно / остаряло. Бележката в описанието, че клонът изостава от main с 2 commit-а с вероятен конфликт, е невярна вече: спрямо midt-bg/main е 0 назад, MERGEABLE, без конфликт (q от #204 вече е интегриран). BLOCKED статусът е само заради чакащо одобрение на ревю.

Заключение

CI: зелено. Препоръка: APPROVE. Единствено забележка №1 бих оправил преди merge; останалите са по желание/за проверка. Много чиста commit хигиена и коментари, обясняващи „защо" — включително истинска печалба за сигурността.

[ydimitrof]

Преглед на PR: feat(web): филтрите се подават еднократно чрез бутона „Търси“ вместо при всяко превключване

ВЕРДИКТ: COMMENT — няма блокиращи проблеми по сигурност или коректност; има 2 дребни забележки (текст за екранни четци + един пропуск по достъпност).

---

Обобщение

Много добре изпипан PR. Преходът от авто-подаване при всяко превключване към еднократно подаване с „Търси“ (issue #181) е реализиран чисто: неконтролирани input-и (defaultChecked), нативна <Form method="get">, key-ната форма се ремоунтва при смяна на URL, добавени са грижи за достъпност (възстановяване на фокус, aria-busy, polite live region) и чисти помощни функции с добро unit покритие. Тестовете са смислени и целят да разкрият дефекти, а не да минат тривиално.

---

Фаза 0 — Скан за сигурност: ЧИСТО ✅

• Тайни/ключове: няма hardcoded секрети, токени или пароли.
• Зависимости: няма нови или променени пакети.
• URL адреси: няма нови външни URL/endpoint-и.
• Инжекции/XSS: стойностите на скритите <input> идват от URLSearchParams и се рендират като атрибути през JSX → React ги екранира. Няма dangerouslySetInnerHTML, няма конкатенация в SQL/HTML, няма backdoor/обфускация.
• Cache poisoning (CWE-349): preservedParamInputs умишлено пренася само параметри от allow-листата CACHE_QUERY_PARAMS, така че непознат ?zzz=<payload> не попада в SSR тялото на publicCache-нат маршрут. Коректна защита, покрита с тестове (utm_source=poison, zzz=<script>). Одобрено.

Забележка за проверка: тъй като apps/web не е наличен локално в тази среда, не можах да потвърдя, че CACHE_QUERY_PARAMS в workers/cache-key.ts съвпада 1:1 с ключа на кеша. Логиката разчита изцяло на това съвпадение — моля потвърдете, че всеки параметър, който loader-ът чете и който влияе на отговора, е в CACHE_QUERY_PARAMS; иначе такъв параметър ще бъде тихо изхвърлен при подаване на филтри.

---

Коректност и жизнен цикъл (React) — ОК

• Ремоунтът по filterFormKey (изключва cursor/page/sort, сортиран → нечувствителен към реда) коректно преприлага defaultChecked при „Изчисти“, back/forward и споделени връзки, без да събаря отворените <details> и фокуса при странициране/сортиране.
• onSubmit prune-ва празните group-key контроли чрез временно disabled и ги възстановява през queueMicrotask след синхронния прочит на FormData от React Router — правилно решен и коментиран крайният случай „повторно подаване без ремоунт“.
• Скрол-ефектът за повдигане над footer-а чисти listener-ите и requestAnimationFrame — няма изтичане на ресурси.
• Възстановяването на фокус е предпазливо (само ако фокусът е паднал на <body>), което е правилно.

---

Забележки (не-блокиращи)

1. Текст за екранни четци — вероятна грешка в думата. Статусът гласи „Има непроменени филтри…“, но смисълът (и коментарът в кода: „pending, not-yet-applied changes“) е за НЕПРИЛОЖЕНИ промени. „непроменени“ = unchanged, което е обратното. Предложение: „Има неприложени промени. Натиснете „Търси“, за да ги приложите.“ Виж inline коментара.

2. Достъпност (WCAG 4.1.3) — „Избери всички“ не задейства анонса за чакащи промени. onCategoryChange вика e.stopPropagation(), така че change събитието на „Избери всички“ не стига до делегирания onFormChange, който вдига dirtyRef и обявява „Има неприложени промени“. Понеже програматичните .checked записвания по децата не пораждат change събития, кликът върху „Избери всички“ променя селекцията, но остава напълно тих за екранния четец — за разлика от превключването на отделен чекбокс. Виж inline коментара за възможен фикс.

---

Тестове

Покритието на чистата логика е добро и смислено (categorySelectionState, preservedParamInputs, shouldPruneField, filterFormKey, round-trip на нативния GET през loader-а). Самият компонент (DOM/фокус/live region/footer-lift) няма тестове — приемливо предвид native-form подхода, но горните два случая (анонс при „Избери всички“ и текстът на статуса) не са покрити и затова дефектите се промъкнаха.

---

Заключение

Няма проблеми по сигурност или коректност, блокиращи merge. Препоръчвам да се коригира текстът на статуса (т.1) и по желание пропускът по достъпност (т.2) преди merge. Останалото е готово за продукция.

[ydimitrof]

Вероятна грешка в текста за екранни четци: „непроменени“ означава unchanged, но намерението (и коментарът по-горе — „pending, not-yet-applied changes“) е за НЕПРИЛОЖЕНИ промени. Предложение:

setStatus('Има неприложени промени. Натиснете „Търси“, за да ги приложите.');

Това е единственият видим (за екранен четец) низ, който описва състоянието, така че точността тук има значение за WCAG 4.1.3.

[ydimitrof]

Достъпност (WCAG 4.1.3): e.stopPropagation() тук спира change събитието на „Избери всички“ да достигне делегирания onFormChange (ред 156+), който вдига dirtyRef и обявява „Има неприложени промени“. Понеже програматичните .checked записвания по децата в onCategoryChange не пораждат change събития, кликът върху „Избери всички“ променя селекцията, но остава напълно тих за екранния четец — за разлика от превключването на отделен чекбокс.

Възможен фикс: маркирайте dirty състоянието директно в onCategoryChange (напр. извикайте същата логика, която вдига dirtyRef/setStatus), вместо да разчитате на бълбукането, което тук умишлено спирате.

[lyubomir-bozhinov]

Одобрявам наново на връх c3e1800. Трите бележки от ревюто са адресирани, и — браво — сама намери и затвори реален cache-poisoning вектор (CWE-349), който аз бях пропуснал:

• cache-poisoning fix (коректен и без регресия): preservedParamInputs вече емитва само параметри от CACHE_QUERY_PARAMS. Проверих: това е точно множеството, по което cache-key.ts (ред 51) строи ключа, а CI drift-guard-ът гарантира, че всеки консумиран от loader параметър е вътре — тъй че ограничението не може да изпусне легитимен параметър (пада само реално неизползван шум като utm_source/zzz). Точно допълва #222 откъм body-то.
• filterFormKey .sort() — order-insensitive, коректно; тестът го покрива.
• a11y — aria-busy, polite live region и връщане на фокуса към „Търси“ след remount (WCAG 2.4.3/4.1.3) — над това, което поисках.

Едно дребно (non-blocking) за follow-up: „избери всички“ се клобва при първия toggle на editing burst. Inline ref-ът (FilterRail.tsx:265-266) пише indeterminate от APPLIED състоянието на всеки render, а setStatus в onFormChange предизвиква точно такъв render, който презаписва живата стойност от syncSelectAll. Repro: категория с applied „всички избрани“, махни първото дете → „избери всички“ се показва без тире/без отметка, докато деца още са чекнати (само първият toggle; следващите не викат setStatus, тъй че остават верни). Козметично — децата носят реалния submit. Fix: махни indeterminate write от inline ref-а и остави syncSelectAll да е единственият източник (или re-sync след render).

Одобрението стои; дребното не блокира merge.

[nikimilenkov]

Прегледах отново на HEAD c3e1800 — двата блокиращи проблема са затворени и проверени. Оттеглям „Изисквам промени" и одобрявам.

✅ Сигурност — cache poisoning (CWE-349): оправено. preservedParamInputs вече пренася само параметри от CACHE_QUERY_PARAMS, така че непознат ?zzz=… не влиза в SSR тялото. Плюс два регресионни теста (zzz=<script> → пада, utm_source → []), които заключват инварианта. 👍

✅ Достъпност — загуба на фокус (WCAG 2.4.3): оправено. След „Търси" фокусът се връща на бутона; таймингът е коректен (buttonRef сочи новия бутон след remount-а), guard-ът activeElement === body не краде фокус, а submittedRef ограничава ефекта само до реален submit. Като бонус syncSelectAll оправи и застоялия „Избери всички".

Няколко неблокиращи дреболии за после:

• Текст (@cefothe също го отбеляза): FilterRail.tsx:159 „Има непроменени филтри…" — „непроменени" противоречи на „приложите"; трябва „неприложени" (напр. „Има неприложени промени по филтрите.").
• Обхват на live region-а. busy/aria-busy идват от глобалния useNavigation(), затова при paging/сортиране панелът също маркира „зареждане" и status може да се преобяви — дублира собствения live region на ListControls. Не е WCAG 4.1.3 нарушение (съобщенията присъстват коректно), само малко шум; при желание — ограничи го до submittedRef.
• „Избери всички" не задейства съобщението за неприложени промени (stopPropagation го спира) → редакция само през него е безшумна за screen reader до „Търси".
• Index-базиран key на preserved hidden input-ите (${key}-${i}) — по-добре ${key}-${value}-${i}.

Иначе — много чиста работа: истинска печалба за сигурността и солиден a11y fix. Одобрявам.

[lyubomir-bozhinov]

Прегледах стриктно на връх 402123a. Силна, добре обмислена работа — single-submit („Търси") вместо навигация на всеки toggle (#181). Логиката е изнесена в чисти функции с реални тестове; a11y и cache-безопасността са адресирани. Одобрявам, с няколко дребни бележки (не блокират).

Каквото проверих, че държи:

• CWE-349 (cache poisoning) е затворен коректно: preservedParamInputs пренася като hidden inputs само параметри от CANONICAL_QUERY_PARAMS (и не-owned) — произволен ?zzz=payload се дропва, тъй че не може да инжектира неключиран body в publicCache-нат list route. Тестът заковава и регресиите от #181 (submit да не изтрива q/bids).
• Uncontrolled inputs + keyed <Form>: defaultChecked + remount по filterFormKey (без cursor/page/sort) → clear/back-forward/споделен линк пре-прилагат URL-състоянието, а paging/sort не ремоунтват (пазят отворените групи + фокуса). Коректен модел.
• Мобилно: floating pill-ът е само @media (min-width: 960.02px); под 960px барът е in-flow в края на рейла (в CSS-only свития disclosure) — няма fixed-overlay да покрива последния ред. minmax(0, 1fr) (вместо 1fr) спира хоризонталния overflow от широката таблица/дълги имена — добра диагноза; box-sizing: border-box фиксът на бутона също.
• A11y: фокусът се връща на ремоунтнатия „Търси" след settle (WCAG 2.4.3), polite live-region за „Зареждане…/обновени", aria-busy. Ресурсите (scroll/resize/raf) се чистят в cleanup-а.

Дребни (низходящ приоритет, не блокират):

1. q остава в filterFormKey (за разлика от sort/cursor/page), тъй че смяна на търсенето ремоунтва рейла. По собствената ти логика („изключваме каквото не мени кои чекбокса са отметнати") q също отговаря на условието. Ефектът е малък, защото open={someSelected} връща детерминиран изглед, но за консистентност — обмисли next.delete('q') в filterFormKey.
2. Toggle на „Избери всички" е тих за screen reader: onCategoryChange вика stopPropagation(), тъй че onFormChange-announcement-ът „има непроменени филтри" не гръмва, а програматичните .checked по децата не пускат change-събития. За иначе много a11y-внимателен PR — струва си да се announce-не и тук.
3. Prune→re-enable в onSubmit разчита React Router да чете FormData синхронно преди микротаска. Worst case при промяна е фрагментиране на edge-кеша (?value=/?eu= празни), не грешни данни — приемливо, но една e2e проверка (вържи с #238) би заковала таймингa.

Одобрявам.

[ydimitrof]

Ревю на PR: списъчните филтри се подават наведнъж чрез бутона „Търси“ (issue #181)

ВЕРДИКТ: COMMENT — препоръчвам корекции преди merge; няма блокиращи проблеми със сигурността.

Фаза 0 — Скан за сигурност: ЧИСТО ✅

• Без hardcoded тайни (API ключове/пароли/токени) — няма нито една.
• Без промени по URL / нови външни домейни — цялата промяна е клиентска (native <Form method=get>).
• Без зловреден код — няма backdoor-и, code injection, обфускация, eval или dangerouslySetInnerHTML.
• Без нови зависимости — vitest вече е в проекта; няма добавени пакети.
• Без SQL повърхност — промяната е чисто фронтенд; няма заявки към D1/база, така че SQL injection е неприложимо тук.
• XSS: стойностите на скритите инпути идват от URL, но се рендират като атрибути през React (авто-escape); неизвестните параметри дори не се пренасят. Няма XSS вектор.
• Cache poisoning (CWE-349) — това всъщност е силна страна на PR-а: preservedParamInputs пренася само параметри от allow-листата CANONICAL_QUERY_PARAMS, което гарантира, че всеки пренесен параметър е част от cache ключа и не може да инжектира unkeyed съдържание в publicCache-нат route. Отлично покрито и с тест.

OWASP: не откривам нарушения (A03 Injection / A01 Access control / A05 Misconfiguration) в обхвата на промяната.

Силни страни

• Чисто разделяне на логиката (filterRail.logic.ts) от компонента; чистите функции са смислено покрити с тестове (празна категория, стойности извън категорията, повтарящи се параметри, order-insensitive ключ).
• Прогресивно подобрение: работи и без JS; keyset cursor се нулира коректно чрез изпускане на cursor/page.
• Внимание към достъпност (live region, връщане на фокус, aria-busy) и към round-trip на URL през loader-а (тест „native FilterRail GET submit round-trips“).
• Реализацията съответства на issue #181: една навигация вместо заявка на всяко превключване.

Забележки (некритични)

1. Подвеждащо съобщение за екранни четци (локализация). setStatus('Има непроменени филтри…') — „непроменени“ означава „unchanged“, а намерението е „неприложени/непотвърдени“ промени. Виж inline коментара.
2. Пропуск в достъпността при „Изчисти филтрите“. Възстановяването на фокуса е само за подаване през „Търси“ (submittedRef); линкът „Изчисти“ е вътре в keyed <Form> и при remount фокусът пада на <body> (същият WCAG 2.4.3 проблем, който бутонът избягва). Виж inline коментара.
3. Крехка връзка с реда на изпълнение в React Router. onSubmit разчита, че RR извиква потребителския onSubmit преди да прочете FormData (за да сработи disable-пруненето). Ако бъдеща версия на RR смени този ред, пруненето тихо ще спре да работи и празните ?value=&eu= ще се върнат. Струва си кратък коментар/тест-guard около това допускане.

Тестове и покритие

Чистата логика е добре тествана. Ефектите на компонента (връщане на фокус, footer-lift, syncSelectAll) не са покрити с unit тестове — авторите го обосновават (браузърно поведение на native form). Приемливо, но покритието на клоновете на компонента остава под целевото; при възможност добавете лек DOM/e2e тест за „select all“ синхронизацията и връщането на фокуса.

Извод: висококачествен, добре обоснован PR без проблеми със сигурността; горните три козметични/UX бележки да се адресират преди merge.

[ydimitrof]

Локализация: съобщението за екранни четци е подвеждащо. „непроменени“ означава „unchanged“, но тук намерението е точно обратното — има променени, но още неприложени филтри (виж коментара по-горе: „pending, not-yet-applied changes“). Предложение: „Има неприложени филтри. Натиснете „Търси“, за да ги приложите.“ (или „непотвърдени филтри“). Иначе screen-reader обявява семантично грешно състояние (WCAG 4.1.3).

[ydimitrof]

Пропуск в достъпността, симетричен на грижливо решения случай с „Търси“: този ефект връща фокуса само когато навигацията идва от submit (submittedRef). Но линковете „Изчисти филтрите“ и „Изтегли CSV“ са вътре в keyed <Form> — кликването на „Изчисти“ сменя formKey, ремонтира формата и фокусът пада на <body> без възстановяване (същият WCAG 2.4.3 проблем, който покривате за бутона). Обмислете да маркирате и clear-навигацията (напр. флаг при onClick на линка) и да върнете фокуса към стабилен елемент след remount.

[ydimitrof]

Добра защита срещу cache poisoning (CWE-349): allow-листата CANONICAL_QUERY_PARAMS гарантира, че всеки пренесен параметър е част от cache ключа. Едно съображение за поддръжка: тази проверка създава невидима зависимост — ако бъде добавен нов канонизиран параметър, който НЕ е част от cache ключа в cache-key.ts, guard-ът тихо ще започне да го пренася в SSR тялото. Струва си кратък коментар/тест, който да закрепи инварианта „CANONICAL_QUERY_PARAMS ⊆ cache-key params“, за да не се разминат двата списъка при бъдещи промени.

[lyubomir-bozhinov]

Прегледах #228 на дълбочина срещу дифа (връх b0b6c7b2) — чисто, силна работа. Проверих независимо носещото твърдение за сигурност:

CWE-349 защитата е плътна, защото двете страни делят един източник. preservedParamInputs емитва скрити полета само за CANONICAL_QUERY_PARAMS, а cache-key.ts:21 ключова кеша точно по същия сет (if (CANONICAL_QUERY_PARAMS.has(key))), импортнат от същия app/lib/query-params.ts. Тоест всеки пренесен параметър е част от кеш-ключа → различна стойност не може да колабира върху чужд URL entry, а ?zzz=<script> просто отпада. Ако двата сета се бяха разминали, документираната защита щеше да е фалшив комфорт — но не са. Тестът „drops params outside the cache allow-list … CWE-349" го заковава с реален payload.

a11y е добре обмислено. Keyed <Form> remount-ва при apply → бутонът, който посетителят току-що е натиснал, се unmount-ва и фокусът пада на <body> (WCAG 2.4.3); ефектът връща фокуса на remount-натия „Търси" след settle + polite live-region за pending/loading/updated (4.1.3). filterFormKey е order-insensitive (.sort()), тъй че споделен линк с друг ред на параметрите не remount-ва напразно.

Прогресивно (native GET <Form> работи без JS; празните „Всички" радиота се prune-ват само с JS, безвредни без него). Логиката е изнесена в чист модул с дискриминиращи тестове (вкл. security инварианта). Одобрявам по същество.

[nedda76]

Прегледах компонента, чистите помощници и стиловете. Много добър рефакторинг — изнасянето на дериватите в filterRail.logic.ts с unit тестове е точно правилният ход, а прогресивното подобрение (native <Form>, uncontrolled defaultChecked, keyed remount) е издържано.

Потвърждава се

• Един submit вместо N навигации — пряко сваля повърхността от #122/#181; бутонът „Търси" винаги е видим (не само в <noscript>), тъй че работи и без JS.
• preservedParamInputs носи само параметри от CANONICAL_QUERY_PARAMS — това затваря cache-poisoning дупката (CWE-349, същият клас като #197): скрит input в SSR тялото на publicCache-ната листа не може да вкара некийнат payload. Тестът zzz=<script> го заковава.
• filterFormKey изключва cursor/page/sort и сортира параметрите → remount само при реална промяна на филтрите, стабилен при пагинация/пресортиране и при разбъркан ред на URL-а (shared link). Добре тествано.
• Фокус мениджмънтът след remount (връщане на фокуса към „Търси" само ако е паднал на <body>) и aria-live регионът адресират WCAG 2.4.3 / 4.1.3. Мислено.

Въпроси / бележки

1. Формулировка (ЗА поправка). Живият регион при промяна казва „Има непроменени филтри. Натиснете „Търси"…". „непроменени" е подвеждащо — потребителят току-що ги е променил, но още не са приложени. По-точно е „Има неприложени филтри" (или „непотвърдени промени").
2. queueMicrotask за връщане на disabled. Разчита, че React Router чете FormData синхронно в submit събитието (вярно за RR v7). Ако някога това стане отложено, изчистените празни „Всички" радио-та ще изтекат обратно в URL-а. Коректно е днес — само си струва един ред коментар, който да закове допускането (или мутационен тест: submit без remount → URL без ?value=).
3. overflow-x: clip (дребно). Safari го поддържа от 16 (2022). На по-стар Safari декларацията се игнорира и хоризонталният overflow, който тя пази, може да се върне. Приемливо като прогресивно, само го отбелязвам.

Солидно откъм моя страна, готово за мърдж след дребната корекция на текста в т.1. Само преглед — самият мърдж не е мой.

[ydimitrof]

Преглед на PR: филтрите се прилагат с един „Търси“ бутон вместо при всяко превключване

Обобщение

Промяната е добре структурирана и целенасочена. Чистата логика е изнесена в filterRail.logic.ts (без React/DOM) и е покрита със смислени unit тестове, а round-trip през loader-а е тестван в filters.test.ts. Прогресивното подобрение е запазено (работи и без JS чрез native Form method=get), достъпността е обмислена (възстановяване на фокус, role=status, aria-busy), а нулирането на cursor/page идва „безплатно“ от липсата на съответните полета.

Сигурност (Фаза 0 — чисто, дори подобрено)

• Няма hardcoded тайни, нови зависимости или подозрителни URL промени.
• preservedParamInputs ограничава пренасяните hidden полета до CANONICAL_QUERY_PARAMS — това е реална защита срещу cache poisoning (CWE-349): произволен ?zzz=<payload> не може да инжектира некеширано съдържание в SSR тялото на publicCache-нат route. Тестовете го покриват изрично. Одобрено.

Тестове

• Добро покритие на чистата логика, вкл. гранични случаи (празна категория, Set вход, повтарящи се стойности, регресии #181/#197/#228).
• Забележка: интерактивното поведение на самия компонент (delegated onFormChange, syncSelectAll, prune-при-submit с queueMicrotask re-enable, възстановяване на фокуса) не е покрито от тест — разчита на браузъра. Приемливо предвид uncontrolled подхода, но е зона без автоматична защита при регресия.

Констатации (незадължителни, но препоръчителни)

1. Подвеждащ текст за екранни четци (FilterRail.tsx): съобщението „Има непроменени филтри…“ е логически противоречиво — „непроменени“ значи unchanged, а намерението е неприложени/чакащи промени. Вж. инлайн коментара.
2. Глобалното navigation.state задвижва анонса (FilterRail.tsx, ред 76): busy реагира на всяка навигация в приложението (пагинация, смяна на подредба, клик върху резултат към детайлна страница), а не само на подаване на филтрите. Затова aria-busy върху сайдбара и анонсът „Зареждане на резултатите…“ се четат и при несвързани навигации (напр. напускане към детайлна страница). Обмислете ограничаване на анонса до реалните filter-submit-и (напр. чрез submittedRef).

CSS / Layout

• overflow-x: clip и minmax(0, 1fr) са коректни решения на overflow проблема; box-sizing: border-box върху .filter-apply е обосновано. position: fixed пилюлата зависи от липса на transform/filter/contain при предци — коментарът го твърди; струва си да се провери при бъдещи layout промени.

Заключение

Няма блокиращи проблеми. Кодът е чист, тестван и сигурен. Двете констатации са дребни (текст за екранни четци и свръх-анонсиране), затова оставям COMMENT вместо APPROVE, за да бъдат адресирани преди merge.

[ydimitrof]

Съобщението към екранния четец е подвеждащо: „непроменени“ означава unchanged, но намерението (видно и от коментара „pending, not-yet-applied changes“) е точно обратното — че има неприложени/чакащи промени. Предложение: „Има неприложени промени по филтрите. Натиснете „Търси“, за да ги приложите.“

[ydimitrof]

busy се извежда от глобалното navigation.state, така че aria-busy върху сайдбара и политният анонс „Зареждане на резултатите…“ се задействат при ВСЯКА навигация — пагинация, смяна на подредба и дори клик върху резултат, който води към детайлна страница — а не само при подаване на филтрите. Това води до подвеждащо свръх-анонсиране за екранни четци. Обмислете да ограничите анонса/aria-busy до навигациите, инициирани от „Търси“ (напр. чрез вече наличния submittedRef).