Conversation
…rpcast, Email, GitHub
…ar>, <NameDisplay>, <AddressDisplay>
…& document further steps
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughRemoved many local UI primitives, icons, datetime/hooks, identity utilities, cards, footer, and synced-clock code; replaced usages with imports from Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant Feed as DisplayRegistrarActionsFeed
participant Referrals as ENSReferralsClient
participant Utils as getReferralQualificationInfo
participant Card as RegistrarActionCardMemo
Browser->>Feed: render DisplayRegistrarActionsFeed
Feed->>Referrals: fetch registrar actions
Referrals-->>Feed: actions list
Feed->>Utils: compute qualification info for action
Utils-->>Feed: qualification info
Feed->>Card: render RegistrarActionCardMemo(links, referralProgramField)
Card-->>Browser: rendered card with links & program info
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR migrates the ENSAwards application to use the @namehash/namehash-ui package (v1.5.1), replacing locally implemented UI components and utilities with standardized exports from the shared library. The changes also add the sonner toast notification library as a dependency.
Changes:
- Added
@namehash/namehash-ui(v1.5.1) andsonner(v2.0.7) as dependencies - Removed local implementations of common components (icons, datetime utilities, identity displays, chain utilities) in favor of namehash-ui exports
- Updated imports across the codebase to reference the new package
Reviewed changes
Copilot reviewed 61 out of 62 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json / pnpm-lock.yaml | Added namehash-ui and sonner dependencies |
| ensawards.org/src/styles/global.css | Imported namehash-ui styles, removed duplicate dark theme CSS |
| ensawards.org/src/utils/* | Removed local clock, chain, and namespace utilities replaced by package exports |
| ensawards.org/src/components/atoms/* | Removed local icon, datetime, identity components replaced by namehash-ui |
| ensawards.org/src/components/ui/sonner.tsx | Added Toaster component wrapper |
| ensawards.org/src/layouts/Layout.astro | Integrated Toaster component |
| Various component files | Updated imports to use namehash-ui exports |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ensawards.org/src/pages/mocks/alert-countdown.astro (1)
73-88: Remove extraneous props fromAbsoluteTime.The
ctaTextandlinkprops (lines 77-78) appear to be copy-paste artifacts fromAlertCountdownusage. A datetime formatting component likeAbsoluteTimewouldn't use these props. They will either be silently ignored or cause runtime warnings.Suggested fix
<AbsoluteTime client:load timestamp={(Date.now() + 1.75 * millisecondsInMinute) / millisecondsInSecond} - ctaText="Learn more" - link="/ens-referral-awards" options={{ year: "numeric", month: "short", day: "numeric", hour: "numeric", minute: "numeric", second: "numeric", hour12: false, }} />
🤖 Fix all issues with AI agents
In `@ensawards.org/src/components/atoms/badges/BenchmarkResultBadge.tsx`:
- Around line 36-42: In BenchmarkResultBadge.tsx fix the malformed base passed
to new URL when building identityLinkDetails: replace the string
"https:ensawards.org/" with a proper origin like "https://ensawards.org/" so new
URL(getEnsAdvocateDetailsRelativePath(identity.address),
"https://ensawards.org/") constructs correct absolute links; locate the new URL
invocation inside the identityLinkDetails object and update the base
accordingly.
In `@ensawards.org/src/components/atoms/cards/ContractCard.astro`:
- Around line 58-61: The href prop is being passed the full object returned by
getBlockExplorerAddressDetailsUrl instead of the URL string; update the usage in
ContractCard (where getBlockExplorerAddressDetailsUrl(...) is called) to safely
access the href property with optional chaining (use the returned object's .href
via ?.) so null/unsupported chains are handled and the anchor receives a proper
string or undefined.
In `@ensawards.org/src/components/atoms/cards/ReferrerCard.tsx`:
- Around line 66-72: The base URL string used when constructing
identityLinkDetails is malformed (missing "//"); in ReferrerCard.tsx update the
URL construction that wraps getEnsAdvocateDetailsRelativePath(referrer.referrer)
to use "https://ensawards.org/" instead of "https:ensawards.org/" in all
occurrences (the identityLinkDetails blocks around lines where
getEnsAdvocateDetailsRelativePath is used—e.g., the four instances that build
new URL(..., "https:ensawards.org/")). Ensure each identityLinkDetails.link uses
new URL(getEnsAdvocateDetailsRelativePath(...), "https://ensawards.org/").
In `@ensawards.org/src/components/atoms/ReferralProgramTimeline.tsx`:
- Around line 12-13: There are two separate imports from the same module
(`AbsoluteTime` and `useNow` from "@namehash/namehash-ui"); consolidate them
into a single import statement by replacing the two import lines with one that
imports both symbols (e.g., import { AbsoluteTime, useNow } from
"@namehash/namehash-ui") and remove the duplicate import.
In
`@ensawards.org/src/components/ens-advocates/details-page-components/advocate-profile/AdvocateProfileWithoutName.tsx`:
- Around line 43-45: The anchor href can become undefined because
getBlockExplorerAddressDetailsUrl and getEnsManagerAddressDetailsUrl may return
null; update AdvocateProfileWithoutName.tsx to handle nulls by checking the
result of getBlockExplorerAddressDetailsUrl(getENSRootChainId(namespaceId),
address) and getEnsManagerAddressDetailsUrl(getENSRootChainId(namespaceId),
namespaceId) before using .href — if either returns null, either don't render
the <a> (render plain text or a disabled link with aria-disabled and a
tooltip/message) or render a clear fallback (e.g., "Explorer unavailable") so
users aren’t presented with an anchor without a navigable href; ensure to
reference those functions and the component name when making the change.
In
`@ensawards.org/src/components/referral-awards-program/referrals/DisplayRegistrarActionsFeed.tsx`:
- Around line 61-64: The URL base string in DisplayRegistrarActionsFeed (where
link is constructed with new
URL(getEnsAdvocateDetailsRelativePath(namedRegistrarAction.action.registrant),
"https:ensawards.org/")) is malformed (missing "//"); update the base to a valid
origin such as "https://ensawards.org/" so new URL(...) constructs correctly;
locate the link construction in DisplayRegistrarActionsFeed.tsx (the new URL
call that uses getEnsAdvocateDetailsRelativePath and
namedRegistrarAction.action.registrant) and replace the base string accordingly.
- Line 45: Remove the unnecessary computation of qualifiedReferralPrograms in
DisplayRegistrarActionsFeed.tsx: delete the line that assigns const
qualifiedReferralPrograms = getReferralQualificationInfo(namedRegistrarAction)
and remove the getReferralQualificationInfo import if it’s no longer used, since
referralProgramField logic is not yet active; reintroduce the call only when the
referralProgramField prop and the commented usage are enabled.
In `@ensawards.org/src/components/ui/sonner.tsx`:
- Around line 5-22: The Toaster component spreads {...props} after setting
toastOptions on the Sonner component so a caller-supplied toastOptions will
completely override the default classNames; fix by destructuring props (e.g.,
const { toastOptions: userToastOptions, ...rest } = props) and pass merged
toastOptions to Sonner: merge the top-level toastOptions and deep-merge
classNames (use spreads to combine default classNames with
userToastOptions?.classNames) and then spread ...rest into Sonner so callers can
override other props without clobbering the preset classNames.
In `@ensawards.org/src/utils/chains.ts`:
- Around line 3-4: Update the TODO above the SUPPORTED_CHAINS export to
reference the exact blocking source in `@namehash/namehash-ui`: find the PR number
or the minimum released package version that adds SUPPORTED_CHAINS (e.g., PR
#<number> or vX.Y.Z) and replace the generic TODO with that identifier and a
short action (e.g., "remove when `@namehash/namehash-ui`@vX.Y.Z is released" or
"remove when PR #<number> is merged"); keep the note adjacent to the
SUPPORTED_CHAINS constant so future maintainers know the precise condition for
removal.
In `@ensawards.org/src/utils/hooks/useMobile.tsx`:
- Line 21: Replace the inline TODO in useMobile.tsx (the comment about replacing
after new `@next` version of namehash-ui adding export of useIsMobile) with a
tracked reference: either a link/ID to the issue or PR that will perform the
migration, or a short rationale plus the specific target version that will
remove the TODO; update the comment to include the issue/PR number (or repo URL)
and an expected milestone (e.g., "remove when namehash-ui vX.Y.Z is released")
so the note is actionable and not easily overlooked.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
ensawards.org/src/components/atoms/ReferralProgramTimeline.tsx (1)
33-36: Missing dependency inuseMemodependency array.The memoization callback uses both
referralProgramRulesandnow, but onlynowis listed in the dependency array. IfreferralProgramRuleschanges while the component remains mounted, the status will not recalculate, leading to stale UI.🐛 Proposed fix
const referralProgramStatus = useMemo( () => calcReferralProgramStatus(referralProgramRules, now), - [now], + [referralProgramRules, now], );ensawards.org/src/components/atoms/badges/BenchmarkResultBadge.tsx (1)
61-63: Remove unusedclientvariable.The
ENSNodeClientinstance is created but never referenced in this component. This is dead code left over from refactoring.♻️ Proposed fix
-const client = new ENSNodeClient({ - url: getENSNodeUrl(), -}); - const ensNodeReactConfig = createConfig({ url: getENSNodeUrl(), });ensawards.org/src/components/ens-advocates/details-page-components/advocate-profile/AdvocateProfileWithoutName.tsx (1)
41-70: Addrel="noopener noreferrer"to external links.Both anchors open new tabs without
rel, which enables reverse-tabnabbing.🔒 Proposed fix
{address !== null && ( <a href={ getBlockExplorerAddressDetailsUrl(getENSRootChainId(namespaceId), address)?.href } target="_blank" + rel="noopener noreferrer" className={cn( shadcnButtonVariants({ variant: "secondary", @@ <a href={getEnsManagerAddressDetailsUrl(address, namespaceId)?.href} target="_blank" + rel="noopener noreferrer" className={cn( shadcnButtonVariants({ variant: "default",ensawards.org/src/components/referral-awards-program/referrals/DisplayRegistrarActionsFeed.tsx (1)
40-69: Centralize the ENS Awards origin to avoid future typos.The base URL is duplicated; extracting it to a single constant reduces drift and makes updates safer.
♻️ Suggested refactor
export function DisplayRegistrarActionsList({ namespaceId, registrarActions, showReferrer = true, }: DisplayRegistrarActionsListProps) { const [animationParent] = useAutoAnimate(); const now = useNow(); + const ensAwardsOrigin = "https://ensawards.org/"; return ( <div ref={animationParent} className="w-full h-fit box-border flex flex-col justify-start items-center gap-3 relative" > {registrarActions.map((namedRegistrarAction) => { const qualifiedReferralPrograms = getReferralQualificationInfo(namedRegistrarAction); return ( <RegistrarActionCardMemo key={namedRegistrarAction.action.id} namespaceId={namespaceId} namedRegistrarAction={namedRegistrarAction} now={now} showReferrer={showReferrer} links={{ name: { isExternal: true, link: getEnsManagerNameDetailsUrl(namedRegistrarAction.name, namespaceId), }, registrant: { isExternal: false, link: new URL( getEnsAdvocateDetailsRelativePath(namedRegistrarAction.action.registrant), - "https://ensawards.org/", + ensAwardsOrigin, ), }, referrerLinkFunction: (address, namespaceId) => - new URL(getEnsAdvocateDetailsRelativePath(address), "https://ensawards.org/"), + new URL(getEnsAdvocateDetailsRelativePath(address), ensAwardsOrigin), }}
🤖 Fix all issues with AI agents
In `@ensawards.org/src/components/atoms/cards/ReferrerCard.tsx`:
- Around line 66-72: The same construction new
URL(getEnsAdvocateDetailsRelativePath(...), "https://ensawards.org/") is
repeated; create a single local constant (e.g., advocateUrl) once at the top of
ReferrerCard's render/functional body using
getEnsAdvocateDetailsRelativePath(referrer.referrer) and reuse that constant for
all identityLinkDetails.link usages (the occurrences around identityLinkDetails,
and the other blocks at the same component: the repeats at the other three
locations). Update references to use advocateUrl instead of recreating the URL
to avoid duplication and extra allocations.
In `@ensawards.org/src/data/contracts.test.ts`:
- Around line 18-19: The import millisecondsInSecond is unused; either remove
the import from contracts.test.ts or replace the hardcoded timeout value 3000
with millisecondsInSecond * 3 (or another appropriate multiplier) where the test
timeout is set (referencing the timeout literal 3000 in the test file), and
ensure any replaced usage updates any related comments or variable names
accordingly.
♻️ Duplicate comments (1)
ensawards.org/src/components/atoms/cards/ContractCard.astro (1)
58-61: Fix correctly applied —?.hrefaccess now in place.The optional chaining properly extracts the URL string from the returned object, consistent with usage elsewhere in the codebase. Based on learnings, the null case shouldn't occur in practice since ENSAwards only supports chains with block explorers, but this defensive pattern is still appropriate.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 67 out of 68 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@ensawards.org/src/components/atoms/cards/ReferrerCard.tsx`:
- Around line 54-57: Refactor the creation of advocateDetailsUrl in
ReferrerCard.tsx to avoid using import.meta.url during SSR: detect runtime
(e.g., typeof window !== 'undefined') and, when client-side, construct the URL
with window.location.origin (or location.href) +
getEnsAdvocateDetailsRelativePath(referrer.referrer); when server-side, use a
safe server base (env-provided SITE_ORIGIN or a relative path string) so
import.meta.url is never used on the server; update the symbol
advocateDetailsUrl and the call to
getEnsAdvocateDetailsRelativePath(referrer.referrer) accordingly and apply the
same pattern to BenchmarkResultBadge.tsx and DisplayRegistrarActionsFeed.tsx.
♻️ Duplicate comments (2)
ensawards.org/src/components/atoms/badges/BenchmarkResultBadge.tsx (1)
36-39: Avoidimport.meta.urlas the base for internal links.Using
import.meta.urlcan drop a configured base path and can producefile://links during SSR/pre-render. Preferdocument.baseURI(or a configured site origin) to keep internal links correct across environments.🛠️ Suggested fix
identityLinkDetails={{ isExternal: false, - link: new URL(getEnsAdvocateDetailsRelativePath(identity.address), import.meta.url), + link: new URL( + getEnsAdvocateDetailsRelativePath(identity.address), + typeof document !== "undefined" ? document.baseURI : "https://ensawards.org/", + ), }}ensawards.org/src/components/referral-awards-program/referrals/DisplayRegistrarActionsFeed.tsx (1)
33-71:import.meta.urlcan resolve to file/CDN origin; internal links may misroute.If this renders in SSR or assets are served from a different origin,
new URL(..., import.meta.url)can yieldfile://…or the asset host, sending users to the wrong origin. Prefer a runtime-origin/env base with an SSR-safe fallback and reuse it for both links.🐛 Proposed fix
const [animationParent] = useAutoAnimate(); const now = useNow(); + const baseOrigin = globalThis.location?.origin ?? "https://ensawards.org"; @@ registrant: { isExternal: false, link: new URL( getEnsAdvocateDetailsRelativePath(namedRegistrarAction.action.registrant), - import.meta.url, + baseOrigin, ), }, referrer: { isExternal: false, getLink: (address: Address, _namespaceId: ENSNamespaceId) => - new URL(getEnsAdvocateDetailsRelativePath(address), import.meta.url), + new URL(getEnsAdvocateDetailsRelativePath(address), baseOrigin), },
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 68 out of 69 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Greptile SummaryThis PR successfully migrates ENSAwards to use the Key Changes:
Implementation Quality:
Considerations:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant ReferrerLeaderboard
participant ENSReferralsClient
participant ENSNodeAPI
participant RegistrarActionsFeed
participant NamehashUI
User->>ReferrerLeaderboard: View leaderboard page
ReferrerLeaderboard->>ENSReferralsClient: new ENSReferralsClient({url})
ReferrerLeaderboard->>ENSReferralsClient: getReferrerLeaderboardPage({page, recordsPerPage})
ENSReferralsClient->>ENSNodeAPI: GET /referrer-leaderboard
ENSNodeAPI-->>ENSReferralsClient: ReferrerLeaderboardPage data
ENSReferralsClient-->>ReferrerLeaderboard: response with leaderboard data
ReferrerLeaderboard->>NamehashUI: ResolveAndDisplayIdentity (referrer info)
NamehashUI-->>ReferrerLeaderboard: Rendered referrer cards
User->>RegistrarActionsFeed: View registrar actions
RegistrarActionsFeed->>NamehashUI: RegistrarActionCardMemo (with links config)
NamehashUI-->>RegistrarActionsFeed: Rendered action cards
RegistrarActionsFeed->>RegistrarActionsFeed: getReferralQualificationInfo()
RegistrarActionsFeed->>NamehashUI: LabeledField (incentive program)
NamehashUI-->>User: Display actions with referral programs
|
lightwalker-eth
left a comment
There was a problem hiding this comment.
@Y3drk Super work here 🚀
Substantial PR → apply
namehash-uiCloses Issues #906 and #105
Reviewer Focus (Read This First)
What reviewers should focus on
namehash-uiensawards.org/src/components/referral-awards-program/referrers/ReferrerLeaderboardSnippet.tsxandensawards.org/src/components/referral-awards-program/referrers/ReferrerLeaderboard.tsx)Problem & Motivation
Why this exists
Phase 5of the strategy suggested in the aforementioned Issue. The 1st part was about applying the package in ENSAdmin.What Changed (Concrete)
What actually changed
namehash-uipackage and deleted unused code. (changes mostly located inensawards.org/src/components)ENSNodeClientwithENSReferralsClientinensawards.org/src/components/referral-awards-program/referrers/ReferrerLeaderboardSnippet.tsxandensawards.org/src/components/referral-awards-program/referrers/ReferrerLeaderboard.tsxtypescriptas adevDependency(seepnpm-workspace.yaml:5,package.json:15, andensawards.org/package.json:66)Design & Planning
How this approach was chosen
Phase 5...Self-Review
packages/namehash-ui/src/components/registrar-actions/RegistrarActionCard.tsxand therefore I had to make a small fixing PR in theensnoderepo (PR #1552). This bug is non-breaking.namehash-uicounterparts.Cross-Codebase Alignment
apps/ensadmin,packages/ens-referrals,packages/namehash-ui(in ensnode repo, there is a small, separate PR #1552 with a minor refactor)packages/namehash-ui/src/components/identity/Identity.tsx:120-121)Downstream & Consumer Impact
Who this affects and how
ENSNodeClient→ENSReferralsClient)Testing Evidence
namehash-uipackage version but the preview one (@next)Scope Reductions
What you intentionally didn't do
No scope reductions.
Risk Analysis
How this could go wrong
@nextversion of the package in the production deployment of ENSAwards (not illegal per se, but against good practices. We could also wait for a new release before merging, but that seems unnecessary).Pre-Review Checklist (Blocking)