-
Notifications
You must be signed in to change notification settings - Fork 16
fix(namehash-ui): Fix <RegistrarActionCard> props #1552
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(namehash-ui): Fix <RegistrarActionCard> props #1552
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
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. 📝 WalkthroughWalkthroughReplaces a function-style referrerLinkFunction with a ReferrerLinkData object (isExternal + getLink) across registrar action components. Updates prop names and call sites so referrer resolution uses referrerLinkData.getLink(...) and referrerLinkData.isExternal instead of the previous function prop. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
…/fix/fix-registrar-action-card-props
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors the RegistrarActionCard component's props to improve type safety and API consistency. The change was identified as a bug fix during self-review of PR #109 in ENSAwards.
Changes:
- Replaced the
referrerLinkFunctionprop with areferrerLinkDataobject that encapsulates both the link generation function and anisExternalboolean flag - Updated the
RegistrarActionCardcomponent and its internalResolveAndDisplayReferrerIdentityhelper to use the new prop structure - Applied the refactored prop structure in ENSAdmin's usage of the component
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
packages/namehash-ui/src/components/registrar-actions/RegistrarActionCard.tsx |
Introduced ReferrerLinkData interface and updated component props to use the new structure instead of ReferrerLinkFunction |
apps/ensadmin/src/components/registrar-actions/display-registrar-actions-panel.tsx |
Updated to pass referrer object with isExternal and getLink properties instead of referrerLinkFunction |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
packages/namehash-ui/src/components/registrar-actions/RegistrarActionCard.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Greptile SummaryRefactored the Key Changes:
This change enables the component to be properly integrated into ENSAwards (external repository) where different link behaviors are needed. Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Consumer as ENSAdmin Consumer
participant Card as RegistrarActionCard
participant ReferrerDisplay as ResolveAndDisplayReferrerIdentity
participant Identity as ResolveAndDisplayIdentity
participant LinkResolver as getEnsManagerAddressDetailsUrl
Consumer->>Card: Pass referrer prop {isExternal, getLink}
Card->>ReferrerDisplay: Pass referrerLinkData
ReferrerDisplay->>ReferrerDisplay: Decode referral & validate
alt Valid referrer address
ReferrerDisplay->>LinkResolver: getLink(address, namespaceId)
LinkResolver-->>ReferrerDisplay: URL | null
ReferrerDisplay->>Identity: Pass identityLinkDetails {isExternal, link}
Identity-->>Card: Render referrer with link
else Invalid/zero referrer
ReferrerDisplay-->>Card: Render fallback (-/Unknown)
end
|
lightwalker-eth
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Y3drk Looks good, thanks!
Lite PR → Fix
RegistrarActionCardpropsSummary
RegistrarActionCardcomponent in thenamehash-ui(seepackages/namehash-ui/src/components/registrar-actions/RegistrarActionCard.tsx:60-70) and applied these changes in ENSAdminWhy
Testing
<RegistrarActionCard>) in ENSAdmin locally and in the Vercel preview. Test shown expected behavior.Pre-Review Checklist (Blocking)
Deemed changesets unnecessary, because:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.