refactor(reviews): interaction state polish — 5 UX fixes

This commit is contained in:
Александр Зимин
2026-05-14 14:13:57 +00:00
parent 513b57b6b5
commit fb64ca4350
6 changed files with 188 additions and 49 deletions
+7 -33
View File
@@ -17,40 +17,14 @@ record rows reuse ReviewDrawer with status-gated footer.
--- ---
## 2. Interaction state pass — reviews page polish ## 2. Interaction state pass — reviews page polish ⏳ MR !189 pending
**What.** One MR covering five small fixes: Closed in [MR !189](https://git.nstart.cloud/2-6/2-6-4/terravault/ordinis/-/merge_requests/189) — all 5 fixes in one MR:
1. Records empty state — icon + description + audit link
- Records empty state: add description + primary action ("Queue is clear, see 2. Reject reason inline validation — aria-invalid + role=alert
/audit for past decisions"). Current `<EmptyState title={t('reviews.empty')} />` 3. SchemaDraftDrawer empty state — QueryErrorState для errors + dashed block для no-draft
is just a heading. 4. Approve/reject success flash — 600ms green/pink band before close
- Reject reason inline validation: today `handleReject` does 5. Bulk failed reasons humanized — extracted `humanizeBulkReason()` lib + 7 unit tests
`if (!comment.trim()) return` silently. Button click → nothing happens. Show
inline error or set the TextArea into error state.
- Drawer empty state (`workflow.schemaDraft.empty`): distinguish "not loaded",
"not found" (deleted), "no permission". Today the same blank message covers
all three.
- Approve action: drawer closes immediately on success. Add a 500ms success
state (or toast) so the reviewer gets visual confirmation. Currently feels
like the click might not have registered.
- Post-bulk-result polish: the `bulkResult` Alert lists business keys but the
failed list shows raw `reason` codes (`self_approve_forbidden`,
`draft_not_pending`). Map through `extractReviewError` already in the file.
**Why.** Pass 2 rated state coverage 5/10. These are the visible gaps. Each
is 10-30 LOC; the value is interaction confidence.
**Pros.** All in one place — `reviews.tsx`, `SchemaDraftDrawer.tsx`. Low risk.
**Cons.** Five different surfaces in one MR means review surface is wider than
a single-purpose change. Still worth grouping because they're all "state
polish" semantically.
**Context.** Pass 2 finding. None of these block functionality; they're trust
builders. Reject silent fail is the most user-visible — a reviewer clicks the
button and nothing happens, then notices the TextArea is empty after a beat.
**Depends on / blocked by.** Nothing.
--- ---
@@ -206,13 +206,24 @@ export function SchemaDraftDrawer({
> >
<div className="space-y-4"> <div className="space-y-4">
{isLoading && <LoadingBlock size="md" label={t('loading')} />} {isLoading && <LoadingBlock size="md" label={t('loading')} />}
{/* QueryErrorState уже сам обрабатывает 403/404 с human messages —
больше не надо guessить здесь. Раньше один blank empty покрывал
"not loaded / not found / forbidden" одинаково. */}
{error && <QueryErrorState error={error} onRetry={() => refetch()} />} {error && <QueryErrorState error={error} onRetry={() => refetch()} />}
{!isLoading && !draft && ( {!isLoading && !error && !draft && (
<p className="text-body text-mute"> <div className="flex flex-col items-center justify-center text-center py-10 px-4 gap-2 rounded-md border border-dashed border-line bg-surface-2/30">
<p className="text-body text-ink-2">
{t('workflow.schemaDraft.empty', { {t('workflow.schemaDraft.empty', {
defaultValue: 'Активного черновика схемы нет.', defaultValue: 'Активного черновика на этом словаре нет.',
})} })}
</p> </p>
<p className="text-cell text-mute max-w-md">
{t('workflow.schemaDraft.emptyHint', {
defaultValue:
'Создайте новый черновик через editor словаря — кнопка «Создать черновик схемы» появится если ваши изменения требуют ревью.',
})}
</p>
</div>
)} )}
{draft && ( {draft && (
+16
View File
@@ -581,6 +581,14 @@ i18n
'reviews.tab.records': 'Записи', 'reviews.tab.records': 'Записи',
'reviews.tab.schemas': 'Схемы', 'reviews.tab.schemas': 'Схемы',
'reviews.tab.mine': 'Мои', 'reviews.tab.mine': 'Мои',
// Interaction state polish (TODO 2)
'reviews.emptyDescription':
'Очередь чистая. Прошлые решения можно посмотреть в журнале аудита.',
'reviews.emptyAuditLink': 'Открыть /audit',
'reviews.drawer.rejectReasonRequired':
'Укажите причину отклонения — comment обязателен для reject (compliance).',
'workflow.schemaDraft.emptyHint':
'Создайте новый черновик через editor словаря — кнопка «Создать черновик схемы» появится если ваши изменения требуют ревью.',
'reviews.schema.empty': 'Очередь пуста', 'reviews.schema.empty': 'Очередь пуста',
'reviews.schema.emptyDescription': 'Схемы на ревью появятся здесь.', 'reviews.schema.emptyDescription': 'Схемы на ревью появятся здесь.',
// === /reviews → "Мои" tab (maker self-tracking) === // === /reviews → "Мои" tab (maker self-tracking) ===
@@ -1315,6 +1323,14 @@ i18n
'reviews.tab.records': 'Records', 'reviews.tab.records': 'Records',
'reviews.tab.schemas': 'Schemas', 'reviews.tab.schemas': 'Schemas',
'reviews.tab.mine': 'Mine', 'reviews.tab.mine': 'Mine',
// Interaction state polish (TODO 2)
'reviews.emptyDescription':
'Queue is clear. Past decisions live in the audit log.',
'reviews.emptyAuditLink': 'Open /audit',
'reviews.drawer.rejectReasonRequired':
'Reject requires a comment (compliance) — fill the comment field first.',
'workflow.schemaDraft.emptyHint':
'Create a new draft via the dictionary editor — the "Create schema draft" button appears when your changes need review.',
'reviews.schema.empty': 'Queue is empty', 'reviews.schema.empty': 'Queue is empty',
'reviews.schema.emptyDescription': 'Schemas under review will appear here.', 'reviews.schema.emptyDescription': 'Schemas under review will appear here.',
// === /reviews → "Mine" tab (maker self-tracking) === // === /reviews → "Mine" tab (maker self-tracking) ===
@@ -0,0 +1,44 @@
import { describe, expect, it } from 'vitest'
import { humanizeBulkReason } from './bulkReason'
describe('humanizeBulkReason', () => {
it('maps self_approve_forbidden to russian title', () => {
expect(humanizeBulkReason('self_approve_forbidden')).toBe(
'Нельзя одобрить свой draft (maker-checker)',
)
})
it('maps draft_not_pending to russian title', () => {
expect(humanizeBulkReason('draft_not_pending')).toBe(
'Draft уже не pending — обнови очередь',
)
})
it('maps forbidden_role to russian title', () => {
expect(humanizeBulkReason('forbidden_role')).toBe(
'Недостаточно прав (нужна reviewer role)',
)
})
it('maps reject_reason_required to russian title', () => {
expect(humanizeBulkReason('reject_reason_required')).toBe(
'Reject требует comment (compliance)',
)
})
it('maps self_reject_forbidden to russian title', () => {
expect(humanizeBulkReason('self_reject_forbidden')).toBe(
'Нельзя отклонить свой draft',
)
})
it('returns unknown reason as-is (backend message fallback)', () => {
expect(humanizeBulkReason('Some raw backend message')).toBe(
'Some raw backend message',
)
})
it('returns empty string for empty input', () => {
expect(humanizeBulkReason('')).toBe('')
})
})
+27
View File
@@ -0,0 +1,27 @@
/**
* Bulk-action failure reason → human-readable label.
*
* <p>Lives in `src/lib` (а не в `routes/reviews.tsx`) потому что bulk runner
* captures axios errors как plain string `reason` (либо `response.data.code`,
* либо `error.message`) и хранит в `BulkResult.failed[].reason`. Раньше
* reviewer видел сырой code типа `self_approve_forbidden` в bulk result
* alert'е — теперь маппит на russain title из {@code extractReviewError}
* mapping (single source of truth).
*
* <p>Если reason нет в known codes — возвращает as-is (backend message
* вероятно уже human-readable, либо unknown server error который оставляем
* сырым чтобы reviewer мог скопировать в bug report).
*
* <p>Surfaced during TODO 2 (`/plan-design-review` interaction state pass
* 2026-05-14). Pure function — unit tested через `bulkReason.test.ts`.
*/
export function humanizeBulkReason(reason: string): string {
const known: Record<string, string> = {
self_approve_forbidden: 'Нельзя одобрить свой draft (maker-checker)',
self_reject_forbidden: 'Нельзя отклонить свой draft',
draft_not_pending: 'Draft уже не pending — обнови очередь',
forbidden_role: 'Недостаточно прав (нужна reviewer role)',
reject_reason_required: 'Reject требует comment (compliance)',
}
return known[reason] ?? reason
}
+73 -6
View File
@@ -38,6 +38,7 @@ import { useApproveDraft, useRejectDraft, useWithdrawDraft } from '@/api/mutatio
import type { DraftOperation, DraftResponse, DraftStatus } from '@/api/client' import type { DraftOperation, DraftResponse, DraftStatus } from '@/api/client'
import { UserCell } from '@/lib/useUserDisplay' import { UserCell } from '@/lib/useUserDisplay'
import { shallowDiffSummary, isEmptyDiffSummary } from '@/lib/diff' import { shallowDiffSummary, isEmptyDiffSummary } from '@/lib/diff'
import { humanizeBulkReason } from '@/lib/bulkReason'
import { import {
MyDraftsPanel, MyDraftsPanel,
classifyRecordStatus, classifyRecordStatus,
@@ -368,7 +369,7 @@ function ReviewsPage() {
<ul className="ml-4 mt-1 list-disc"> <ul className="ml-4 mt-1 list-disc">
{bulkResult.failed.map((f) => ( {bulkResult.failed.map((f) => (
<li key={f.id}> <li key={f.id}>
<span className="font-mono">{f.bk}</span>: {f.reason} <span className="font-mono">{f.bk}</span>: {humanizeBulkReason(f.reason)}
</li> </li>
))} ))}
</ul> </ul>
@@ -379,7 +380,19 @@ function ReviewsPage() {
)} )}
{activeTab === 'records' && queue.data && queue.data.totalElements === 0 && ( {activeTab === 'records' && queue.data && queue.data.totalElements === 0 && (
<EmptyState title={t('reviews.empty')} /> <EmptyState
icon={<CheckCircleIcon weight="duotone" size={32} />}
title={t('reviews.empty')}
description={t('reviews.emptyDescription', {
defaultValue:
'Очередь чистая. Прошлые решения можно посмотреть в журнале аудита.',
})}
action={
<Link to="/audit" className="text-accent hover:underline text-cell">
{t('reviews.emptyAuditLink', { defaultValue: 'Открыть /audit' })}
</Link>
}
/>
)} )}
{activeTab === 'records' && queue.data && queue.data.totalElements > 0 && ( {activeTab === 'records' && queue.data && queue.data.totalElements > 0 && (
@@ -609,6 +622,14 @@ function ReviewDrawer({ draftId, onClose }: DrawerProps) {
const rejectMut = useRejectDraft() const rejectMut = useRejectDraft()
const withdrawMut = useWithdrawDraft() const withdrawMut = useWithdrawDraft()
const [comment, setComment] = useState('') const [comment, setComment] = useState('')
/** Inline error для reject reason — show when reviewer кликнул Reject с
* пустым comment'ом. Реально cleared автоматически когда user печатает. */
const [rejectError, setRejectError] = useState<string | null>(null)
/** Brief success affordance после approve/reject mutation — drawer
* остаётся 600ms с зелёной/красной полосой, чтобы reviewer почувствовал
* что click registered. Без этого drawer закрывается мгновенно, click
* feedback теряется. */
const [decisionFlash, setDecisionFlash] = useState<'approve' | 'reject' | null>(null)
// Identity gates — определяют что показать в footer'е drawer'а. // Identity gates — определяют что показать в footer'е drawer'а.
// //
@@ -627,8 +648,14 @@ function ReviewDrawer({ draftId, onClose }: DrawerProps) {
{ id: draftId, comment: comment.trim() || undefined }, { id: draftId, comment: comment.trim() || undefined },
{ {
onSuccess: () => { onSuccess: () => {
setDecisionFlash('approve')
// Hold drawer 600ms так чтобы reviewer увидел зелёную полосу;
// потом reset state и close. Без флэша approve было instant.
setTimeout(() => {
setComment('') setComment('')
setDecisionFlash(null)
onClose() onClose()
}, 600)
}, },
}, },
) )
@@ -636,13 +663,29 @@ function ReviewDrawer({ draftId, onClose }: DrawerProps) {
const handleReject = () => { const handleReject = () => {
if (!draftId) return if (!draftId) return
if (!comment.trim()) return // backend will 400, but fail fast in UI if (!comment.trim()) {
// Раньше return был silent — reviewer кликал Reject и ничего не
// происходило. Backend всё равно вернёт 400 missing_reject_reason,
// но user-friendly inline error лучше.
setRejectError(
t('reviews.drawer.rejectReasonRequired', {
defaultValue:
'Укажите причину отклонения — comment обязателен для reject (compliance).',
}),
)
return
}
setRejectError(null)
rejectMut.mutate( rejectMut.mutate(
{ id: draftId, comment: comment.trim() }, { id: draftId, comment: comment.trim() },
{ {
onSuccess: () => { onSuccess: () => {
setDecisionFlash('reject')
setTimeout(() => {
setComment('') setComment('')
setDecisionFlash(null)
onClose() onClose()
}, 600)
}, },
}, },
) )
@@ -828,7 +871,16 @@ function ReviewDrawer({ draftId, onClose }: DrawerProps) {
)} )}
{isPending && !isMineAndPending && ( {isPending && !isMineAndPending && (
<div className="border-t border-line pt-3 space-y-2"> <div
className={
'border-t pt-3 space-y-2 transition-colors duration-300 -mx-4 px-4 ' +
(decisionFlash === 'approve'
? 'border-green bg-green-bg/30'
: decisionFlash === 'reject'
? 'border-pink bg-pink-bg/30'
: 'border-line')
}
>
<label <label
htmlFor="review-comment" htmlFor="review-comment"
className="text-cap text-ink-2" className="text-cap text-ink-2"
@@ -838,9 +890,24 @@ function ReviewDrawer({ draftId, onClose }: DrawerProps) {
<TextInput <TextInput
id="review-comment" id="review-comment"
value={comment} value={comment}
onChange={(e) => setComment(e.target.value)} onChange={(e) => {
setComment(e.target.value)
// Stale validation error gone как только user печатает.
if (rejectError && e.target.value.trim()) setRejectError(null)
}}
placeholder={t('reviews.drawer.commentPlaceholder')} placeholder={t('reviews.drawer.commentPlaceholder')}
aria-invalid={Boolean(rejectError)}
aria-describedby={rejectError ? 'review-comment-error' : undefined}
/> />
{rejectError && (
<p
id="review-comment-error"
role="alert"
className="text-cell text-pink"
>
{rejectError}
</p>
)}
<div className="flex items-center justify-end gap-2 pt-2"> <div className="flex items-center justify-end gap-2 pt-2">
<Button <Button
variant="secondary" variant="secondary"
@@ -853,7 +920,7 @@ function ReviewDrawer({ draftId, onClose }: DrawerProps) {
variant="danger" variant="danger"
leftIcon={<XCircleIcon weight="bold" size={14} />} leftIcon={<XCircleIcon weight="bold" size={14} />}
onClick={handleReject} onClick={handleReject}
disabled={!comment.trim() || rejectMut.isPending || approveMut.isPending} disabled={rejectMut.isPending || approveMut.isPending}
loading={rejectMut.isPending} loading={rejectMut.isPending}
> >
{t('reviews.action.reject')} {t('reviews.action.reject')}