Fix #924 - some expiring domains not showing as expired #930
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@emarc99 is attempting to deploy a commit to the LFG Labs Team on Vercel. A member of the Team first needs to authorize it. |
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request introduces changes to multiple components focusing on domain expiration handling and user interface improvements. The modifications include updating the Changes
Assessment against linked issues
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (5)
components/UI/walletConnect.tsx (2)
36-39: Consider adding error handlingWhile the implementation is correct, consider adding try-catch to handle potential errors when checking for window.starknet_argentX.
useEffect(() => { - if (typeof window !== "undefined") - setIsArgentMobile(isInArgentMobileAppBrowser()); + if (typeof window !== "undefined") { + try { + setIsArgentMobile(isInArgentMobileAppBrowser()); + } catch (error) { + console.error("Failed to detect Argent mobile browser:", error); + setIsArgentMobile(false); + } + } }, []);
105-113: Consider extracting the wallet name logic for better readabilityWhile the implementation is correct, the wallet name logic could be extracted into a helper function for better readability.
+const getDisplayWalletName = (connector: Connector, isMobile: boolean, needInstall: boolean) => { + const prefix = needInstall ? "Install " : ""; + const name = connector.id === "argentMobile" && isMobile + ? "Argent" + : getConnectorName(connector.id); + return `${prefix}${name}`; +}; <p> - {needInstall(connector, isAvailable) ? "Install " : ""} - {connector.id === "argentMobile" && isMobile - ? "Argent" - : getConnectorName(connector.id)} + {getDisplayWalletName(connector, isMobile, needInstall(connector, isAvailable))} </p>components/identities/identitiesGalleryV1.tsx (1)
72-89: Consider enhancing the expired domain warningWhile the current implementation meets the basic requirements, consider these enhancements to make the expired status more prominent:
- Add a visual "EXPIRED" badge or label next to the domain name
- Use a more prominent warning container instead of just an icon
- Add a direct "Renew" button in the warning
Example implementation:
<div className={styles.expiryWarning}> <Tooltip title={ isIdentityExpired(identity) ? "This domain has expired! Renew it now to keep using it." : `This domain will expire on ${timestampToReadableDate(identity?.domain_expiry ?? 0)}` } arrow > <div className={styles.warningContainer}> <RenewalIcon color={isIdentityExpired(identity) ? "#FF3333" : "#FFDE21"} width="12" /> + {isIdentityExpired(identity) && ( + <span className={styles.expiredBadge}>EXPIRED</span> + )} + {isIdentityExpired(identity) && ( + <button + className={styles.renewButton} + onClick={(e) => { + e.stopPropagation(); + router.push('/subscription'); + }} + > + Renew + </button> + )} </div> </Tooltip> </div>Add these CSS classes to your styles module:
.warningContainer { display: flex; align-items: center; gap: 4px; } .expiredBadge { background: #FF3333; color: white; padding: 2px 4px; border-radius: 4px; font-size: 10px; font-weight: bold; } .renewButton { background: #FF3333; color: white; padding: 2px 8px; border-radius: 4px; font-size: 12px; cursor: pointer; }🧰 Tools
🪛 Biome (1.9.4)
[error] 72-72: Expected an expression but instead found '<<'.
Expected an expression here.
(parse)
[error] 72-72: Expected an expression but instead found '<<'.
Expected an expression here.
(parse)
[error] 73-74: Expected a JSX attribute but instead found '? "This domain is expired"
: `'.Expected a JSX attribute here.
(parse)
[error] 74-74: Expected a JSX attribute but instead found ', this'.
Expected a JSX attribute here.
(parse)
[error] 74-74: expected
...but instead foundtimestampToReadableDateRemove timestampToReadableDate
(parse)
[error] 74-75: Expected an expression for the left hand side of the
===operator.This operator requires a left hand side value
(parse)
[error] 75-75: Expected an expression but instead found '==='.
Expected an expression here.
(parse)
[error] 75-75: Expected an expression but instead found '='.
Expected an expression here.
(parse)
[error] 74-75: Invalid assignment to
======This expression cannot be assigned to
(parse)
[error] 76-76: Expected an expression, or an assignment but instead found '?'.
Expected an expression, or an assignment here.
(parse)
[error] 77-78: Expected an expression for the left hand side of the
>>>operator.This operator requires a left hand side value
(parse)
[error] 78-78: Expected an expression but instead found '>>>'.
Expected an expression here.
(parse)
[error] 78-78: Expected an expression but instead found '>'.
Expected an expression here.
(parse)
[error] 78-78: expected
,but instead found:Remove :
(parse)
[error] 79-79: expected
,but instead foundidentityRemove identity
(parse)
[error] 83-84: Expected a JSX Expression, a Element, or a text but instead found '<<<<<<'.
Expected a JSX Expression, a Element, or a text here.
(parse)
[error] 85-85: expected
,but instead foundcolorRemove color
(parse)
[error] 85-85: expected
,but instead found=Remove =
(parse)
[error] 85-86: Expected a JSX attribute but instead found '======='.
Expected a JSX attribute here.
(parse)
[error] 87-87: expected
...but instead found}Remove }
(parse)
[error] 88-88: expected
>but instead found<Remove <
(parse)
[error] 84-87: Expected corresponding JSX closing tag for 'HEAD'.
Opening tag
closing tag
(parse)
[error] 72-83: Expected corresponding JSX closing tag for 'HEAD'.
Opening tag
closing tag
(parse)
[error] 88-89: Unexpected token. Did you mean
{'>'}or>?(parse)
[error] 89-89: Unexpected token. Did you mean
{'>'}or>?(parse)
[error] 89-89: Unexpected token. Did you mean
{'>'}or>?(parse)
[error] 89-89: Unexpected token. Did you mean
{'>'}or>?(parse)
[error] 89-89: Unexpected token. Did you mean
{'>'}or>?(parse)
[error] 89-89: Unexpected token. Did you mean
{'>'}or>?(parse)
[error] 89-89: Unexpected token. Did you mean
{'>'}or>?(parse)
[error] 74-75: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
utils/connectorWrapper.ts (2)
Line range hint
41-92: Add error handling for local storage operations.The local storage operations in
getLastConnectorandgetLastConnectedshould include try-catch blocks to handle potential errors.Consider updating the implementation:
export const getLastConnector = (): Connector | null => { - if (localStorage.getItem("SID-lastUsedConnector")) { - const connectordId = localStorage.getItem("SID-lastUsedConnector"); - const connector = getConnectors().find((item) => item.id === connectordId); - return connector || null; + try { + const connectorId = localStorage.getItem("SID-lastUsedConnector"); + if (!connectorId) return null; + return getConnectors().find((item) => item.id === connectorId) || null; + } catch (error) { + console.error("Error accessing localStorage:", error); + return null; } - return null; }; export const getLastConnected = (): Connector | null => { - if (localStorage.getItem("SID-connectedWallet")) { - const connectordId = localStorage.getItem("SID-connectedWallet"); - const connector = getConnectors().find((item) => item.id === connectordId); - return connector || null; + try { + const connectorId = localStorage.getItem("SID-connectedWallet"); + if (!connectorId) return null; + return getConnectors().find((item) => item.id === connectorId) || null; + } catch (error) { + console.error("Error accessing localStorage:", error); + return null; } - return null; };
93-104: Improve type safety in the Argent mobile browser detection.While the function works correctly, we can enhance type safety and readability.
Consider this improved implementation:
export const isInArgentMobileAppBrowser = (): boolean => { - if (typeof window === "undefined" || !window?.starknet_argentX) { + if (typeof window === "undefined" || !window?.starknet_argentX || typeof window.starknet_argentX !== "object") { return false; } - const starknetMobile = - window?.starknet_argentX as unknown as StarknetWindowObject & { - isInAppBrowser: boolean; - }; + interface ArgentMobileWindow extends StarknetWindowObject { + isInAppBrowser: boolean; + } + + const starknetMobile = window.starknet_argentX as ArgentMobileWindow; - return starknetMobile?.isInAppBrowser; + return Boolean(starknetMobile?.isInAppBrowser); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
.gitignore(1 hunks)components/UI/navbar.tsx(1 hunks)components/UI/walletConnect.tsx(4 hunks)components/identities/identitiesGalleryV1.tsx(1 hunks)public/fonts/poppins/OFL.txt(1 hunks)utils/connectorWrapper.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🧰 Additional context used
🪛 Biome (1.9.4)
components/UI/navbar.tsx
[error] 309-309: Expected a type parameter but instead found '<<<<<<'.
Expected a type parameter here.
(parse)
[error] 309-309: expected , but instead found HEAD
Remove HEAD
(parse)
[error] 310-310: expected , but instead found href
Remove href
(parse)
[error] 310-310: expected , but instead found =
Remove =
(parse)
[error] 310-310: expected , but instead found process
Remove process
(parse)
[error] 310-310: expected , but instead found as
Remove as
(parse)
[error] 310-310: expected , but instead found string
Remove string
(parse)
[error] 310-310: expected , but instead found }
Remove }
(parse)
[error] 310-311: expected , but instead found ===
Remove ===
(parse)
[error] 313-313: Unexpected token. Did you mean {'>'} or >?
(parse)
[error] 313-313: Unexpected token. Did you mean {'>'} or >?
(parse)
[error] 313-313: Unexpected token. Did you mean {'>'} or >?
(parse)
[error] 313-313: Unexpected token. Did you mean {'>'} or >?
(parse)
[error] 313-313: Unexpected token. Did you mean {'>'} or >?
(parse)
[error] 313-313: Unexpected token. Did you mean {'>'} or >?
(parse)
components/identities/identitiesGalleryV1.tsx
[error] 72-72: Expected an expression but instead found '<<'.
Expected an expression here.
(parse)
[error] 72-72: Expected an expression but instead found '<<'.
Expected an expression here.
(parse)
[error] 73-74: Expected a JSX attribute but instead found '? "This domain is expired"
: `'.
Expected a JSX attribute here.
(parse)
[error] 74-74: Expected a JSX attribute but instead found ', this'.
Expected a JSX attribute here.
(parse)
[error] 74-74: expected ... but instead found timestampToReadableDate
Remove timestampToReadableDate
(parse)
[error] 74-75: Expected an expression for the left hand side of the === operator.
This operator requires a left hand side value
(parse)
[error] 75-75: Expected an expression but instead found '==='.
Expected an expression here.
(parse)
[error] 75-75: Expected an expression but instead found '='.
Expected an expression here.
(parse)
[error] 74-75: Invalid assignment to ======
This expression cannot be assigned to
(parse)
[error] 76-76: Expected an expression, or an assignment but instead found '?'.
Expected an expression, or an assignment here.
(parse)
[error] 77-78: Expected an expression for the left hand side of the >>> operator.
This operator requires a left hand side value
(parse)
[error] 78-78: Expected an expression but instead found '>>>'.
Expected an expression here.
(parse)
[error] 78-78: Expected an expression but instead found '>'.
Expected an expression here.
(parse)
[error] 78-78: expected , but instead found :
Remove :
(parse)
[error] 79-79: expected , but instead found identity
Remove identity
(parse)
[error] 83-84: Expected a JSX Expression, a Element, or a text but instead found '<<<<<<'.
Expected a JSX Expression, a Element, or a text here.
(parse)
[error] 85-85: expected , but instead found color
Remove color
(parse)
[error] 85-85: expected , but instead found =
Remove =
(parse)
[error] 85-86: Expected a JSX attribute but instead found '======='.
Expected a JSX attribute here.
(parse)
[error] 87-87: expected ... but instead found }
Remove }
(parse)
[error] 88-88: expected > but instead found <
Remove <
(parse)
[error] 84-87: Expected corresponding JSX closing tag for 'HEAD'.
Opening tag
closing tag
(parse)
[error] 72-83: Expected corresponding JSX closing tag for 'HEAD'.
Opening tag
closing tag
(parse)
[error] 88-89: Unexpected token. Did you mean {'>'} or >?
(parse)
[error] 89-89: Unexpected token. Did you mean {'>'} or >?
(parse)
[error] 89-89: Unexpected token. Did you mean {'>'} or >?
(parse)
[error] 89-89: Unexpected token. Did you mean {'>'} or >?
(parse)
[error] 89-89: Unexpected token. Did you mean {'>'} or >?
(parse)
[error] 89-89: Unexpected token. Did you mean {'>'} or >?
(parse)
[error] 89-89: Unexpected token. Did you mean {'>'} or >?
(parse)
[error] 74-75: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
🪛 LanguageTool
public/fonts/poppins/OFL.txt
[typographical] ~9-~9: If you want to indicate numerical ranges or time ranges, consider using an en dash.
Context: ...------ SIL OPEN FONT LICENSE Version 1.1 - 26 February 2007 -----------------------...
(DASH_RULE)
🔇 Additional comments (4)
components/UI/walletConnect.tsx (2)
1-1: LGTM! Clean implementation of state management
The addition of the Argent mobile detection state and its import follow React best practices.
Also applies to: 11-11, 29-29
Line range hint 41-47: Verify connector filtering behavior across different environments
The filtering logic looks correct, but let's verify its behavior across different environments and wallet configurations.
✅ Verification successful
Connector filtering logic is correctly implemented and consistent across the codebase
The verification confirms that the connector filtering logic is well-implemented and consistent:
- The
filterConnectorsfunction is only used inwalletConnect.tsxand follows clear, predictable logic:- Returns only argentMobile connector if in Argent mobile browser
- Returns all connectors for desktop
- Filters out argentX on mobile devices
- The implementation is supported by comprehensive test coverage in
connectorWrapper.test.js - The filtering works in conjunction with
sortConnectorswhich properly orders available connectors first - The connector configuration in
connectorWrapper.tsaligns with the filtering logic, properly defining all supported wallet types
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other uses of filterConnectors to ensure consistent behavior
rg -A 5 "filterConnectors" --type typescript
# Search for tests related to connector filtering
rg -l "filterConnectors.*test" --type typescript
Length of output: 165
Script:
#!/bin/bash
# Search for other uses of filterConnectors without file type restriction
rg -A 5 "filterConnectors"
# Search for tests related to connector filtering
rg -l "filterConnectors.*test"
# Search for connector-related files
fd -e ts -e tsx "connector|wallet"
Length of output: 1143
Script:
#!/bin/bash
# Check the implementation of sortConnectors and related wallet logic
cat components/UI/walletConnect.tsx
# Check connector wrapper implementation
cat utils/connectorWrapper.ts
# Look for any connector configuration or setup
rg "Connector" -A 5
Length of output: 61632
components/identities/identitiesGalleryV1.tsx (1)
76-78: Improved UI/UX for domain expiration warnings
The changes enhance the user experience through:
- More actionable tooltip text that guides users to renew
- Consistent iconography using RenewalIcon
- Clear visual hierarchy with red for expired and yellow for expiring soon domains
Also applies to: 88-89
🧰 Tools
🪛 Biome (1.9.4)
[error] 76-76: Expected an expression, or an assignment but instead found '?'.
Expected an expression, or an assignment here.
(parse)
[error] 77-78: Expected an expression for the left hand side of the >>> operator.
This operator requires a left hand side value
(parse)
[error] 78-78: Expected an expression but instead found '>>>'.
Expected an expression here.
(parse)
[error] 78-78: Expected an expression but instead found '>'.
Expected an expression here.
(parse)
[error] 78-78: expected , but instead found :
Remove :
(parse)
utils/connectorWrapper.ts (1)
Line range hint 1-40: LGTM! Connector setup and imports are well-structured.
The connector configuration is properly implemented with appropriate imports and environment variable usage.
| <<<<<<< HEAD | ||
| ? "This domain is expired" | ||
| : `Be careful, this domain will expire on ${timestampToReadableDate( | ||
| ======= | ||
| ? "This domain has expired! Renew it now to keep using it." | ||
| : `This domain will expire on ${timestampToReadableDate( | ||
| >>>>>>> e09c027 (Fix: Add distinct warning for expired domains) | ||
| identity?.domain_expiry ?? 0 | ||
| )}` | ||
| } | ||
| arrow | ||
| > | ||
| <<<<<<< HEAD | ||
| <ErrorIcon color="error" /> | ||
| ======= | ||
| {/* <ErrorIcon color="error" /> */} | ||
| <RenewalIcon color={isIdentityExpired(identity) ? "#FF3333" : "#FFDE21"} width="12" /> | ||
| >>>>>>> e09c027 (Fix: Add distinct warning for expired domains) |
There was a problem hiding this comment.
Resolve merge conflicts before merging
There are unresolved merge conflicts in the code. The conflicts involve changes to the tooltip text and icon for expired domains. Please resolve these conflicts by choosing the appropriate version:
-
Original version:
- Message: "This domain is expired"
- Icon: ErrorIcon in red
-
New version:
- Message: "This domain has expired! Renew it now to keep using it."
- Icon: RenewalIcon with conditional color
🧰 Tools
🪛 Biome (1.9.4)
[error] 72-72: Expected an expression but instead found '<<'.
Expected an expression here.
(parse)
[error] 72-72: Expected an expression but instead found '<<'.
Expected an expression here.
(parse)
[error] 73-74: Expected a JSX attribute but instead found '? "This domain is expired"
: `'.
Expected a JSX attribute here.
(parse)
[error] 74-74: Expected a JSX attribute but instead found ', this'.
Expected a JSX attribute here.
(parse)
[error] 74-74: expected ... but instead found timestampToReadableDate
Remove timestampToReadableDate
(parse)
[error] 74-75: Expected an expression for the left hand side of the === operator.
This operator requires a left hand side value
(parse)
[error] 75-75: Expected an expression but instead found '==='.
Expected an expression here.
(parse)
[error] 75-75: Expected an expression but instead found '='.
Expected an expression here.
(parse)
[error] 74-75: Invalid assignment to ======
This expression cannot be assigned to
(parse)
[error] 76-76: Expected an expression, or an assignment but instead found '?'.
Expected an expression, or an assignment here.
(parse)
[error] 77-78: Expected an expression for the left hand side of the >>> operator.
This operator requires a left hand side value
(parse)
[error] 78-78: Expected an expression but instead found '>>>'.
Expected an expression here.
(parse)
[error] 78-78: Expected an expression but instead found '>'.
Expected an expression here.
(parse)
[error] 78-78: expected , but instead found :
Remove :
(parse)
[error] 79-79: expected , but instead found identity
Remove identity
(parse)
[error] 83-84: Expected a JSX Expression, a Element, or a text but instead found '<<<<<<'.
Expected a JSX Expression, a Element, or a text here.
(parse)
[error] 85-85: expected , but instead found color
Remove color
(parse)
[error] 85-85: expected , but instead found =
Remove =
(parse)
[error] 85-86: Expected a JSX attribute but instead found '======='.
Expected a JSX attribute here.
(parse)
[error] 87-87: expected ... but instead found }
Remove }
(parse)
[error] 88-88: expected > but instead found <
Remove <
(parse)
[error] 84-87: Expected corresponding JSX closing tag for 'HEAD'.
Opening tag
closing tag
(parse)
[error] 72-83: Expected corresponding JSX closing tag for 'HEAD'.
Opening tag
closing tag
(parse)
[error] 88-89: Unexpected token. Did you mean {'>'} or >?
(parse)
[error] 89-89: Unexpected token. Did you mean {'>'} or >?
(parse)
[error] 89-89: Unexpected token. Did you mean {'>'} or >?
(parse)
[error] 89-89: Unexpected token. Did you mean {'>'} or >?
(parse)
[error] 89-89: Unexpected token. Did you mean {'>'} or >?
(parse)
[error] 89-89: Unexpected token. Did you mean {'>'} or >?
(parse)
[error] 89-89: Unexpected token. Did you mean {'>'} or >?
(parse)
[error] 74-75: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
components/UI/navbar.tsx
Outdated
| <<<<<<< HEAD | ||
| href={process.env.NEXT_PUBLIC_STARKNET_ID as string} | ||
| ======= | ||
| href={ `${process.env.NEXT_PUBLIC_STARKNET_ID as string}` } | ||
| >>>>>>> e09c027 (Fix: Add distinct warning for expired domains) |
There was a problem hiding this comment.
Resolve merge conflict markers
There's an unresolved merge conflict in the code. The conflict markers (<<<<<<< HEAD, =======, >>>>>>>) need to be removed, and the correct version needs to be selected.
Remove the conflict markers and choose one of these versions:
- <<<<<<< HEAD
- href={process.env.NEXT_PUBLIC_STARKNET_ID as string}
- =======
- href={ `${process.env.NEXT_PUBLIC_STARKNET_ID as string}` }
- >>>>>>> e09c027 (Fix: Add distinct warning for expired domains)
+ href={process.env.NEXT_PUBLIC_STARKNET_ID as string}Note: The change to wrap the environment variable in a template literal (${...}) appears unnecessary as it doesn't affect the string's value. Additionally, this change seems unrelated to the PR's objective of adding warnings for expired domains.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <<<<<<< HEAD | |
| href={process.env.NEXT_PUBLIC_STARKNET_ID as string} | |
| ======= | |
| href={ `${process.env.NEXT_PUBLIC_STARKNET_ID as string}` } | |
| >>>>>>> e09c027 (Fix: Add distinct warning for expired domains) | |
| href={process.env.NEXT_PUBLIC_STARKNET_ID as string} |
🧰 Tools
🪛 Biome (1.9.4)
[error] 309-309: Expected a type parameter but instead found '<<<<<<'.
Expected a type parameter here.
(parse)
[error] 309-309: expected , but instead found HEAD
Remove HEAD
(parse)
[error] 310-310: expected , but instead found href
Remove href
(parse)
[error] 310-310: expected , but instead found =
Remove =
(parse)
[error] 310-310: expected , but instead found process
Remove process
(parse)
[error] 310-310: expected , but instead found as
Remove as
(parse)
[error] 310-310: expected , but instead found string
Remove string
(parse)
[error] 310-310: expected , but instead found }
Remove }
(parse)
[error] 310-311: expected , but instead found ===
Remove ===
(parse)
[error] 313-313: Unexpected token. Did you mean {'>'} or >?
(parse)
[error] 313-313: Unexpected token. Did you mean {'>'} or >?
(parse)
[error] 313-313: Unexpected token. Did you mean {'>'} or >?
(parse)
[error] 313-313: Unexpected token. Did you mean {'>'} or >?
(parse)
[error] 313-313: Unexpected token. Did you mean {'>'} or >?
(parse)
[error] 313-313: Unexpected token. Did you mean {'>'} or >?
(parse)
|
wen ready ? @emarc99 |
|
I am still working on it, trying to detect where errors occur by passing statically-generated domain_expiry values in precise format as the response from api.starknet.id and using relevant functions within the project. Will make a PR soon, please.
|
need to finish asap for OD hack |
There was a problem hiding this comment.
Doesn't work for me.
The aim of the issue is just to show the red warning icon on every expired domain. Where currently there are some domains that show as expired, and other that doesn't, even though they are expired. You can check on my screenshot, the "TESTET.STARK" domain should show as expired.
You can try to mock with my address if it can help you: 0x05F1F8De723d8117daA26ec24320d0EacabC53a3D642Acb0880846486e73283a
| identity?.domain_expiry ?? 0 | ||
| )}` | ||
| } | ||
| arrow | ||
| > | ||
| <ErrorIcon color="error" /> | ||
| {/* <ErrorIcon color="error" /> */} | ||
| <RenewalIcon color={isIdentityExpired(identity) ? "#FF3333" : "#FFDE21"} width="12" /> |
There was a problem hiding this comment.
Why are you changing the icon ? Also the hover tooltip doesn't work for me on your PR
There was a problem hiding this comment.
Ok, I'll make sure the requested changes are applied, and try the mock address, thanks.
There was a problem hiding this comment.
curl https://api.starknet.id/renewal/get_non_subscribed_domains?addr=0x05F1F8De723d8117daA26ec24320d0EacabC53a3D642Acb0880846486e73283a
gives below result
["testapple.stark","sdfsdqdsd.stark","testniin.stark","qzdqsddqz.stark","test5.stark","niko.stark","dsqqsdsqds.stark","testnico.stark","testnicooo.stark"]
There was a problem hiding this comment.
curl https://api.starknet.id/renewal/get_non_subscribed_domains?addr=0x05F1F8De723d8117daA26ec24320d0EacabC53a3D642Acb0880846486e73283agives below result ["testapple.stark","sdfsdqdsd.stark","testniin.stark","qzdqsddqz.stark","test5.stark","niko.stark","dsqqsdsqds.stark","testnico.stark","testnicooo.stark"]
Yes, because it returns the domains that are not subscribed to auto renewal.
There was a problem hiding this comment.
Obviously, the ones from response have the red icon from the image you shared. "TESTET.STARK" is missing.
There was a problem hiding this comment.
curl https://api.starknet.id/renewal/get_non_subscribed_domains?addr=0x05F1F8De723d8117daA26ec24320d0EacabC53a3D642Acb0880846486e73283agives below result ["testapple.stark","sdfsdqdsd.stark","testniin.stark","qzdqsddqz.stark","test5.stark","niko.stark","dsqqsdsqds.stark","testnico.stark","testnicooo.stark"]Yes, because it returns the domains that are not subscribed to auto renewal.
So, can I change this to get all domains, then filter to get expired ones for showing red for expired and yellow for expiring soon?
Basically, I am asking if I can use another api edpoint apart from /renewal/get_non_subscribed_domains? to achieve my task? I am thinking of using /addr_to_full_ids?addr= for expired/expiring soon and /renewal/get_non_subscribed_domains? for renew subscription indicator.
There was a problem hiding this comment.
Obviously, the ones from response have the red icon from the image you shared. "TESTET.STARK" is missing.
No, e.g.: niko.stark is not expired but is returned by this route as I disabled the autorenewal for it
Please don't use this route, it has nothing to do with the issue
There was a problem hiding this comment.
After studying api.starknet.id/src/endpoints/renewal /get_non_subscribed_domains.rs The renewal/get_non_subscribed_domains response, filters out some expired domains based on their renewal status. This means even though they are expired, they are not considered "non-subscribed"
Yes but that's not what we want
There was a problem hiding this comment.
Just use the expiry of each domain to determine if it's expired, and then show the red icon to tell the user it's expired
…starknet.id into fix-lfglabs-dev#924 Implemented distinct warning for expired domains
|
@Marchand-Nicolas please help test. I tested by mocking your public address. |
| <ErrorIcon color="error" /> | ||
| </Tooltip> | ||
| </div> | ||
| ) : isExpiringSoon ? ( |
There was a problem hiding this comment.
Here it should be isExpiringSoon && needAutoRenewal?.includes(identity.domain)
Because we don't want to tell the user it'll expire soon if they enabled auto renewal (as it'll be renewed automatically)
emarc99
changed the title
Close #924
Summary by CodeRabbit
New Features
Bug Fixes
Chores
.envto the.gitignorefile for better version control practices.