refactor(reviews): interaction state polish — 5 UX fixes
This commit is contained in:
Александр Зимин
@@ -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:
|
||||
|
||||
- Records empty state: add description + primary action ("Queue is clear, see
|
||||
/audit for past decisions"). Current `<EmptyState title={t('reviews.empty')} />`
|
||||
is just a heading.
|
||||
- Reject reason inline validation: today `handleReject` does
|
||||
`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.
|
||||
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
|
||||
2. Reject reason inline validation — aria-invalid + role=alert
|
||||
3. SchemaDraftDrawer empty state — QueryErrorState для errors + dashed block для no-draft
|
||||
4. Approve/reject success flash — 600ms green/pink band before close
|
||||
5. Bulk failed reasons humanized — extracted `humanizeBulkReason()` lib + 7 unit tests
|
||||
|
||||
---
|
||||
|
||||
|
||||
@@ -206,13 +206,24 @@ export function SchemaDraftDrawer({
|
||||
>
|
||||
<div className="space-y-4">
|
||||
{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()} />}
|
||||
{!isLoading && !draft && (
|
||||
<p className="text-body text-mute">
|
||||
{t('workflow.schemaDraft.empty', {
|
||||
defaultValue: 'Активного черновика схемы нет.',
|
||||
})}
|
||||
</p>
|
||||
{!isLoading && !error && !draft && (
|
||||
<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', {
|
||||
defaultValue: 'Активного черновика на этом словаре нет.',
|
||||
})}
|
||||
</p>
|
||||
<p className="text-cell text-mute max-w-md">
|
||||
{t('workflow.schemaDraft.emptyHint', {
|
||||
defaultValue:
|
||||
'Создайте новый черновик через editor словаря — кнопка «Создать черновик схемы» появится если ваши изменения требуют ревью.',
|
||||
})}
|
||||
</p>
|
||||
</div>
|
||||
)}
|
||||
|
||||
{draft && (
|
||||
|
||||
@@ -581,6 +581,14 @@ i18n
|
||||
'reviews.tab.records': 'Записи',
|
||||
'reviews.tab.schemas': 'Схемы',
|
||||
'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.emptyDescription': 'Схемы на ревью появятся здесь.',
|
||||
// === /reviews → "Мои" tab (maker self-tracking) ===
|
||||
@@ -1315,6 +1323,14 @@ i18n
|
||||
'reviews.tab.records': 'Records',
|
||||
'reviews.tab.schemas': 'Schemas',
|
||||
'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.emptyDescription': 'Schemas under review will appear here.',
|
||||
// === /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('')
|
||||
})
|
||||
})
|
||||
@@ -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
|
||||
}
|
||||
@@ -38,6 +38,7 @@ import { useApproveDraft, useRejectDraft, useWithdrawDraft } from '@/api/mutatio
|
||||
import type { DraftOperation, DraftResponse, DraftStatus } from '@/api/client'
|
||||
import { UserCell } from '@/lib/useUserDisplay'
|
||||
import { shallowDiffSummary, isEmptyDiffSummary } from '@/lib/diff'
|
||||
import { humanizeBulkReason } from '@/lib/bulkReason'
|
||||
import {
|
||||
MyDraftsPanel,
|
||||
classifyRecordStatus,
|
||||
@@ -368,7 +369,7 @@ function ReviewsPage() {
|
||||
<ul className="ml-4 mt-1 list-disc">
|
||||
{bulkResult.failed.map((f) => (
|
||||
<li key={f.id}>
|
||||
<span className="font-mono">{f.bk}</span>: {f.reason}
|
||||
<span className="font-mono">{f.bk}</span>: {humanizeBulkReason(f.reason)}
|
||||
</li>
|
||||
))}
|
||||
</ul>
|
||||
@@ -379,7 +380,19 @@ function ReviewsPage() {
|
||||
)}
|
||||
|
||||
{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 && (
|
||||
@@ -609,6 +622,14 @@ function ReviewDrawer({ draftId, onClose }: DrawerProps) {
|
||||
const rejectMut = useRejectDraft()
|
||||
const withdrawMut = useWithdrawDraft()
|
||||
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'а.
|
||||
//
|
||||
@@ -627,8 +648,14 @@ function ReviewDrawer({ draftId, onClose }: DrawerProps) {
|
||||
{ id: draftId, comment: comment.trim() || undefined },
|
||||
{
|
||||
onSuccess: () => {
|
||||
setComment('')
|
||||
onClose()
|
||||
setDecisionFlash('approve')
|
||||
// Hold drawer 600ms так чтобы reviewer увидел зелёную полосу;
|
||||
// потом reset state и close. Без флэша approve было instant.
|
||||
setTimeout(() => {
|
||||
setComment('')
|
||||
setDecisionFlash(null)
|
||||
onClose()
|
||||
}, 600)
|
||||
},
|
||||
},
|
||||
)
|
||||
@@ -636,13 +663,29 @@ function ReviewDrawer({ draftId, onClose }: DrawerProps) {
|
||||
|
||||
const handleReject = () => {
|
||||
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(
|
||||
{ id: draftId, comment: comment.trim() },
|
||||
{
|
||||
onSuccess: () => {
|
||||
setComment('')
|
||||
onClose()
|
||||
setDecisionFlash('reject')
|
||||
setTimeout(() => {
|
||||
setComment('')
|
||||
setDecisionFlash(null)
|
||||
onClose()
|
||||
}, 600)
|
||||
},
|
||||
},
|
||||
)
|
||||
@@ -828,7 +871,16 @@ function ReviewDrawer({ draftId, onClose }: DrawerProps) {
|
||||
)}
|
||||
|
||||
{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
|
||||
htmlFor="review-comment"
|
||||
className="text-cap text-ink-2"
|
||||
@@ -838,9 +890,24 @@ function ReviewDrawer({ draftId, onClose }: DrawerProps) {
|
||||
<TextInput
|
||||
id="review-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')}
|
||||
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">
|
||||
<Button
|
||||
variant="secondary"
|
||||
@@ -853,7 +920,7 @@ function ReviewDrawer({ draftId, onClose }: DrawerProps) {
|
||||
variant="danger"
|
||||
leftIcon={<XCircleIcon weight="bold" size={14} />}
|
||||
onClick={handleReject}
|
||||
disabled={!comment.trim() || rejectMut.isPending || approveMut.isPending}
|
||||
disabled={rejectMut.isPending || approveMut.isPending}
|
||||
loading={rejectMut.isPending}
|
||||
>
|
||||
{t('reviews.action.reject')}
|
||||
|
||||
Reference in New Issue
Block a user