-
Notifications
You must be signed in to change notification settings - Fork 146
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
feat: domain popup #171
feat: domain popup #171
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Super Cool ! Some Important code changes needed :)
components/quests/reward.tsx
Outdated
onClick={() => window.open("https://app.starknet.id/")} | ||
onClose={() => setShowDomainPopup(false)} | ||
/> | ||
) : null} | ||
<div className="flex"> |
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.
why is it here too ?
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.
Because in the issue it is written "When the user wants to claim a reward after completing the quest or to connect wallet and doesn't have a domain, they are prompted to get one through a popup that redirects them to the Starknet ID page. When the user doesn't have a domain yet, a "Get Your Domain" button is visible on their profile."
So that's why I am also showing the popup when trying to claim a reward
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.
Ok i see to me you can delete that because we'll make indispensable once the guy is connected on the quest page and can not close it.
components/quests/reward.tsx
Outdated
banner="/visuals/profile.webp" | ||
description="To access Starknet Quest, you must own a Starknet domain. It's your passport to the Starknet ecosystem. Get yours now." | ||
buttonName="Get a Starknet Domain" | ||
onClick={() => window.open("https://app.starknet.id/")} |
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.
You need to check if we're on testnet or not
hooks/useHasDomain.ts
Outdated
import { StarknetIdJsContext } from "../context/StarknetIdJsProvider"; | ||
import { hexToDecimal } from "../utils/feltService"; | ||
|
||
export default function useHasDomain(address: string | BN | undefined) { |
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.
useHasRootDomain :)
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.
Just a last 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.
Lgtm
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.
Lgtm
close: #147