Skip to content

fix: create proposal fixes#1904

Merged
pikonha merged 4 commits into
devfrom
fix/create-proposal
May 18, 2026
Merged

fix: create proposal fixes#1904
pikonha merged 4 commits into
devfrom
fix/create-proposal

Conversation

@brunod-e
Copy link
Copy Markdown
Collaborator

@brunod-e brunod-e self-assigned this May 13, 2026
@brunod-e brunod-e requested a review from isadorable-png as a code owner May 13, 2026 22:57
@railway-app
Copy link
Copy Markdown

railway-app Bot commented May 13, 2026

This PR was not deployed automatically as @brunod-e does not have access to the Railway project.

In order to get automatic PR deploys, please add @brunod-e to your workspace on Railway.

@vercel
Copy link
Copy Markdown

vercel Bot commented May 13, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
anticapture Ready Ready Preview, Comment May 18, 2026 2:39pm
anticapture-storybook Ready Ready Preview, Comment May 18, 2026 2:39pm

Request Review

@isadorable-png
Copy link
Copy Markdown
Collaborator

🎨 UI Review

Automated review · Spec (ClickUp DEV-659) · Preview (auth-gated 401, see caveats)

ℹ️ Path A — Reviewed without a Figma reference (no Figma URL in PR body, DEV-659, or linked design task 86ahar8xb). Grounded in 3 ClickUp task attachments + UX-expert evaluation.
⚠️ Vercel preview returned 401 (SAML — no bypass token configured). Regression check and responsive verification skipped.


Add Transfer Modal

Spec 1.2 — Suggested tokens row missing [ClickUp-image-confirmed]

The PR ships the spec 1.1 label change ("ERC-20""ERC-20 Token") but does not add the suggested-tokens badge row described in spec 1.2. The ClickUp mockup shows USDT • USDC • DAI • WETH • ENS chips below the Token Contract Address field; selecting one should fill the input. The AddTransferModal.tsx diff is a single line — no badge row added.

→ Question for author: is this scoped to a follow-up PR? If yes, link the issue so this doesn't get re-flagged. If not, it's a missing acceptance criterion (AddTransferModal.tsx:264-271).


Add Custom Action Modal — ABI fetch step

ENS name replaced with hex address after lookup [UX-heuristic]

When the user enters walt.ens.eth, setContractAddress(resolved) (AddCustomActionModal.tsx:344-346) overwrites the input with the 0x address — the typed ENS disappears from the visible input. Pro: clarifies which address was looked up. Con: loses readable ENS context (Etherscan-style UIs typically keep both visible).

→ Question for author: intentional? If yes, consider an inline helper like Resolved: 0x… (walt.ens.eth) so the ENS isn't lost. If unintentional, drop the auto-replace and pass resolved to lookupAbi directly.


Help Popover

Substring path match is too loose [Code-only]

pathname?.includes("/proposals/new") (HelpPopover.tsx:89) matches any path containing that substring — a future /proposals/newest or /proposals/new-thing would also bump the help button up. Use pathname === "/proposals/new" for exact match, or pathname?.startsWith("/proposals/new/") if sub-routes exist.

Position offset unverifiable without preview [UX-heuristic — preview-401]

The fix uses bottom-16 (64px) on the create-proposal route. Without an accessible preview I can't confirm the Publish CTA is fully reachable at that offset across viewports, especially on small screens where a floating button + bottom CTA bar can re-overlap. Worth a visual check before merge.


Spec acceptance — delivered correctly (no change needed)

  • 1.1"ERC-20 Token" label (AddTransferModal.tsx:267) matches the ClickUp mockup
  • 2.1 — ABI fetch resolves ENS first, then uses the resolved address (AddCustomActionModal.tsx:336-347)
  • 2.2 — function picker filters view/pure; status label says "N write function" (AddCustomActionModal.tsx:291-294, 573)
  • 3.1onWheel={e => e.stopPropagation()} on <Select> lets the wheel scroll the listbox without scrolling the page (Select.tsx:109)
  • 3.2 — Help popover shifts to bottom-16 on /proposals/new (modulo the substring nit above)
  • 4.1 — uint inputs gated to decimal or 0x hex at keystroke; uint8/16/etc. validated against max
  • 4.2 — bool args render as <Select> with true/false items
  • 4.3 — arrays show ["item1", "item2"] placeholder + JSON.parse validation
  • 4.4 — address validates as hex or ENS; bytesN enforces hex + exact byte length
  • ✅ Error display timing is good: touchedArgs Set defers errors until blur — no yelling while typing
  • ✅ Selecting a new function resets both args and touchedArgs so stale state doesn't carry over

chore: run simplify on the input validation
@pikonha pikonha merged commit 0d3791e into dev May 18, 2026
11 checks passed
@pikonha pikonha deleted the fix/create-proposal branch May 18, 2026 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants