Fix: Reviewed and fixed Refont of Confirmation page after enabling Subscription#1179
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. |
WalkthroughThe changes update the confirmation page's design and typography, particularly after subscription activation. Adjustments include modifying CSS classes and layout, refining icon sizes, introducing a new font, and updating the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SubscriptionConfirmationPage
participant CopyIcon
participant DoneFilledIcon
User->>SubscriptionConfirmationPage: Visit confirmation page
SubscriptionConfirmationPage->>CopyIcon: Render referral link copy icon (width 20px)
SubscriptionConfirmationPage->>DoneFilledIcon: Render done icon
SubscriptionConfirmationPage->>SubscriptionConfirmationPage: Apply updated CSS classes and layout
User->>SubscriptionConfirmationPage: Interact with referral/copy features
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (6)
styles/globals.css (1)
124-126: Reuse Tailwind utilities instead of another custom helper class
.strongerduplicates the semantics offont-extrabold(font-weight 800).
Unless you specifically need the Poppins-ExtraBold face over other weights, consider:- .stronger { - font-family: "Poppins-ExtraBold"; - } + /* remove class and switch markup back to Tailwind's `font-extrabold` */Keeps global CSS lighter and avoids an extra network request for a weight you might not really use elsewhere.
pages/subscriptionConfirmation.tsx (1)
41-46: Fix wording + semantics
Small copy/HTML nits:
- “Earn up to $8 per friend” (singular).
- Use semantic
<p>instead of bare<div>for accessibility / line-spacing.- <div>Refer your friends to Starknet ID and earn crypto ! </div> - <div className="stronger"> - Earn up to 8$ per friends with your referral link below. - </div> + <p>Refer your friends to Starknet ID and earn crypto!</p> + <p className="stronger"> + Earn up to $8 per friend with your referral link below. + </p>styles/components/confirmation.module.css (4)
8-14: Hard-coded spacing values might clash with the design system
gap: 10px&padding-top: 55pxare “magic numbers”.
Prefer using CSS variables (e.g.var(--spacing-md)) or Tailwind spacing utilities to keep spacing consistent across pages.
39-49:copyAddr– improve tappable area & selection UX
Nice refactor, but two quick wins:
- Add
user-select: allso users can still select the text if click-to-copy fails.- Increase touch target to at least 44 px height for WCAG compliance on mobile.
.copyAddr { @@ display: flex; align-items: center; + user-select: all; @@ - margin-bottom: 40px; + margin-bottom: 40px; + min-height: 44px; /* mobile touch target */ }
51-56: Consider removing.contentand rely on global typography
The new.contentduplicates global font/line-height rules; if only used once, inline Tailwind classes (leading-7 text-base py-2) keep the CSS leaner.
86-87: Nit: hex with alpha – stick to lowercase
#4545451Ais valid but the mix of uppercase hex digits breaks some linters; prefer#4545451a.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
components/UI/iconsComponents/icons/copyIcon.tsx(1 hunks)pages/subscriptionConfirmation.tsx(2 hunks)styles/components/confirmation.module.css(4 hunks)styles/components/navbar.module.css(1 hunks)styles/globals.css(2 hunks)
🔇 Additional comments (4)
components/UI/iconsComponents/icons/copyIcon.tsx (1)
8-8: Confirm the icon still renders correctly with the smaller viewBox
Reducing theviewBoxfrom24to20without regenerating the path data can introduce clipping if any coordinate >= 20.
The current path tops out at18.83, so it should fit, but please sanity-check in all contexts (dark/light themes & retina).
If any clipping is observed, keep the original24viewBox and down-scale viawidth/heightinstead.styles/components/navbar.module.css (1)
66-67: Potential double vertical padding
height: 12vh+ newpadding-top: 15pxcan over-shift the logo/menu vertically (especially on small screens).
Check visual alignment against the existing media-queries; you may need to add:.navbarContainer { box-sizing: border-box; /* ensure the padding doesn’t grow the element past 12vh */ }or simply move the padding into the breakpoint-specific blocks where the navbar height already changes.
pages/subscriptionConfirmation.tsx (1)
60-66: Keep icon sizes consistent
CopyIconis now20pxwhile the successDoneFilledIconremains25px; the result is a jarring size jump after copy.
Either:- <DoneFilledIcon width="25" ... + <DoneFilledIcon width="20" ...or bump
CopyIconback to 24/25 to match.styles/components/confirmation.module.css (1)
74-78: Absolute positioning may overlap on very tall screens
The balloon is now positioned attop: 25%; left: 20%. On ultra-wide / tall screens it might overlap the main copy.
Consider converting toem/remoffsets or limiting withmax-widthin a media query.
| @font-face { | ||
| font-family: "Poppins-ExtraBold"; | ||
| src: url(../public/fonts/poppins/Poppins-ExtraBold.ttf); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ship web-optimised font formats as well
Only ttf is declared; browsers will download the full-fat file (~100 KB-plus) even on mobile.
Add at least a woff2 source for modern browsers and keep ttf as a fallback:
@font-face {
font-family: "Poppins-ExtraBold";
- src: url(../public/fonts/poppins/Poppins-ExtraBold.ttf);
+ src: url(../public/fonts/poppins/Poppins-ExtraBold.woff2) format("woff2"),
+ url(../public/fonts/poppins/Poppins-ExtraBold.ttf) format("truetype");
}📝 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.
| @font-face { | |
| font-family: "Poppins-ExtraBold"; | |
| src: url(../public/fonts/poppins/Poppins-ExtraBold.ttf); | |
| } | |
| @font-face { | |
| font-family: "Poppins-ExtraBold"; | |
| src: url(../public/fonts/poppins/Poppins-ExtraBold.woff2) format("woff2"), | |
| url(../public/fonts/poppins/Poppins-ExtraBold.ttf) format("truetype"); | |
| } |
🤖 Prompt for AI Agents
In styles/globals.css around lines 18 to 21, the @font-face rule only includes a
TTF font source, which is large and not optimized for web use. Add a WOFF2 font
source before the TTF source to provide a smaller, web-optimized format for
modern browsers while keeping TTF as a fallback. Update the src property to
include both formats with proper format declarations.
Marchand-Nicolas
left a comment
Marchand-Nicolas
left a comment
There was a problem hiding this comment.
Well done! It's looks really better now! Lgtm :)
2 participants
Summary by CodeRabbit