Skip to content
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.

Sign up for GitHub

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: added modal for expired Domain #1075

Open
wants to merge 1 commit into
base: testnet
Choose a base branch
from
Open

Fix: added modal for expired Domain #1075

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 29 additions & 1 deletion components/UI/navbar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,13 @@ const Navbar: FunctionComponent = () => {
const [profile, setProfile] = useState<StarkProfile | undefined>(undefined);
const { starknetIdNavigator } = useContext(StarknetIdJsContext);
const { isWrongNetwork, setIsWrongNetwork } = useIsWrongNetwork();

const [showWalletConnectModal, setShowWalletConnectModal] =
useState<boolean>(false);

const [showModalExpiredDomain, setShowModalExpiredDomain] =
useState<boolean>(false);

const router = useRouter();
const { switchChainAsync } = useSwitchChain({
params: {
Expand Down Expand Up @@ -186,7 +191,12 @@ const Navbar: FunctionComponent = () => {
<li className={styles.menuItem}>My Identities</li>
</Link>
<Link href="/">
<li className={styles.menuItem}>Domains</li>
<li
className={styles.menuItem}
onClick={() => setShowModalExpiredDomain(true)}
>
Domains
</li>
Comment on lines +194 to +199
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve UX by separating navigation and modal trigger.

The current implementation is potentially confusing as it uses a Link component but prevents navigation with onClick. This could lead to unexpected behavior and poor user experience.

Consider one of these approaches:

  1. Remove the Link wrapper if navigation is not intended:
-<Link href="/">
  <li
    className={styles.menuItem}
    onClick={() => setShowModalExpiredDomain(true)}
  >
    Domains
  </li>
-</Link>
  1. Or handle both navigation and modal display if both are needed:
-<Link href="/">
+<Link href="/domains">
  <li
    className={styles.menuItem}
    onClick={(e) => {
+     e.preventDefault();
      setShowModalExpiredDomain(true);
    }}
  >
    Domains
  </li>
</Link>

Committable suggestion skipped: line range outside the PR's diff.

</Link>
{/* <Link href="/jointhetribe">
<li className={styles.menuItem}>Join the tribe</li>
Expand Down Expand Up @@ -425,6 +435,24 @@ const Navbar: FunctionComponent = () => {
}
lottie={errorLottie}
/>

<ModalMessage
open={showModalExpiredDomain}
title={"Domain Expiration Notice"}
closeModal={() => setShowModalExpiredDomain(false)}
message={
<div className="mt-3 flex flex-col items-center justify-center text-center mx-3">
<p>
Your domain has expired. Would you like to renew it now to keep
your website active?
</p>
<div className="mt-5">
<Button onClick={() => alert('Renew Domain clicked')}>Renew Domain</Button>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Replace alert with proper domain renewal handling.

Using alert() for domain renewal is not production-ready and provides poor user experience.

Consider implementing a proper domain renewal handler:

-<Button onClick={() => alert('Renew Domain clicked')}>Renew Domain</Button>
+<Button onClick={() => handleDomainRenewal()}>Renew Domain</Button>

Add the following handler function to manage the renewal process:

const handleDomainRenewal = async () => {
  try {
    // Implement domain renewal logic here
    setShowModalExpiredDomain(false);
  } catch (error) {
    // Handle errors appropriately
    console.error('Domain renewal failed:', error);
  }
};

</div>
</div>
}
// lottie={errorLottie}
/>
<ModalWallet
domain={domainOrAddress}
open={showWallet}
Expand Down