fix(addresses): reveal rendered addresses - #1345
Conversation
Use AddressOutput for visible address slots while leaving string-only and hash paths unchanged.\n\nRefs APP-1088
thekidnamedkd
changed the title
thekidnamedkd
changed the title
milosh86
left a comment
milosh86
left a comment
There was a problem hiding this comment.
A couple of valid points, I confirmed them:
Bugs
<div> inside <p>— hydration error and dropped styling (3 sites)
AddressOutput renders a root <div class="flex min-w-0 items-center gap-2"> unless hasInteractiveAncestor is set (which switches it to a <span>). Three call sites drop it straight into a <p>:
apps/app/src/daos/alchemix/components/alchemixSubmitVote/alchemixSubmitVote.tsx:295
apps/app/src/daos/alchemix/components/alchemixSubmitVote/alchemixSubmitVote.tsx:341
React 19 flags it directly:
In HTML, `<div>` cannot be a descendant of `<p>`. This will cause a hydration error.
> <p className="text-neutral-500 text-sm leading-tight">
<_p address="0x12345678...">
> <div className="flex min-w-0 items-center gap-2">
Second effect, confirmed by feeding the SSR markup through a real HTML parser: the parser auto-closes the <p> and hoists the address out as a sibling, so the <p> ends up empty and its classes never reach the address. text-neutral-500 text-sm leading-tight (simulation flow) and truncate font-semibold text-base text-neutral-800 (alchemix) are silently lost.
The branch already gets this right in ~8 other <p> sites via hasInteractiveAncestor — these three were missed. But note the fix differs here: none of the three has an interactive ancestor, so adding that flag would wrongly strip the copy control and keyboard access. Swap the <p> for a <div> (keeping the classes on it) instead.
- Two copy buttons for the same address in one row
[apps/app/src/plugins/gaugeVoterPlugin/dialogs/gaugeVoterVoteDialog/gaugeVoterVoteDialogItem/gaugeVoterVoteDialogItem.tsx:103-112]
(https://github.com/aragon/app/blob/address-tooltip/apps/app/src/plugins/gaugeVoterPlugin/dialogs/gaugeVoterVoteDialog/gaugeVoterVoteDialogItem/gaugeVoterVoteDialogItem.tsx#L103-L112) — the name line and the address line are both bare AddressOutput, and copy defaults to true. One dialog row now carries two identical copy controls for gaugeAddress, plus two extra tab stops. The sibling component doing the same name/address pair, gaugeVoterGaugeListItemStructure.tsx, passes hasInteractiveAncestor and gets exactly one; governanceBodyInfo.tsx likewise.
- Copy button injected into the wizard header chrome
apps/app/src/modules/application/components/navigations/navigationWizard/navigationWizard.tsx:131 — the parent is <div className="flex items-center gap-x-2"> holding the DAO name then DaoAvatar. Since AddressOutput's root is itself a flex row ending in a copy button, the rendered order becomes name → copy button → avatar. Page titles in this same branch use copy={false} for exactly this reason (dashboardDefaultHeader.tsx:69, daoMemberDetailsPageClient.tsx:243).
Nits
classNametargets the label span, not the root. Verified: it lands on the innermost<span class="inline-block min-w-0 max-w-full …">. SoclassName="w-full …"innavigationDao.tsx:137(replacing<Clipboard className="w-full">) andclassName="w-fit"insimulationFlowVisualizationItem.tsx:103(replacing<Link className="w-fit">) are now no-ops — both sit inside amax-w-fitanchor. Cosmetic; the kit applies its owntruncateso ellipsis still works.gaugeVoterVoteDialogItem.test.tsx:90 closeButtons.at(-1)is positional and just drifted once for this reason;getByRole('button', { name: … })wouldn't.IPolicySettingItem.copyValue(daoPolicyDetailsClientUtils.ts:24) is dead now that the page stopped reading it.
# Conflicts: # apps/app/src/modules/dashboard/pages/daoDashboardPage/daoDashboardPageClient.tsx # pnpm-lock.yaml # pnpm-workspace.yaml
# Conflicts: # apps/app/src/modules/governance/pages/daoProposalDetailsPage/daoProposalDetailsPageClient.tsx # pnpm-lock.yaml # pnpm-workspace.yaml
|
🚀 Preview Deployment: View Here |
|
E2E results (preview) Smoke
|
2 participants
Description
Replace direct visible address truncation renders with
AddressOutputso users can reveal and copy complete addresses across action details, permissions, finance, governance, navigation, plugin, and voting surfaces.Related issue: APP-1088
Dependency status
This refit uses
@aragon/gov-ui-kit2.11.3, published from aragon/gov-ui-kit#768 at merge commit652c9a108f1fa4b3cc10032549b6a6dfa413b131. Its release notes include theDefinitionList.Itemon-chain value mode, root-exportedInteractiveAncestorContext, inline-safeAddressOutput/Clipboardroots, root-levelAddressOutput.className, and DataList interactive-row layout preservation. The published package types additionally confirmAvatarIconnow uses anHTMLSpanElementroot, and the bundle includes 32-byte hash truncation. The temporary/tmptarball override has been removed; the catalog and lockfile now resolve normally to 2.11.3.Refitted behavior
DefinitionList.Item, puthref,isExternal, andisOnchainEntity: trueonlink, and usecopyValuewhenever the visible label is an ENS/name/token symbol. Raw addresses and 32-byte hashes are passed through for the kit's built-in checksum/hash truncation. There are noAddressOutputdescendants underDefinitionList.Iteminapps/app.NavigationDaoHome, selectableAssetInput, permissions accordion headers, and governance-body accordion headers) provideInteractiveAncestorContextonce at the wrapper. DataList overlay rows do not receive providers, so copy/reveal controls remain usable. There are no per-instancehasInteractiveAncestorprops and therefore no survivors to justify.<p>elements because fix(APP-4058): Fix canSimulate logic to allow simulating in executable state and update simulation dialog UI #768 makes the kit roots inline-safe spans. NohasInteractiveAncestorworkaround is added. One boundary is worth recording for future work: the span root makes the link-lessAddressOutputvalid phrasing content, but passinghrefwraps the label in the kitLink, whose internal<div class="flex items-center gap-x-1 …">still triggersIn HTML, <div> cannot be a descendant of <p>. None of the three reported sites passeshref, and an AST audit confirmsapps/apphas zeroAddressOutputsites that combinehrefwith a<p>/heading parent, so no surface is affected — buthref+<p>must stay off-limits.Page.Headerand its callers retain a string-onlytitle; the presentational page header renders an address title withcopy={false}. DAO navigation and wizard chrome are also copy-free.PermissionEntityCellandPermissionGraphNodeintentionally render oneAddressOutputwithaddresspluslabel, preserving reveal/copy affordances for named entities.PermissionNodeDetailPanelkeeps a plain named heading because its dedicated address row owns reveal/copy.IPolicySettingItemremains a plain data contract and keepscopyValueas an input. The page renderer maps its string/plain{ address, value, link, copyValue }data intoDefinitionList.Itemprops.Remove gaugebutton, and its test selects by role/name rather than position.truncateclasses were removed.navigationDao'sw-fulland simulation-flow'sw-fitremain because fix(APP-4058): Fix canSimulate logic to allow simulating in executable state and update simulation dialog UI #768 appliesclassNameto theAddressOutputroot.Milosh review disposition
<p>hydration reports are resolved by fix(APP-4058): Fix canSimulate logic to allow simulating in executable state and update simulation dialog UI #768's inline-safe span roots; app markup is restored to<p>without suppressing address controls.classNameconcern is resolved by fix(APP-4058): Fix canSimulate logic to allow simulating in executable state and update simulation dialog UI #768 applying it to the root; only meaningful root sizing classes remain.IPolicySettingItem.copyValueis intentionally retained and consumed by the refitted page renderer, per the new DefinitionList contract.GitHub exposes Milosh's feedback as one top-level review (
pullrequestreview-5049228444) and reports zero inline review threads, so there are no resolvable thread objects. A PR comment answers each review point against that original review.Verification
pnpm install --no-frozen-lockfile— passed with the published 2.11.3 package; no local file override remains.pnpm --filter @aragon/app type-check— passed against published 2.11.3.pnpm --filter @aragon/app lint:check— passed; 2,750 files checked, no fixes.pnpm exec jestsuite inapps/app— 390 suites / 2,534 tests passed, with zerocannot be a descendant/ hydration output across the run.simulationFlowVisualizationItem.test.tsx— the one previously-untested affected surface now has coverage asserting the fallback address renders as valid phrasing content (nop div, no descendant error) and that the linked variant exposes exactly oneCopycontrol. Both assertions were confirmed to be discriminating: thep divselector detects a real<div>-in-<p>, and anhref-inside-<p>render does emit React's descendant error.alchemixSubmitVote.test.tsx— 1 suite / 20 tests passed with the<p>address markup and no React DOM nesting/hydration warning output.DefinitionList.Itemwithlink={{ href, isExternal, isOnchainEntity: true }}and a raw address child renders exactly 1 anchor, exactly 1Copybutton, and the value truncated to0x1234…7890; thecopyValue+ label variant renders the label with a single copy control. This exercises the contract behind all 37 on-chain items rather than inferring it.AddressOutputdescendants underDefinitionList.Item.hasInteractiveAncestorprops; noReactNodepage-title widening;git diff --checkpassed.pnpm devserver — Next.js 16.3.0 started successfully athttp://localhost:3000; the public landing page rendered in Chrome DevTools.https://dev.backend.aragonservices.in/v3/daos/ethereum-mainnet/ens/boundless.dao.eth), so dashboard, member details, proposal details, settings/policy, gauge dialogs, simulation-flow, and the Alchemix vote surface could not render. Browser/server evidence was captured; no claim is made for those blocked surfaces. The browser also showed pre-existing CSP/WalletConnect local-development diagnostics unrelated to these changes.Type of Change
Developer Checklist
Review Checklist