Skip to content

fix(web): improve SmartSearch status accessibility - #301

Open
drnecrotix wants to merge 2 commits into
midt-bg:mainfrom
drnecrotix:fix/accessibility-small-improvements
Open

fix(web): improve SmartSearch status accessibility#301
drnecrotix wants to merge 2 commits into
midt-bg:mainfrom
drnecrotix:fix/accessibility-small-improvements

Conversation

@drnecrotix

Copy link
Copy Markdown

Какво и защо

Този PR подобрява достъпността на състоянията „Търсене…“ и „Няма съвпадения“ в компонента SmartSearch.

Досега тези съобщения се визуализираха като <li> елементи с role="presentation", което може да попречи на screen reader-и и други помощни технологии да бъдат уведомени коректно при промяна на състоянието.

Промяната заменя тези елементи с отделен status region, използващ role="status" и aria-live="polite".

Съществуващото визуално поведение и логиката на търсенето се запазват.

Свързан issue

Няма свързан issue — промяната е малко и самостоятелно подобрение на достъпността.

Вид промяна

  • fix — поправка на бъг
  • feat — нова функционалност
  • docs — документация
  • refactor / perf / style — без промяна в поведението
  • test / ci / build / chore — поддръжка

Как е тествано

Ръчно е проверена промяната в SmartSearch и генерираният markup.

Проверено е, че:

  • съобщението „Търсене…“ се предоставя чрез live status region;
  • съобщението „Няма съвпадения“ използва същия status region;
  • съществуващото поведение на търсенето не се променя.

Чеклист

  • Комитите следват conventional commits и нямат Co-Authored-By: trailer, който сочи към агент (Claude Code, Codex, Cursor, Copilot). Трейлъри с хора са наред и не се махат — те са начинът заслугата на сътрудника да оцелее при squash
  • PR-ът е с един логически обхват и е от форк към midt-bg/sigma:main
  • pnpm typecheck минава
  • pnpm test (поне за засегнатите пакети) минава
  • pnpm lint е чисто
  • Няма комитнати тайни, .env* или .dev.vars
  • Документацията в docs/ е обновена, ако промяната го налага

Discord

dr.necrotix

@drnecrotix

Copy link
Copy Markdown
Author

Благодаря за прегледа!
Промяната е ограничена само до accessibility поведението на SmartSearch и не засяга логиката на търсенето или визуалния дизайн.

@ydimitrof ydimitrof left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Преглед на PR: fix(web): improve SmartSearch status accessibility

Обобщение

Промяната премества съобщенията за състояние („Търсене…“ и „Няма съвпадения…“) извън списъка <ul> в отделен <div> с role="status" и aria-live="polite". Това е коректно и смислено подобрение на достъпността.

Оценка по измерения

Достъпност / Код качество — отлично

  • Преди съобщенията бяха <li role="presentation"> вътре в <ul> — семантично неправилно, тъй като статус текст не е елемент от списъка. Новият подход с role="status" е правилният начин за обявяване на живо на промени в състоянието към екранните четци.
  • Обединяването на двете състояния (зареждане/без резултати) в един жив регион е добре — гарантира, че преходите между тях се обявяват консистентно.
  • Намаляване на дублирането (един елемент вместо два условни <li>) — съответства на принципа за без дублиране на код.

Сигурност — чисто (Фаза 0)

  • Няма твърдо кодирани тайни, URL промени, зависимости или зловредни шаблони. Текстът се рендира като JSX съдържание (без dangerouslySetInnerHTML), така че няма XSS риск.

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

  • Тривиална промяна в рендиране; без влияние върху производителността.

Дребни забележки (незадължителни)

  • role="status" вече предполага имплицитно aria-live="polite"aria-atomic="true"), така че изричното aria-live="polite" е донякъде излишно — безвредно е и подобрява четимостта, затова е приемливо.
  • Проверете дали CSS правилата за .smart-search-empty не са били специфични за <li> (напр. подравняване/padding в контекста на списъка), тъй като елементът вече е <div> извън <ul>.

Тестове / Документация

  • Не са добавени тестове, но за чисто представяща промяна в достъпността това е приемливо; препоръчително е (незадължително) добавяне на тест, който проверява наличието на role="status" при празни резултати.

Заключение: Промяната е атомарна, фокусирана и подобрява достъпността без рискове. Одобрявам.

</ul>

{!hasResults && (
<div className="smart-search-empty" role="status" aria-live="polite">

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Добро подобрение — role="status" е правилният избор за обявяване на състояние към екранни четци вместо <li role="presentation">. Дребна забележка: role="status" вече включва имплицитно aria-live="polite"aria-atomic="true"), така че изричното aria-live="polite" е технически излишно. Може да се остави за яснота — безвредно е.


{!hasResults && (
<div className="smart-search-empty" role="status" aria-live="polite">
{loading ? 'Търсене…' : 'Няма съвпадения. Пробвай с име, ЕИК или УНП.'}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Проверете дали CSS класът smart-search-empty не е разчитал на това елементът да е <li> вътре в <ul> (напр. padding/подравняване, наследено от списъка). Сега елементът е <div> извън <ul>, така че визуалното оформление може леко да се различава.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants