fix: address PR #2139 review feedback and the panel E2E failure - #2140
Conversation
…ction settles Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…short viewport Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
🚅 Deployed to the anticapture-pr-2140 environment in anticapture-infra
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c7d735dfaa
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
🔍 Vercel preview: https://anticapture-9uy2bvx0h-ful.vercel.app |
isadorable-png
left a comment
isadorable-png
left a comment
There was a problem hiding this comment.
🎨 UI Review
Automated review · Figma: 🛰️ Product Design (reachable for the Panel screens) · Spec · Preview
ℹ️ This is a fix-up PR closing out my UI review on #2139, so it's checked against that review's asks plus the current Figma frame (3633:96644, resolved via linked spec task86ajyc1qv— the file-key's page listing itself only exposes a Cover page to this MCP integration, but the concrete node ID still resolves). TheCast Your Vote(Tornado) modal has no Figma frame anywhere in this file or its linked tasks, so that screen is reviewed via code + UX judgment. The dashboard's own Vercel preview isn't deployed for this branch (only Storybook is), and the Storybook preview is blocked from this environment's egress proxy — so this is a diff-level review; nothing here needed a live render to confirm.
Panel — Governance risk chart
Nice-to-have
- No code changes needed. Noting for the record: the
h3→h2bump is a DOM/semantics-only change — the Figma text style for this label (Alternative/Small: Roboto Mono Medium 13px, uppercase,tracking-[6px]) matchestext-alternative-sm tracking-alternative-sm font-mono font-medium uppercasepixel-for-pixel, so there's no visual delta, only a fixed heading outline (apps/dashboard/features/panel/components/DaoProtectionLevels.tsx:76). [Figma-confirmed]
Panel — Testimonial carousel
Nice-to-have
- The prev/next
IconButtons now render a 44px hit box (size-11) against the FigmaButtoninstance's literal 36px spec (isSquare, default size). This is the right call for the WCAG touch-target minimum, and it doesn't regress anything visible: the icon itself staysiconClassName="size-4"andvariant="ghost"is transparent at rest, so the resting appearance is unchanged — the only visible delta is a slightly largerhover:bg-surface-contrastsquare on hover. Flagging the spec/code divergence so design can bump the component's touch-target size to 44px officially, rather than leaving code as the one source of truth for it. (apps/dashboard/features/panel/components/TestimonialCarousel.tsx:38,:94) [Figma-confirmed]
Governance — Cast Your Vote modal (Tornado)
No Figma reference exists for this modal (checked the linked spec tasks and searched ClickUp workspace-wide), so this is evaluated on UX heuristics alone.
Nice-to-have
- The new delegator status message shares the footer row with Cancel/Submit (
flex items-center justify-end gap-2, no wrap). The sentence ("Could not load your delegators, so voting is disabled. Retry") wraps at word boundaries by default so nothing clips, but on the narrow mobile drawer it can push to 2–3 lines while the buttons stay vertically centered against a taller row, reading a little uneven. Addflex-wrapto the footer container, and considerw-fullon the message<p>so it drops to its own line above the buttons instead of squeezing the row. (apps/dashboard/features/governance/components/modals/VotingModal.tsx:391) [UX-heuristic]
Mobile
No separate mobile frame exists in Figma for this section (checked siblings of the 3633:96644 frame — only the one desktop frame + a "Feature Description" panel). Nothing mobile-specific stood out beyond the footer-wrap note above; the carousel's arrow hit-target fix is itself the mobile-relevant change here and reads correctly on both breakpoints since it's Tailwind-responsive by default (no lg:/md: split).
Validated against Figma — no change needed
DaoProtectionLevelsheading text, size, weight, tracking, and color all match the FigmaContainernode (3633:96657) exactly — the only change was the semantic tag.- The Tornado disabled-Submit messaging (loading vs. error+Retry) correctly mirrors the intent from the prior review:
handleSubmitstill fail-closes onisTornDelegatorListIncomplete || tornDelegatorsErroreven though the UI now explains why, so a stale/partial delegator list still can't slip through if a user manages to re-enable the button via a race. - No design-system violation on the inline
Retry<button>inVotingModal.tsx— the DS'sUnderlinedButton/UnderlinedLinkcomponents apply a heavier uppercase-mono-with-dashed-border treatment meant for standalone CTAs (e.g. "HOW THE FRAMEWORK WORKS →"), not a plain inline text link inside a sentence, so hand-rolling the small underlined action here is the correct call, not a gap.
Generated by Claude Code
2 participants
Fixes the Codex and UI review comments left on #2139, plus the failing Dashboard E2E check.
Codex (relayer, P2)
ProposalEnactmentServiceno longer drops the in-flight dedupe lock when the receipt wait times out: the lock is held (bounded to 4 extra receipt waits) until the broadcast transaction settles, so a repeated queue/execute request during that window joins the existing transaction instead of broadcasting a duplicate that reverts and wastes relayer gas. Covered by two new unit tests.UI review (isadorable)
DaoProtectionLevels: heading bumpedh3->h2so the panel outline no longer skips a level after the page'sh1.TestimonialCarousel: prev/next arrows enlarged from 36px to 44px to meet the minimum touch-target size.VotingModal(Tornado): the footer now explains a disabled Submit — "Loading your delegators..." while the delegator list pages in, and an error message with a Retry action when the fetch fails (previously the button just stayed dead with no feedback).Dashboard E2E
panel.spec.tsassumed the Monitored DAOs table is taller than the 640px short viewport; commenting the Shutter DAO out of the list dropped the table to 613px and failed all retries. The assertion now checks the real invariant directly — the table container never clips rows behind an inner scrollbar (scrollHeight <= clientHeight) — so the test no longer breaks when DAOs come and go.🤖 Generated with Claude Code