-
Notifications
You must be signed in to change notification settings - Fork 116
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
Update the Domain Expiration feature - Remove buttons #1089
Update the Domain Expiration feature - Remove buttons #1089
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@tali-creator 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 several updates across multiple components. The Changes
Possibly related PRs
Suggested labels
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (7)
✨ Finishing Touches
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 (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (3)
components/UI/iconsComponents/icons/renewalIcon.tsx (1)
3-3
: Consider adding a default color value.While the component works correctly, consider adding a default value for the color prop to ensure the icon renders properly even if the color prop is omitted.
-const RenewalIcon: FunctionComponent<IconProps> = ({ color, width }) => { +const RenewalIcon: FunctionComponent<IconProps> = ({ color = "#0C8654", width }) => {components/identities/actions/identityActions.tsx (2)
250-251
: Consider extracting Tailwind classes to a named class.The inline Tailwind classes make the JSX harder to read. Consider extracting these to a named class in your CSS module or a component to improve readability and maintainability.
-<button - className="flex items-center bg-white hover:opacity-80 transition-all duration-300 w-[280px] h-[60px] rounded-[12px] border-t-[1px] border-r-[4px] border-b-[4px] border-[#D32F2F] border-l-[1px] p-[8px] gap-[12px]" - onClick={() => router.push("/renewal")}> +<button + className={styles.expiredDomainButton} + onClick={() => router.push("/renewal")}>Then define the class in your module CSS file:
.expiredDomainButton { display: flex; align-items: center; background-color: white; width: 280px; height: 60px; border-radius: 12px; border-top: 1px solid #D32F2F; border-right: 4px solid #D32F2F; border-bottom: 4px solid #D32F2F; border-left: 1px solid #D32F2F; padding: 8px; gap: 12px; transition: opacity 300ms; } .expiredDomainButton:hover { opacity: 0.8; }
260-263
: Use proper date formatting for expired domains.The current implementation shows "Expired on [date]". Consider expressing this in terms of how long ago it expired for better user context.
-<span className="text-[12px] text-[#8C8989]"> - {`Expired on ${timestampToReadableDate( - identity?.domainExpiry ?? 0 - )}`} -</span> +<span className="text-[12px] text-[#8C8989]"> + {`Expired ${getTimeSinceExpiry(identity?.domainExpiry ?? 0)} ago`} +</span>You would need to implement a helper function like:
function getTimeSinceExpiry(timestamp: number): string { const now = Math.floor(Date.now() / 1000); const secondsSinceExpiry = now - timestamp; if (secondsSinceExpiry < 60 * 60 * 24) { return "today"; } else if (secondsSinceExpiry < 60 * 60 * 24 * 7) { return `${Math.floor(secondsSinceExpiry / (60 * 60 * 24))} days`; } else if (secondsSinceExpiry < 60 * 60 * 24 * 30) { return `${Math.floor(secondsSinceExpiry / (60 * 60 * 24 * 7))} weeks`; } else { return `${Math.floor(secondsSinceExpiry / (60 * 60 * 24 * 30))} months`; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/UI/iconsComponents/icons/renewalIcon.tsx
(1 hunks)components/identities/actions/identityActions.tsx
(2 hunks)
🔇 Additional comments (7)
components/UI/iconsComponents/icons/renewalIcon.tsx (2)
17-17
: Good refactoring to use dynamic color prop.Making the icon's color configurable improves component reusability. This change allows the icon to be styled contextually based on the domain's status (e.g., expired vs. active).
21-21
: Consistent implementation of dynamic color prop.The color prop is correctly applied to all relevant SVG elements while maintaining the white highlight at line 33, which appears to be intentional.
Also applies to: 25-25, 29-29
components/identities/actions/identityActions.tsx (5)
248-265
: Improved UI for expired domains.The new button for expired domains effectively communicates urgency with its red styling and clear messaging. The design nicely separates this critical action from regular domain management actions.
269-282
: Consistent usage of RenewalIcon with color prop.The RenewalIcon now correctly uses the color prop, allowing it to adapt to different states and styles. Good job maintaining consistency between the expired and non-expired states.
285-295
: Improved organization of domain management actions.Moving the "CHANGE DOMAIN TARGET" action outside of the conditional rendering makes it consistently available, which is a good UX improvement.
297-370
: Well-structured "View more" section.The reorganization of actions with the "View more/less" toggle improves the UI by reducing clutter while keeping all functionality accessible. The conditional rendering logic is clear and effective.
336-356
:❓ Verification inconclusive
Verify subscription-related functionality.
The subscription actions appear correctly implemented, but since they involve transaction functionality, it's important to ensure they work as expected.
🏁 Script executed:
#!/bin/bash # Check if there are any references to the subscription functionality elsewhere # to ensure there are no inconsistencies # Search for subscription-related components or functions echo "Searching for subscription-related components or functions:" rg -A 2 -B 2 "subscription" --type tsx --type tsLength of output: 205
Review Verification: Subscription Actions Integration
The subscription actions in
components/identities/actions/identityActions.tsx
(lines 336–356) appear to be conditionally rendering based on the auto-renewal state. The code correctly provides separate UI elements for enabling and disabling subscription functionality. However, our initial automated search for subscription-related references was hindered by a file type recognition issue.Please ensure the following manually:
- Confirm that subscription transaction handling (triggered via the "ENABLE SUBSCRIPTION" and "DISABLE SUBSCRIPTION" actions) integrates correctly with the broader subscription logic.
- Verify that the
disableRenewal
function and related routing (usingrouter.push("/subscription")
) are properly connected elsewhere in the codebase.- Double-check that any subscription-related configurations and UI elements (like the renewal icons) are consistent and reliable.
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 did a really good job, lgtm!
close: #1028
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes