Conversation
pikonha
requested review from
LeonardoVieira1630,
PedroBinotto,
brunod-e and
isadorable-png
as code owners
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![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: 6356353ade
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const proposalRoute = `${WHITELABEL_ROUTES.proposals}/${proposalId}`; | ||
|
|
||
| return isOffchain | ||
| ? `${basePath}/${proposalRoute}?proposalType=offchain` | ||
| : `${basePath}/${proposalRoute}`; |
There was a problem hiding this comment.
getDaoProposalPath always constructs .../proposals/{id} (or ?proposalType=offchain). On standard dashboard paths, basePath resolves to /{dao} (e.g. /aave/governance), so this now emits /aave/proposals/..., but non-whitelabel detail pages still live under /[daoId]/governance/proposal/[proposalId] and /[daoId]/governance/offchain-proposal/[proposalId]. That makes proposal links from governance/feed/delegate views 404 for normal (non-whitelabel-hostname) users.
Useful? React with 👍 / 👎.
| return basePath || "/"; | ||
| } | ||
|
|
||
| return `${basePath}/${page}`; |
There was a problem hiding this comment.
getDaoPagePath appends the page directly to the DAO base path (/{dao}/{page}), so callers requesting page: "proposals" now navigate to /{dao}/proposals. In regular routes, proposal lists are still served from /{dao}/governance, so controls like the proposal-detail breadcrumb now send users to an unmapped URL and fail with 404 outside whitelabel rewrites.
Useful? React with 👍 / 👎.
| const NAV_ITEMS = [ | ||
| { | ||
| label: "Proposals", | ||
| page: WHITELABEL_ROUTES.proposals, | ||
| icon: Landmark, | ||
| enabled: (daoId: DaoIdEnum) => !!daoConfigByDaoId[daoId].governancePage, | ||
| }, | ||
| { | ||
| label: "Delegates", | ||
| page: WHITELABEL_ROUTES.delegates, | ||
| icon: UserCheck, | ||
| enabled: (daoId: DaoIdEnum) => !!daoConfigByDaoId[daoId].dataTables, | ||
| }, | ||
| { | ||
| label: "Activity Feed", | ||
| page: WHITELABEL_ROUTES.activityFeed, | ||
| icon: Newspaper, | ||
| enabled: (daoId: DaoIdEnum) => !!daoConfigByDaoId[daoId].activityFeed, | ||
| }, | ||
| { | ||
| label: "SPP Accountability", | ||
| page: WHITELABEL_ROUTES.sppAccountability, | ||
| icon: Building2, | ||
| enabled: (daoId: DaoIdEnum) => daoId === "ENS", | ||
| }, | ||
| ] as const; |
There was a problem hiding this comment.
Nav Items are duplicated here and in WhitelabelSidebar, extract it in a const
| page: item.page, | ||
| }), | ||
| })).filter((item) => item.enabled(daoId)), | ||
| [daoConfig, daoId, pathname], |
There was a problem hiding this comment.
| [daoConfig, daoId, pathname], | |
| [daoId, pathname], |
| const { votes } = useVoterInfo({ | ||
| address: address ?? "", | ||
| daoId, | ||
| proposalId: proposal?.id ?? "", | ||
| decimals, | ||
| }); |
There was a problem hiding this comment.
This is firing graphql requests per proposal in the list
| label: "SPP Accountability", | ||
| page: WHITELABEL_ROUTES.sppAccountability, | ||
| icon: Building2, | ||
| enabled: (daoId: DaoIdEnum) => daoId === "ENS", |
There was a problem hiding this comment.
we should use serviceProviders boolean from daoConfig
| const filteredOnchainProposals = useMemo( | ||
| () => | ||
| proposals.filter((proposal) => | ||
| proposal.title.toLowerCase().includes(normalizedSearch), | ||
| ), | ||
| [normalizedSearch, proposals], | ||
| ); | ||
| const filteredOffchainProposals = useMemo( | ||
| () => | ||
| offchainProposals.filter((proposal) => | ||
| proposal.title.toLowerCase().includes(normalizedSearch), | ||
| ), | ||
| [normalizedSearch, offchainProposals], | ||
| ); |
There was a problem hiding this comment.
this filter are only client side so I dont know if its too useful because if a proposal isnt fetched yet it will not appear on search
| "/", | ||
| `/${WHITELABEL_ROUTES.proposals}`, | ||
| `/${WHITELABEL_ROUTES.delegates}`, | ||
| "/activity-feed", |
There was a problem hiding this comment.
| "/activity-feed", | |
| "/${WHITELABEL_ROUTES.activityFeed}", |
| `/${WHITELABEL_ROUTES.sppAccountability}`, | ||
| `/${daoSlug}/${WHITELABEL_ROUTES.proposals}`, | ||
| `/${daoSlug}/${WHITELABEL_ROUTES.delegates}`, | ||
| `/${daoSlug}/activity-feed`, |
There was a problem hiding this comment.
| `/${daoSlug}/activity-feed`, | |
| `/${daoSlug}/${WHITELABEL_ROUTES.activityFeed}`, |
| return normalizedPathname === `/${daoSlug}` || | ||
| normalizedPathname.startsWith(`/${daoSlug}/`) | ||
| ? `/${daoSlug}` | ||
| : normalizedPathname === internalBasePath || | ||
| normalizedPathname.startsWith(`${internalBasePath}/`) | ||
| ? internalBasePath | ||
| : ""; | ||
| }; |
There was a problem hiding this comment.
| return normalizedPathname === `/${daoSlug}` || | |
| normalizedPathname.startsWith(`/${daoSlug}/`) | |
| ? `/${daoSlug}` | |
| : normalizedPathname === internalBasePath || | |
| normalizedPathname.startsWith(`${internalBasePath}/`) | |
| ? internalBasePath | |
| : ""; | |
| }; | |
| if (normalizedPathname === `/${daoSlug}` || normalizedPathname.startsWith(`/${daoSlug}/`)) | |
| return `/${daoSlug}`; | |
| if (normalizedPathname === internalBasePath || | |
| normalizedPathname.startsWith(`${internalBasePath}/`)) | |
| return internalBasePath; | |
| return ""; | |
| }; |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3edefc49bf
ℹ️ 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".
| const { votes } = useVoterInfo({ | ||
| address: address ?? "", | ||
| daoId, | ||
| proposalId: proposal?.id ?? "", | ||
| decimals, | ||
| }); |
There was a problem hiding this comment.
Calling useVoterInfo inside each ProposalItem introduces an N+1 query pattern for connected users: rendering a page of proposals now fires one GetAccountPower request per row (plus repeats on pagination/re-renders), which can significantly increase latency and backend load on governance/overview screens. This regression is introduced here because proposalId varies per card, so Apollo cannot collapse these into a single request.
Useful? React with 👍 / 👎.
| const { daoId } = await params; | ||
| const daoIdEnum = daoId.toUpperCase() as DaoIdEnum; | ||
|
|
||
| return <HoldersAndDelegatesSection daoId={daoIdEnum} />; |
There was a problem hiding this comment.
This page renders HoldersAndDelegatesSection without forcing the active tab, but that component defaults to tokenHolders; as a result /whitelabel/[daoId]/delegates shows token holders instead of delegates despite the route and metadata saying “Delegates”. Users returning from delegate detail to this route are also dropped into the wrong view.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7bccd429c2
ℹ️ 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".
| <ProposalListSection | ||
| loading={onchainLoading} | ||
| hasItems={proposals.length > 0} | ||
| hasItems={filteredOnchainProposals.length > 0} |
There was a problem hiding this comment.
Using filteredOnchainProposals.length > 0 (and the offchain equivalent) as hasItems makes the list enter the empty-state branch whenever the current page has no title matches, and that branch does not render the loadMoreRef sentinel that triggers fetchNextPage. In practice, a search term that only appears on later pages will incorrectly show “No proposals match your search” and never load additional pages, so search results can be silently incomplete.
Useful? React with 👍 / 👎.
| }: WhitelabelIndexPageProps) { | ||
| const { daoId } = await params; | ||
|
|
||
| redirect(`/whitelabel/${daoId}/proposals`); |
There was a problem hiding this comment.
This redirect always sends users to /whitelabel/{daoId}/proposals; combined with middleware rewriting / to /whitelabel/{daoId}, visiting a whitelabel hostname root (e.g. governance.ens.domains/) will expose and lock users onto internal implementation paths instead of the public /proposals URL scheme. That breaks the clean custom-domain routing behavior introduced by hostname rewrites.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7f00e1976
ℹ️ 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".
| return ALL_DAOS.includes(forcedDao as DaoIdEnum) | ||
| ? (forcedDao as DaoIdEnum) |
There was a problem hiding this comment.
resolveForcedDao currently accepts any value in ALL_DAOS, so setting FORCE_DAO to a non-whitelabel DAO (for example AAVE, which is shown in .env.example) makes middleware rewrite requests to /whitelabel/<dao>/.... Those routes immediately 404 because app/whitelabel/[daoId]/layout.tsx rejects DAOs without whitelabel config, so local forced-domain development breaks for most DAO values.
Useful? React with 👍 / 👎.
| `/${WHITELABEL_ROUTES.proposals}`, | ||
| `/${WHITELABEL_ROUTES.delegates}`, | ||
| `/${WHITELABEL_ROUTES.holdersAndDelegates}`, | ||
| `/${WHITELABEL_ROUTES.activityFeed}`, | ||
| `/${WHITELABEL_ROUTES.serviceProviders}`, |
There was a problem hiding this comment.
The rewrite candidate list excludes the governance path, so requests like /governance or /governance/proposal/<id> on a whitelabel hostname are not rewritten to /whitelabel/[daoId]/... and fall through to 404s, even though internal whitelabel governance pages exist. This breaks legacy/bookmarked governance URLs on custom domains.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: afc7959ab7
ℹ️ 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".
| appName: "Shutter Governance", | ||
| logo: ShutterIcon, | ||
| }, | ||
| hostnames: ["governance.ens.domains"], // change later |
There was a problem hiding this comment.
This sets Shutter’s hostnames to governance.ens.domains, which is the same hostname configured for ENS. Because resolveDaoIdFromHostname builds a single hostname→DAO map, the later SHU entry overwrites ENS and requests for governance.ens.domains resolve to SHU instead of ENS, breaking the primary whitelabel routing target introduced in this change.
Useful? React with 👍 / 👎.
| const isWhitelabelRoute = Boolean( | ||
| getWhitelabelBasePath({ | ||
| daoId: daoId.toUpperCase() as DaoIdEnum, | ||
| pathname, | ||
| }), |
There was a problem hiding this comment.
isWhitelabelRoute is derived from getWhitelabelBasePath, but that helper intentionally returns a base path for normal /{dao}/... routes too. As a result, hideConnectWallet becomes true on standard proposal detail pages and suppresses the proposal-header connect CTA for non-whitelabel users, which is a regression in the normal navigation flow.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 69f79a7804
ℹ️ 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".
| return `/${WHITELABEL_ROUTES.notifications}`; | ||
| } | ||
|
|
||
| return `/whitelabel/${daoId.toLowerCase()}/${WHITELABEL_ROUTES.notifications}`; |
There was a problem hiding this comment.
getDaoNotificationsPath falls back to /whitelabel/{dao}/notifications for non-whitelabel page contexts, and VoteSuccessContent uses this helper for the “Get Notifications” button after voting. On standard proposal routes (for example /aave/governance/proposal/...), this sends users to a whitelabel URL that is rejected by app/whitelabel/[daoId]/layout.tsx for DAOs without whitelabel config, so the new post-vote CTA becomes a 404 for most DAOs.
Useful? React with 👍 / 👎.
|
|
||
| export const HoldersAndDelegatesSection = ({ daoId }: { daoId: DaoIdEnum }) => { | ||
| const pathname = usePathname(); | ||
| const isWhitelabel = Boolean(getWhitelabelBasePath({ daoId, pathname })); |
There was a problem hiding this comment.
HoldersAndDelegatesSection treats any truthy getWhitelabelBasePath result as whitelabel, but that helper also returns /{dao} on normal routes like /[daoId]/holders-and-delegates. This makes isWhitelabel true on standard pages, so the default tab flips to delegates instead of tokenHolders, changing existing non-whitelabel behavior and initial data shown to users.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fcea846874
ℹ️ 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".
|
|
||
| export const GlobalProviders = ({ children }: { children: ReactNode }) => { | ||
| const pathname = usePathname(); | ||
| const isWhitelabelRoute = pathname?.startsWith("/whitelabel/"); |
There was a problem hiding this comment.
The whitelabel check relies on pathname.startsWith("/whitelabel/"), but middleware rewrites custom-domain requests like /proposals to /whitelabel/{daoId}/... internally while the browser URL stays /proposals (apps/dashboard/middleware.ts). In that common custom-domain flow this condition stays false, so the dark class is never removed and whitelabel pages render with dark-mode utility variants enabled, which breaks the intended branded theme.
Useful? React with 👍 / 👎.
| {(snapshotLink !== undefined ? true : isOngoing) && ( | ||
| <Button onClick={() => setIsVotingModalOpen(true)}> |
There was a problem hiding this comment.
For offchain proposals, the CTA condition uses snapshotLink !== undefined ? true : isOngoing, which always evaluates to true on Snapshot proposals regardless of status. That means closed offchain proposals on whitelabel detail pages still show “Cast your vote” and open the voting modal, leading users into a vote flow that cannot succeed once voting has ended.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fab8687b77
ℹ️ 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".
| theme: "light", | ||
| requestFeatureUrl: "https://blockful.io/contact", | ||
| customDomain: "governance.ens.domains", // change later | ||
| forumBaseUrl: "https://discuss.ens.domains/", // change later |
There was a problem hiding this comment.
SHU sets whitelabel.forumBaseUrl to the ENS forum, and this commit makes TitleSection render a "Forum discussion" link via getWhitelabelForumProposalUrl for DAOs that have whitelabel config. On SHU proposal pages, users are therefore sent to ENS search results instead of Shutter governance discussions, so the new CTA consistently navigates to the wrong community.
Useful? React with 👍 / 👎.
| export const ShutterAttackBanner = () => { | ||
| const pathname = usePathname(); | ||
|
|
||
| if (pathname.startsWith("/whitelabel/")) { |
There was a problem hiding this comment.
The middleware rewrites whitelabel hostnames to /whitelabel/{daoId}/... internally, but the browser pathname on custom domains stays /proposals, /delegates, etc. This guard only checks pathname.startsWith('/whitelabel/'), so the Shutter-specific global banner still renders on branded whitelabel domains where it should be hidden.
Useful? React with 👍 / 👎.
🎨 UI Review — PR #1801 Whitelabel Governance
Layout / Shell
Proposals
Proposal DetailCurrent results
Status / timeline
Links
Votes tab
Actions tab
Holders & Delegates
Service Providers
Notifications
Governance Settings
MobileMenu
Header
Proposal Detail
Holders & Delegates
Service Providers
|
3 participants
Summary
governance.ens.domains), theme, and branding config/whitelabel/[daoId]Next.js route with all governance sub-pages (overview, proposals, delegates, feed, service providers)offchainProposalsflag for ENS, COMP, GTC, and UNIuseConnectedWalletVotingPowerhook andWhitelabelDelegateDetailPagecomponentTest plan
governance.ens.domainslocally (or with middleware override) and verify the whitelabel layout renders with ENS branding🤖 Generated with Claude Code