Fix: Redesign of the payment page in the final step of registration #990
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@aidenwong812 is attempting to deploy a commit to the LFG Labs Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe pull request involves a comprehensive redesign of the registration flow, focusing on the payment page and associated components. Changes span multiple files including Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
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: 1
🧹 Nitpick comments (15)
components/domains/steps/upsellCard.tsx (3)
46-49: Check heading structure for accessibility.
Using<h3>is beneficial for emphasis, but ensure there's a logical heading hierarchy (e.g., preceding<h1>or<h2>) to assist screen readers and maintain semantic correctness.
51-57: Provide a descriptivearia-labelledby.
Instead of"demo-controlled-radio-buttons-group", consider using a more descriptive label ID to improve accessibility and clarity for screen readers.-aria-labelledby="demo-controlled-radio-buttons-group" +aria-labelledby="upsell-selection-radio-buttons-group"
58-65: Consider i18n strategy for the label text.
“‘Yes, count on me!’” works well for informal contexts, but if internationalization is a requirement, externalize these strings to a localization file.components/domains/registerV3.tsx (2)
58-72: Consider validating domain inputs
This conditional block for the SearchBar and UserInfoForm is clear. However, there’s no apparent validation or feedback mechanism if the domain is invalid or left empty. Consider adding some basic validation or an error message to enhance user experience.
73-89: Consolidate multi-step registration logic
The rendering for steps 2 and 3 looks correct. As the registration flow scales (e.g., new steps or variants), repeating multiple conditionals might introduce complexity. Centralizing the logic (e.g., using a step-based map or a wizard-like component) could reduce future maintenance complexity.components/domains/steps/registerSteps.tsx (3)
50-60: Group label and icon for accessibility
Bundling the label withContactCardIconhelps visual grouping, but consider addingaria-labelor descriptive text for screen readers.
75-88: PFP step logic
The PFP step is rendered only ifshowPfpis true, which depends on whether the user has NFTs. This is a clean approach, but ensure that skipping this step (when the user has no NFTs) won’t cause issues in subsequent steps.
89-92: Optional refactor for icon code reuse
Both theContactCardIconand thisPfpIconshare a similar structure. Consider extracting the repeated logic (icon, text, and checkmark) into a reusable smaller component to keep your code DRY.styles/components/textField.module.css (1)
27-31: Provide a fallback for custom font
Switching to"QuickZap"from the default fonts can enhance style consistency. Add a generic fallback font (e.g.,font-family: "QuickZap", sans-serif;) in case"QuickZap"fails to load.styles/components/upsellCard.module.css (2)
44-49: Reduced font size and line height
Lowering.catchfont size from 36px to 24px and line height from 42px to 32px can improve readability on smaller screens. Just confirm it still meets design guidelines for brand consistency.
53-57: Adjusting spacing and text color
Decreasing top margin and changing color tovar(--content-600, #8c8989)is likely part of a broader restyle. Make sure these changes align with the site’s overall responsive design, especially in “dark mode” or theme overrides.styles/components/registerV3.module.css (4)
3-3: Consider using relative units for container paddingWhile the layout changes look good, using fixed padding (
166px) might cause issues on very small screens, even with the media queries. Consider using relative units (e.g.,clamp()) for more fluid responsiveness..container { - padding: 0px 166px; + padding: 0px clamp(16px, 10vw, 166px); }Also applies to: 6-6, 12-12
161-165: Enhance checkout button alignment propertiesThe
checkoutButtonclass could benefit from additional alignment properties to ensure consistent centering across different screen sizes..checkoutButton { width: 100%; display: flex; justify-content: center; + align-items: center; + gap: 8px; }
340-344: Standardize breakpoints and improve mobile layoutThe current breakpoints seem arbitrary and some media queries have overlapping concerns. Consider:
- Using standard breakpoints (e.g., 1440px, 1024px, 768px, 640px)
- Consolidating overlapping media queries
- Ensuring the mobile steps layout maintains visual hierarchy
-@media (max-width: 1570px) { +@media (max-width: 1440px) { .container { - padding: 0px 120px; + padding: 0px clamp(32px, 8vw, 120px); } } @media screen and (max-width: 640px) { .progressSteps { width: 100%; flex-direction: row; height: 120px; align-items: flex-start; justify-content: space-around; + overflow-x: auto; + -webkit-overflow-scrolling: touch; + scrollbar-width: none; } + + .progressSteps::-webkit-scrollbar { + display: none; + } }Also applies to: 378-380, 392-406
6-6: Maintain consistency in color variable usageThere's inconsistent usage of color variables throughout the file. Some colors use CSS variables (e.g.,
var(--background)), while others use direct hex values (e.g.,#FFFFFF).Consider converting all color values to use CSS variables for better theme consistency and maintenance:
- background-color: #FFFFFF + background-color: var(--background-paper)Also applies to: 150-150, 255-255
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
public/register/grass.pngis excluded by!**/*.png
📒 Files selected for processing (8)
components/domains/registerV3.tsx(1 hunks)components/domains/steps/checkoutCard.tsx(2 hunks)components/domains/steps/registerSteps.tsx(3 hunks)components/domains/steps/upsellCard.tsx(2 hunks)styles/components/registerV3.module.css(6 hunks)styles/components/textField.module.css(1 hunks)styles/components/upsellCard.module.css(2 hunks)utils/discounts/evergreen.ts(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- utils/discounts/evergreen.ts
- components/domains/steps/checkoutCard.tsx
🔇 Additional comments (10)
components/domains/steps/upsellCard.tsx (3)
3-3: Import looks good.
It's appropriate to import these Material UI components here, and there's no immediate issue with this addition.
66-70: Maintain consistent tone across labels.
“‘No, thanks!’” is consistent with the overall friendly tone. If future alternatives are considered, ensure uniformity in style and messaging.
73-73: Divider usage is appropriate.
This helps visually separate the upsell section from subsequent content. Just confirm thatclassName="w-full"aligns with your overall layout conventions.components/domains/steps/registerSteps.tsx (4)
2-2: New imports look correct
ImportingImagefromnext/imageandDoneFilledIcondoes not introduce obvious issues. Ensure that the icon component and assets are properly licensed and loaded for production.Also applies to: 9-9
36-42: Check responsive behavior with the background image
The additionalstepsBackgroundcontainer and background image can enrich the UI. Verify it doesn’t overlap or distort under smaller viewports or different breakpoints.
61-64: Conditionally displaying DoneFilledIcon
The done icon is loosely tied tocurrentStep >= 1. Confirm that the logic accurately reflects the step’s completion state and doesn’t inadvertently appear for uncompleted steps.
99-123: Reevaluate “greater than” vs. “greater than or equal to”
UsingcurrentStep > 3for the checkout step may skip displaying the icon whencurrentStepexactly equals 3. Verify that this condition matches the desired user journey—does the checkout step start at 3 or 4?styles/components/upsellCard.module.css (3)
6-6: Ensure layout remains consistent when width is 100%
Replacing a fixed container width withwidth: 100%may cause layout shifts. Confirm that child elements or sibling components still display correctly, especially in narrow viewports.
21-21: Use of CSS variable for background
Usingvar(--background)is a solid approach for better theming, as long as the variable is defined globally. Ensure that there’s a fallback color if the variable isn’t available.
60-64: New .radioGroupContainer styles
Defining a dedicated container for radio inputs clarifies layout management. This pattern is good for modular design, and theborder-radius: 8px;is consistent with your card style. Keep an eye on uniform padding for narrower screens.
There was a problem hiding this comment.
Really good job! Well done :)
Just a few changes and we're good!
- There should be a cancel button on mobile (to redirect on the home page)
-
On desktop, these small separators do not look the same as on figma, and they should be hidden on mobile
-
The button here do not look like on figma (it should be squared)
- The text here should not be bold
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
styles/components/upsellCard.module.css (2)
6-11: Remove duplicatejustify-contentproperty.The
.containerclass hasjustify-content: space-between;declared twice (lines 3 and 8)..container { display: flex; flex-direction: row; align-items: center; justify-content: space-between; width: 100%; background-color: white; - justify-content: space-between; overflow: hidden; position: relative; z-index: 0; }
Line range hint
60-76: Use CSS variables for consistent theming.The new classes use hardcoded colors which may cause maintenance issues:
#45454533in.radioGroupContainer.radioGroupContainer { width: 100%; - border: 1px solid #45454533; + border: 1px solid var(--border-color, rgba(69, 69, 69, 0.2)); border-radius: 8px; padding: 8px 20px; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
components/domains/steps/checkoutCard.tsx(3 hunks)components/domains/steps/upsellCard.tsx(2 hunks)styles/components/button.module.css(2 hunks)styles/components/registerV3.module.css(6 hunks)styles/components/textField.module.css(1 hunks)styles/components/upsellCard.module.css(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- styles/components/textField.module.css
- components/domains/steps/upsellCard.tsx
- styles/components/registerV3.module.css
- components/domains/steps/checkoutCard.tsx
🔇 Additional comments (4)
styles/components/upsellCard.module.css (2)
44-58: LGTM! Typography improvements enhance readability.The typography changes are well-structured:
- Using CSS variables for colors improves theme consistency
- Reduced font sizes and adjusted line heights enhance readability
- Proper text hierarchy maintained between catch and description
123-127: LGTM! Well-structured responsive design.The media queries effectively handle responsive layout with appropriate breakpoints and clear behavior definitions.
styles/components/button.module.css (2)
16-16: LGTM! Significant visual change from pill to rounded rectangle shape.The change from a pill shape to a more modern rounded rectangle (8px) aligns well with contemporary UI design patterns while maintaining visual hierarchy.
101-101: Verify border-radius consistency across components.The focus ring border-radius correctly matches the button's radius. However, let's ensure this 8px border-radius is consistently applied across other components in the payment page redesign.
✅ Verification successful
Border radius consistency verified across components ✅
The 8px border-radius is consistently applied across standard components including buttons, cards, and containers. The varying border-radius values found in registration components (16px, 52px) appear to be intentional design decisions for specific UI elements.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for border-radius declarations in related CSS files echo "Checking border-radius consistency in CSS files..." rg -g "*.css" -g "*.scss" "border-radius:\s*(8px|500px)" --no-heading echo -e "\nChecking specifically payment and registration related files..." fd -e css -e scss . | grep -iE '(payment|register|checkout)' | xargs rg "border-radius" --no-headingLength of output: 2541
Script:
#!/bin/bash # Check for potential payment-related files and design system constants echo "Checking for payment-related components with different naming patterns..." fd -t f -e css -e scss . | xargs rg -l "(card|billing|invoice|subscription|pay)" echo -e "\nChecking for design system or theme files..." fd -t f -e css -e scss -e js -e ts . | grep -iE '(theme|design-system|constants|variables|tokens)' | xargs catLength of output: 8161
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
styles/components/upsellCard.module.css (1)
59-63: Consider enhancing radio group focus states.While the container styling looks good, consider adding focus states for better accessibility:
.radioGroupContainer { width: 100%; border: 1px solid #45454533; border-radius: 8px; padding: 8px 20px; + position: relative; } +.radioGroupContainer:focus-within { + outline: 2px solid var(--primary); + outline-offset: 2px; +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
styles/components/upsellCard.module.css(4 hunks)
🔇 Additional comments (4)
styles/components/upsellCard.module.css (4)
6-6: Great improvements to container responsiveness!The changes to make the container width flexible and use CSS variables for theming are good practices that improve maintainability and responsiveness.
Also applies to: 20-20
43-48: Typography improvements enhance readability!The adjustments to font sizes, line heights, and spacing create a better visual hierarchy. Good use of CSS variables with fallback colors.
Also applies to: 52-57
72-75: Clean implementation of the divider!Good use of CSS variables for theming consistency.
122-126: Good responsive design consideration!Hiding the divider on mobile screens helps maintain a clean layout on smaller devices.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
components/UI/navbar.tsx (1)
Line range hint
88-98: Consider adding timeout and max retries for wallet connection.The current implementation recursively retries connection on failure without limits, which could lead to infinite retries if there's a persistent issue. This might impact user experience and system resources.
- const connectWallet = async (connector: Connector) => { + const connectWallet = async ( + connector: Connector, + retryCount: number = 0, + maxRetries: number = 3 + ) => { try { - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - await connectAsync({ connector }); + await connectAsync({ connector: connector as Connector }); localStorage.setItem("SID-connectedWallet", connector.id); localStorage.setItem("SID-lastUsedConnector", connector.id); } catch (e) { // Restart the connection if there is an error except if the user has rejected the connection console.error(e); const error = e as Error; - if (error.name !== "UserRejectedRequestError") connectWallet(connector); + if (error.name !== "UserRejectedRequestError" && retryCount < maxRetries) { + await new Promise(resolve => setTimeout(resolve, 1000)); // Add delay between retries + return connectWallet(connector, retryCount + 1, maxRetries); + } + throw error; // Re-throw if max retries reached or user rejected } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/UI/navbar.tsx(1 hunks)
🔇 Additional comments (1)
components/UI/navbar.tsx (1)
Line range hint
88-98: Enhance error handling with user feedback.The current implementation silently retries on errors, which might confuse users about the connection status. Consider adding user feedback for connection attempts and specific error messages.
Let's verify if there's a toast or notification system already in place:
✅ Verification successful
Use existing notification system for wallet connection feedback
The codebase already has a robust notification system using MUI's Alert component. Instead of silently retrying, use the existing
useNotificationManagerhook to display connection status:const { addTransaction } = useNotificationManager(); try { await connectAsync({ connector }); localStorage.setItem("SID-connectedWallet", connector.id); localStorage.setItem("SID-lastUsedConnector", connector.id); } catch (e) { const error = e as Error; if (error.name !== "UserRejectedRequestError") { addTransaction({ type: NotificationType.TRANSACTION, severity: "error", message: "Failed to connect wallet. Retrying...", }); connectWallet(connector); } }🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for toast/notification implementations rg -i "toast|notification|alert" --type ts --type tsxLength of output: 83
Script:
#!/bin/bash # First, check package.json for UI dependencies echo "Checking package.json for UI libraries:" cat package.json | grep -i "toast\|notification\|alert" echo -e "\nSearching for toast/notification patterns:" # Search in both .ts and .tsx files using find and grep find . -type f \( -name "*.ts" -o -name "*.tsx" \) -exec grep -i "toast\|notification\|alert" {} \;Length of output: 41475
There was a problem hiding this comment.
On mobile (and sometimes desktop too) I can see a shadow bellow the buttons, please remove it
Also, when the purchase button is bigger than usual (e.g. when I don't have enough eth) the text is broken. In that case, if there is not enough space, wrap the buttons (but if there is space because the text is smaller, do not wrap)
|
After that we're good I think! gg |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
styles/components/upsellCard.module.css (1)
59-63: Consider adding hover and focus states.The radio group container could benefit from interactive states for better user experience.
.radioGroupContainer { width: 100%; border: 1px solid #45454533; border-radius: 8px; padding: 8px 20px; + transition: border-color 0.2s ease; + } + + .radioGroupContainer:hover, + .radioGroupContainer:focus-within { + border-color: var(--primary); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
styles/components/evmModalMessage.module.css(1 hunks)styles/components/icons.module.css(1 hunks)styles/components/upsellCard.module.css(4 hunks)styles/discount.module.css(1 hunks)styles/domain.module.css(1 hunks)styles/globals.css(3 hunks)styles/verify.module.css(1 hunks)
🔇 Additional comments (11)
styles/verify.module.css (1)
13-13: LGTM! Good use of CSS variables.The removal of the fallback color in favor of using only
var(--content-600)improves maintainability by centralizing color definitions.styles/domain.module.css (1)
28-28: LGTM! Consistent with design system.The color change maintains consistency with other components using the same CSS variable.
styles/components/evmModalMessage.module.css (1)
78-78: LGTM! Good conversion to CSS variable.Converting the hardcoded color
#8c8989tovar(--content-600)improves theme consistency and maintainability.styles/components/icons.module.css (1)
105-105: LGTM! Consistent variable usage.The change aligns with the project-wide effort to standardize color variables.
styles/components/upsellCard.module.css (3)
6-11: LGTM! Improved responsive design.Making the container width flexible with
width: 100%improves the responsive behavior.
20-20: LGTM! Consistent theme usage.Using
var(--background)aligns with the project's CSS variable standardization.
122-126: LGTM! Good mobile optimization.The addition of a 640px breakpoint for hiding the divider improves the mobile layout.
styles/globals.css (3)
34-34: LGTM: New color variable addition for content text.The new
--content-600variable provides a consistent color scheme for content text across the application.
28-28: Maintain consistent color value casing.Good attention to detail in maintaining lowercase hexadecimal color values throughout the file.
Also applies to: 31-31, 83-83
Line range hint
62-66: LGTM: Updated description styling.The description class now uses the centralized
--content-600variable, promoting consistency across the application.styles/discount.module.css (1)
Line range hint
42-47: LGTM: Consistent color variable usage.The
.descriptionclass now uses the centralized--content-600variable without fallback, which is safe since the variable is defined inglobals.css.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
styles/components/registerV3.module.css (4)
1-13: Consider adjusting container padding for better responsiveness.The container's fixed width (1084px) combined with large padding (166px) could cause horizontal scrolling on screens between 1084px and 1416px wide.
.container { display: flex; flex-direction: column; width: 1084px; max-width: 100%; background-color: var(--background); justify-content: space-between; border-radius: 1rem; overflow: hidden; position: relative; z-index: 0; - padding: 0px 166px; + padding: 0px min(166px, 15%); }
27-62: Standardize CSS units for better maintainability.The typography styles mix different units (rem, px) for font sizes and line heights. Consider standardizing to rem for better scalability and maintenance.
.price { color: rgb(31 41 55); - font-size: 1.25rem /* 20px */; - line-height: 1.5rem /* 24px */; + font-size: 1.25rem; + line-height: 1.2; font-style: normal; font-weight: 700; white-space: nowrap; } .legend { color: #6d696a; - font-size: 14px; - line-height: 20px; /* 142.857% */ + font-size: 0.875rem; + line-height: 1.43; font-style: normal; font-weight: 500; }
251-261: Use CSS variables for consistent color management.The
.stepsContaineruses hardcoded colors. Consider using CSS variables for better theme management and consistency..stepsContainer { min-width: 220px; display: flex; position: relative; flex-direction: column; justify-content: space-between; - border: 1px solid #4545451A; + border: 1px solid var(--content-100); border-radius: 16px; box-shadow: 0px 2px 30px 0px rgba(0, 0, 0, 0.06); - background-color: #FFFFFF + background-color: var(--background-paper); }
326-343: Enhance interactive feedback with transitions.The gas method buttons lack transition effects, which could make state changes feel abrupt. Also, consider using CSS variables for consistent colors.
.gasMethod, .gasMethodSelected { border: 1px solid #cdcccc; border-radius: 8px; padding: 0.5rem 1rem; + transition: all 0.2s ease; } .gasMethod:disabled { - color: #666666; - background-color: #cdcccc; + color: var(--content-400); + background-color: var(--content-200); cursor: not-allowed; } .gasMethodSelected { background-color: var(--primary); color: white; border: 1px solid white; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
styles/components/button.module.css(0 hunks)styles/components/registerV3.module.css(1 hunks)styles/components/textField.module.css(1 hunks)
💤 Files with no reviewable changes (1)
- styles/components/button.module.css
🚧 Files skipped from review as they are similar to previous changes (1)
- styles/components/textField.module.css
🔇 Additional comments (1)
styles/components/registerV3.module.css (1)
281-291: Add hover and focus states for interactive steps.The
progressStepclass is missing hover and focus states, which are important for accessibility and user interaction feedback.
Close #954
Summary by CodeRabbit
Release Notes
New Features
UI/UX Improvements
Text Updates
Minor Enhancements