Skip to content

test(desktop): story-cover toast error and edge states - #4167

Merged
Astro-Han merged 1 commit into
apache:mainfrom
liuxiaocs7:liuxiaocs7/toast-failure-states
Aug 30, 2026
Merged

Astro-Han merged 1 commit into
apache:mainfrom
liuxiaocs7:liuxiaocs7/toast-failure-states

Conversation

@liuxiaocs7

@liuxiaocs7 liuxiaocs7 commented Aug 29, 2026

Copy link
Copy Markdown
Member

test(desktop): story-cover toast error and edge states

Third surface under #3944 (one surface per PR): extend Primitives/Toast
with the failure and edge states that never show up in normal use, so the
Storybook gate and the #3893 a11y audit have real targets. Every story
drives the production ToastProvider API — no fabricated UI.

  • ErrorWithDiagnostics — an error toast carrying the provider's
    diagnostics action, plus the branch where opening diagnostics itself
    fails and the provider raises its own failure toast. A failed save, an
    offline sync, and an unauthorised request all land on this one error
    affordance, so it stands in for the trio rather than inventing three
    screens; play exercises the action-failure path a bare error variant
    never reaches.
  • DestructiveConfirm — an irreversible action's confirmation: the confirm
    button wears the destructive variant and initial focus rests on cancel,
    so the dangerous path is never the default.
  • LongContent — very long title and body; the card must wrap legibly
    instead of clipping text or shoving the layout off-screen.
  • ManyStacked — eight notifications at once, exercising the stack's
    density and whatever cap or scroll the layer applies.

Refs #3944, #3893

Generative tooling

Claude Code contributed substantially here — it authored these Storybook stories. The human contributor of record reviewed and submitted the change, and each commit carries a Generated-by: Claude Code trailer per CONTRIBUTING.md.

Visual evidence

Storybook stories captured with Playwright (hosted on the fork's release assets).

error-with-diagnostics — normal / light · narrow / dark

error-with-diagnostics normal / light error-with-diagnostics narrow / dark

destructive-confirm — normal / light · narrow / dark

destructive-confirm normal / light destructive-confirm narrow / dark

long-content — normal / light · narrow / dark

long-content normal / light long-content narrow / dark

many-stacked — normal / light · narrow / dark

many-stacked normal / light many-stacked narrow / dark

@Astro-Han Astro-Han 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.

Thanks for keeping all four states on the production ToastProvider and Astryx overlay path.

I reviewed exact head 4dd6404d504b6502fdd6972bd2380df3086c9203 and found no P0–P3 code issues. The error action, destructive confirmation, long content, and stacked notifications are distinct, useful states, and the exact-head checks are green.

The remaining blocker is visual evidence. The PR body contains no screenshots or recording of the new stories. Please attach:

  • the diagnostics failure sequence, including both the original error toast and the follow-up failure toast;
  • the destructive confirmation with initial focus visibly on Cancel;
  • long content at normal and narrow widths;
  • eight stacked notifications in light and dark themes.

Please also confirm that the destructive action uses the Astryx destructive treatment, long text does not clip or push actions off-screen, and the toast stack remains legible without creating a competing overlay language.

Two evidence boundaries are worth stating explicitly: the diagnostics story uses an intentionally rejected callback rather than the real Electron diagnostics bridge, and Storybook AX smoke does not by itself complete the broader Desktop accessibility journey in #3893. Neither is a product finding in this PR.

Once the visual evidence is available and manually confirmed, the code is suitable for approval.

Review analysis was assisted by Codex and an independent @reviewer agent. Astro-Han verified the exact head, production Toast composition, Astryx ownership, CI, and severity judgment, and owns this review.

中文对照

谢谢你让四个状态都继续走生产 ToastProvider 和 Astryx overlay,没有自创另一套 Toast。

我审查了精确 head 4dd6404d504b6502fdd6972bd2380df3086c9203,没有发现 P0–P3 代码问题。错误操作、破坏性确认、长文案和多条堆叠是边界清楚且有价值的状态,exact-head checks 也已通过。

当前缺的是视觉证据。PR 正文没有新 Story 的截图或录屏。请补充:

  • diagnostics 失败的完整过程,包括原错误 Toast 和后续失败 Toast;
  • 破坏性确认,清楚显示初始焦点位于取消按钮;
  • 正常宽度和窄宽度下的长文案;
  • 浅色、深色主题下的八条 Toast 堆叠。

同时请确认 destructive action 使用 Astryx 的破坏性样式,长文本不会裁切或把操作按钮挤出屏幕,Toast stack 也没有形成另一套 overlay 语言。

还有两个证据边界需要说明:diagnostics Story 使用的是刻意 reject 的 callback,不是真实 Electron diagnostics bridge;Storybook AX smoke 本身也不等于完成 #3893 的完整 Desktop accessibility journey。它们都不是本 PR 的产品问题。

视觉证据补齐并经人工确认后,代码层面可以 Approve。

本次审查分析由 Codex 和独立的 @reviewer 子代理协助;Astro-Han 核验了精确 head、生产 Toast composition、Astryx ownership、CI 和问题分级,并对本次 Review 负责。

@liuxiaocs7

Copy link
Copy Markdown
Member Author

Thanks for the review, and for confirming the four states are clean at the code level.

Confirming the points you raised:

  • Destructive treatment: the confirmation goes through the production ConfirmDialog, which sets actionVariant={destructive ? 'destructive' : 'primary'} on the Astryx AlertDialog, and initial focus rests on Cancel (asserted in the play). So it uses Astryx's own destructive treatment, not a bespoke one.
  • Boundaries: agreed and noted — the diagnostics story uses an intentionally-rejected callback (not the real Electron diagnostics bridge), and the Storybook AX smoke is not the full test(desktop): expand a11y coverage audit to chat, tool results, and error states #3893 Desktop accessibility journey. Neither is claimed as more than it is.

Long-text clipping and stack legibility are visual properties I can't assert in the render smoke, so they're for the screenshots.

On that: I'm in a headless/CI environment and can't attach screenshots to the PR directly. The four states render deterministically under stable ids (primitives-toast--error-with-diagnostics / --destructive-confirm / --long-content / --many-stacked). Happy to host PNGs somewhere if there's a preferred channel, or you can pull the branch and capture the four (including the diagnostics failure sequence and eight stacked toasts in light/dark). No code change in this revision.

@liuxiaocs7
liuxiaocs7 requested a review from Astro-Han August 29, 2026 20:18
@liuxiaocs7

Copy link
Copy Markdown
Member Author

Added a Visual evidence section to the PR description (screenshots on the fork's release assets): the diagnostics failure sequence (error toast + follow-up failure toast), the destructive confirm with focus on Cancel, long content, and the eight stacked notifications — normal/light and narrow/dark.

Third surface under apache#3944 (one surface per PR): extend `Primitives/Toast`
with the failure and edge states that never show up in normal use, so the
Storybook gate and the apache#3893 a11y audit have real targets. Every story
drives the production ToastProvider API — no fabricated UI.

- ErrorWithDiagnostics — an error toast carrying the provider's
  diagnostics action, plus the branch where opening diagnostics itself
  fails and the provider raises its own failure toast. A failed save, an
  offline sync, and an unauthorised request all land on this one error
  affordance, so it stands in for the trio rather than inventing three
  screens; `play` exercises the action-failure path a bare error variant
  never reaches.
- DestructiveConfirm — an irreversible action's confirmation: the confirm
  button wears the destructive variant and initial focus rests on cancel,
  so the dangerous path is never the default.
- LongContent — very long title and body; the card must wrap legibly
  instead of clipping text or shoving the layout off-screen.
- ManyStacked — eight notifications at once, exercising the stack's
  density and whatever cap or scroll the layer applies.

Refs apache#3944, apache#3893


Generated-by: Claude Code
@liuxiaocs7
liuxiaocs7 force-pushed the liuxiaocs7/toast-failure-states branch from 4dd6404 to 611e8a0 Compare August 29, 2026 21:37

@Astro-Han Astro-Han 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.

Thanks for keeping these edge states on the production ToastProvider and Astryx overlay path. Reviewed at exact head 611e8a0e277dc0b6b4a807cf9f74ee21b749532c: the code tree remains the reviewed one, exact-head CI is green, and the supplied standard visual evidence is sufficient. The Toast, destructive confirmation, long-content, and stacked-notification treatments were manually accepted; no additional theme/viewport screenshot matrix is required. I found no remaining P0-P3 issue.

Review analysis was assisted by Codex and independent reviewer agents. I verified the exact head, Astryx ownership, prior review boundary, CI, and manual visual decision, and I own this review.

中文对照

感谢你让这些边界状态继续走生产 ToastProvider 和 Astryx overlay。审查精确 head 611e8a0e277dc0b6b4a807cf9f74ee21b749532c 后,代码 tree 与此前审查一致,exact-head CI 已通过,现有标准视觉证据足够。Toast、破坏性确认、长文案和通知堆叠的视觉已经人工确认,不需要额外补完整主题/宽度截图矩阵。没有剩余 P0-P3 问题。

本次分析由 Codex 和独立 reviewer 子代理协助;我核验了精确 head、Astryx owner、旧审查边界、CI 和人工视觉结论,并对本次 Review 负责。

@Astro-Han
Astro-Han merged commit 1f95a37 into apache:main Aug 30, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/M Under 500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants