Fix: ens domain page responsiveness #1035
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@kayceeDev is attempting to deploy a commit to the LFG Labs Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis pull request updates the ENS domain confirmation page by revising its UI layout. The Changes
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: 2
🧹 Nitpick comments (2)
pages/evmConfirmation.tsx (1)
35-44: Consider improving text readability.While the description content is informative, consider breaking the long text into smaller paragraphs for better readability on mobile devices.
<div className={styles.description}> - Your EVM address has been successfully linked to{" "} - <span className={styles.strong}>{domain}.snid.eth</span>. All EVM - records in ENS now reflect this address. Additionally, we've - updated your profile with your{" "} - <span className={styles.strong}>PFP, GitHub, Twitter</span>, and - other verified Starknet ID information + <p> + Your EVM address has been successfully linked to{" "} + <span className={styles.strong}>{domain}.snid.eth</span>. + </p> + <p> + All EVM records in ENS now reflect this address. + </p> + <p> + Additionally, we've updated your profile with your{" "} + <span className={styles.strong}>PFP, GitHub, Twitter</span>, and + other verified Starknet ID information + </p> </div>styles/components/evmConfirmation.module.css (1)
125-174: Enhance media query breakpoints.The current media queries might not cover all device sizes effectively. Consider:
- Adding a breakpoint for smaller mobile devices (< 768px)
- Using relative units instead of fixed pixel values for positioning
- Testing layout on different device orientations
+@media (max-width: 768px) { + .container { + margin-top: 40px; + } + .title { + font-size: 24px; + line-height: 36px; + } + .description { + font-size: 14px; + line-height: 24px; + } +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
public/ens/baloon.svgis excluded by!**/*.svg
📒 Files selected for processing (2)
pages/evmConfirmation.tsx(2 hunks)styles/components/evmConfirmation.module.css(4 hunks)
🔇 Additional comments (4)
pages/evmConfirmation.tsx (2)
26-34: LGTM! Clean title container implementation.The new title container with the balloon icon provides a better visual hierarchy and user experience.
49-56: LGTM! Improved button structure.The new button structure with the metamask icon provides better visual feedback and alignment.
styles/components/evmConfirmation.module.css (2)
84-87: LGTM! Clean relative positioning.The relative positioning for title container and domain button provides good layout control.
74-82: Consider z-index stacking context.The balloon element has a negative z-index which might cause issues with overlapping elements. Consider reviewing the z-index hierarchy.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for z-index usage in the codebase to verify stacking context rg "z-index" --type cssLength of output: 2683
Stacking Context Verification for .balloon in evmConfirmation.module.css
• The grep results confirm that negative z-index values (such as –1) are used in this file—and in several other CSS files throughout the codebase—likely as an intentional design decision to layer background elements behind primary content.
• While the negative z-index on the .balloon element affects its stacking order, similar patterns elsewhere in the project suggest that the current usage is consistent.
• It is advisable to double-check that the parent element and overall stacking context are set up correctly so that the .balloon appears as intended relative to overlapping content.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
pages/evmConfirmation.tsx (2)
26-34: Add aria-label to titleContainer for better accessibility.The titleContainer introduces a decorative balloon icon but lacks proper accessibility attributes.
- <div className={styles.titleContainer}> + <div className={styles.titleContainer} aria-label="Welcome message with balloon decoration"> <div className={styles.balloon}> - <img alt="floating ballon icon" src="/ens/baloon.svg" /> + <img alt="" role="presentation" src="/ens/baloon.svg" /> </div>
49-56: Consider keyboard navigation for the back button.The back button should be keyboard accessible for better user experience.
- <div onClick={redirect} className={styles.backButton}> + <button + onClick={redirect} + className={styles.backButton} + type="button"> Back to your domain - </div> + </button>styles/components/evmConfirmation.module.css (1)
134-178: Review media query breakpoints.The media queries could be more comprehensive to handle various screen sizes.
Consider adding additional breakpoints for better responsive design:
+@media (max-width: 768px) { + .title { + font-size: 24px; + line-height: 36px; + } + .description { + font-size: 14px; + line-height: 24px; + } +} + +@media (max-width: 480px) { + .coconutLeft, + .coconutRight { + display: none; + } +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
public/ens/baloon.svgis excluded by!**/*.svg
📒 Files selected for processing (2)
pages/evmConfirmation.tsx(2 hunks)styles/components/evmConfirmation.module.css(5 hunks)
🔇 Additional comments (5)
pages/evmConfirmation.tsx (1)
36-43: LGTM! Clear and informative description.The description text effectively communicates the successful linking of EVM address and profile updates.
styles/components/evmConfirmation.module.css (4)
1-3: LGTM! Background color addition.The background color provides good contrast for content visibility.
61-75: Improve coconut elements positioning.The fixed positioning with negative offsets might cause layout issues on different screen sizes.
.coconutLeft { - position: fixed; + position: absolute; left: -10px; bottom: 0; width: 390px; + max-width: 30vw; overflow: hidden; } .coconutRight { - position: fixed; + position: absolute; right: -95px; bottom: 0; width: 390px; + max-width: 30vw; overflow: hidden; }
119-132: Improve Metamask icon positioning.The absolute positioning with fixed pixel values might cause layout issues.
.metamask { position: absolute; bottom: 40px; - left: 500px; + left: 50%; + transform: translateX(-50%); height: 205px; width: 130px; overflow: hidden; } @media (max-width: 1300px) { .metamask { - left: 400px; + left: 45%; } }
77-85: Consider z-index stacking context.The balloon element uses negative z-index which might cause issues with overlapping elements.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for z-index usage in the codebase rg "z-index:" --type cssLength of output: 2684
Z-index stacking context usage appears intentional and consistent.
- The search results show that negative z-index values (like -1) are used elsewhere in the codebase (including in evmConfirmation.module.css alongside positive values), suggesting a deliberate stacking order.
- While negative z-index values can sometimes lead to overlapping issues if the parent stacking context isn’t properly managed, the current implementation seems aligned with patterns found in other modules.
No immediate action is required unless you observe visual or interaction anomalies.
There was a problem hiding this comment.
It looks perfect! Just one optimization: Instead of importing the balloon asset, you can use the one in public/visuals/balloon.webp.
Also, could you try (if possible) downloading better quality coconut trees please (On figma when exporting the asset you can select up to x4 quality) ? If so you'll need to convert the png export to webp using https://image.online-convert.com/convert/png-to-webp
|
Alright you will get revised changes soon. Thank you |
|
@Marchand-Nicolas fixed |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
styles/components/evmConfirmation.module.css (1)
125-132: Metamask Icon Positioning ConsiderationThe
.metamaskclass is now set with absolute positioning (with properties such asbottom: 40px; left: 500px; height: 205px; width: 130px;). Given previous feedback on metamask icon placement, consider whether fixed pixel values are optimal. A more fluid approach might use centering techniques. For example:- left: 500px; + left: 50%; + transform: translateX(-50%);This change could improve responsiveness across various screen sizes.
🧹 Nitpick comments (7)
styles/components/evmConfirmation.module.css (7)
61-67: Fixed Positioning for .coconutLeftThe
.coconutLeftclass uses fixed positioning with explicit pixel values (left: -10px; bottom: 0; width: 330px;). Fixed values can sometimes lead to inconsistent behavior across devices. Although later media queries adjust these values, please verify that these positions render well on a variety of screen sizes.
69-77: Fixed Positioning for .coconutRightSimilarly, the
.coconutRightclass now uses fixed positioning with settings likeright: 0; bottom: 0; width: 400px;along withdisplay: flex;. Ensure that these choices align with your design intentions across devices. Testing on different screen sizes is advised.
79-81: Responsive Image Sizing in Coconut ContainersThe added rule for
.coconutLeft > img, .coconutRight > imgsets the image width to 100%, which helps maintain responsiveness. Consider whether adding a rule likeheight: auto;might further preserve the aspect ratio if image distortion is a concern.
144-148: Media Query Tweaks for Metamask PositioningIn the media query for max-width 1300px, the
.metamaskicon’s horizontal position is adjusted toleft: 400px. While this tweak helps achieve better alignment on moderately sized screens, consider whether a percentage-based approach might offer superior fluidity.
159-163: Responsive Repositioning of .coconutLeft on Smaller ScreensWithin the media query for screens with max-width 1084px, the
.coconutLeftclass is repositioned toleft: -170px; bottom: -110px;. This adjustment seems targeted to accommodate smaller viewports. Ensure that these negative offsets do not hide essential decorative content and that the overall layout remains balanced.
164-167: Responsive Repositioning of .coconutRight on Smaller ScreensThe
.coconutRightadjustments (right: -170px; bottom: -40px;) in the same media query aim for similar responsiveness. Confirm that these changes integrate well with the rest of the design and that the coconut imagery maintains its intended visual impact.
169-171: Balloon Position Adjustment for Small ViewportsChanging the
.balloonclass’stopproperty to-100pxin the media query may help maintain its visual layering on smaller screens. Verify that this revised placement does not conflict with other elements in the layout.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
pages/evmConfirmation.tsx(2 hunks)styles/components/evmConfirmation.module.css(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pages/evmConfirmation.tsx
🔇 Additional comments (5)
styles/components/evmConfirmation.module.css (5)
1-3: .main Class IntroductionThe new
.mainclass sets a clean background color for the page. This is a good base for your layout changes.
83-91: New .balloon Decorative ElementThe newly introduced
.balloonclass uses absolute positioning with negative offsets and a z-index of -1. This appears designed for a decorative background effect. Make sure the chosen offsets (e.g.,top: -65px) and dimensions work harmoniously with other page elements on all devices, especially under differing viewport conditions.
93-97: Relative Positioning for .titleContainer and .domainBtnAssigning
position: relative;to both.titleContainerand.domainBtnis a sound decision for managing the positioning of their absolutely positioned child elements.
134-142: Media Query Adjustments for Coconut WidthsThe media query for screens with a max-width of 1500px adjusts the coconut elements’ widths (
.coconutLeftfrom 330px to 280px and.coconutRightfrom 400px to 330px). This is a positive step toward ensuring the design scales on smaller displays. Please verify these breakpoints against the targeted devices.
191-193: Hiding Metamask Icon on Small ScreensThe media query for max-width 1084px sets
.metamasktodisplay: none;. If the Metamask icon represents a critical action for desktop users, please ensure that alternative affordances (or a different layout) are provided on smaller devices. Otherwise, hiding it might affect user experience.
|
@Marchand-Nicolas Please take a look at the changes |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
styles/components/evmConfirmation.module.css (1)
134-237: LGTM! Well-structured responsive design.Good implementation of breakpoints with comprehensive adjustments for different screen sizes. The height-based media query is a particularly good addition for handling shorter viewports.
Consider using CSS custom properties for repeated values to improve maintainability:
+:root { + --coconut-small-width: 200px; + --balloon-base-width: 115px; +} .coconutLeft { - width: 200px; + width: var(--coconut-small-width); } .coconutRight { - width: 200px; + width: var(--coconut-small-width); } .balloon { - width: 115px; + width: var(--balloon-base-width); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
styles/components/evmConfirmation.module.css(5 hunks)
🔇 Additional comments (4)
styles/components/evmConfirmation.module.css (4)
1-3: LGTM!Clean addition of background color for the main container.
61-81: Consider using relative positioning for coconut elements.The fixed positioning with hardcoded values persists from previous feedback. This approach may still cause layout issues on different screen sizes.
Consider this alternative approach:
.coconutLeft { - position: fixed; + position: absolute; left: -10px; bottom: 0; - width: 330px; + width: min(330px, 30vw); overflow: hidden; } .coconutRight { - position: fixed; + position: absolute; right: 0; bottom: 0; - width: 400px; + width: min(400px, 35vw); display: flex; overflow: hidden; }
83-96: LGTM!Good use of relative positioning on containers to establish positioning context for the absolutely positioned balloon element.
125-132: Consider using relative units for metamask icon positioning.While changing to absolute positioning is an improvement, the hardcoded pixel values may still cause layout issues.
Consider this alternative approach:
.metamask { position: absolute; bottom: 40px; - left: 500px; + left: 50%; + transform: translateX(-50%); height: 205px; - width: 130px; + width: min(130px, 15vw); overflow: hidden; }
Summary
This will fix the design inconsistency in the ens domain page
fixes #959
Screenshots
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Style