fix: perfect dashboard auth overlay positioning on mobile and desktop - #108
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughChangesLanding and page layout
Dashboard presentation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Hero
participant TypingEffect
participant AnimatePresence
participant AIMockup
Hero->>TypingEffect: start prompt interval
TypingEffect->>Hero: update typedText and messageSent
Hero->>AnimatePresence: render submitted chat state
AnimatePresence->>AIMockup: animate user and assistant content
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
🎉 Congratulations @nitinmohan18!Thank you for contributing to HyperLearningTech. Your pull request has been successfully merged into main. 📦 Merge Summary
🚀 Keep Contributing
Thank you for helping make HyperLearningTech better. Happy Coding! 🚀 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
app/dashboard/page.tsx (2)
60-60: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove redundant
w-fullclass.The
inset-x-0class applies bothleft: 0andright: 0, which makes the element implicitly span the full width of its relative parent. Addingw-fullis redundant.♻️ Proposed refactor
- <div className="absolute inset-x-0 top-0 z-50 h-px w-full bg-gradient-to-r from-transparent via-blue-500/30 to-transparent" /> + <div className="absolute inset-x-0 top-0 z-50 h-px bg-gradient-to-r from-transparent via-blue-500/30 to-transparent" />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/dashboard/page.tsx` at line 60, Remove the redundant w-full utility from the div’s className, while preserving the existing inset-x-0 positioning and all other styling classes.
60-60: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove redundant
w-fullclass.The
inset-x-0class applies bothleft: 0andright: 0, which makes the element implicitly span the full width of its relative parent. Addingw-fullis redundant.♻️ Proposed refactor
- <div className="absolute inset-x-0 top-0 z-50 h-px w-full bg-gradient-to-r from-transparent via-blue-500/30 to-transparent" /> + <div className="absolute inset-x-0 top-0 z-50 h-px bg-gradient-to-r from-transparent via-blue-500/30 to-transparent" />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/dashboard/page.tsx` at line 60, Remove the redundant w-full utility from the div’s className, keeping inset-x-0 and all other positioning and styling classes unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@features/landing/hero.tsx`:
- Around line 56-73: Update the useEffect timer lifecycle so its cleanup tracks
and clears the outer timer, the typing interval, and the inner 200ms timeout. Do
not return cleanup from inside the setTimeout callback; retain timer handles in
the effect scope and clear them all on unmount, including during StrictMode
remounts.
- Line 113: Replace each affected min-height expression with valid Tailwind calc
syntax using calc(100vh_-_72px), and make the navbar offset responsive by using
the h-16 value below md and the 72px value at md and above. Apply this
consistently in features/landing/hero.tsx:113,
app/(public)/cgpa-calculator/page.tsx:73, app/(public)/creators/page.tsx:85,
components/contact/ContactSection.tsx:41, features/landing/ai-demo.tsx:120,
features/landing/cgpa-feature.tsx:17, features/landing/faq.tsx:41,
features/landing/features.tsx:34, and features/landing/stats.tsx:86.
---
Nitpick comments:
In `@app/dashboard/page.tsx`:
- Line 60: Remove the redundant w-full utility from the div’s className, while
preserving the existing inset-x-0 positioning and all other styling classes.
- Line 60: Remove the redundant w-full utility from the div’s className, keeping
inset-x-0 and all other positioning and styling classes unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f649ac38-b973-47ae-9ef9-ac3845e9e7da
📒 Files selected for processing (11)
app/(public)/cgpa-calculator/page.tsxapp/(public)/creators/page.tsxapp/dashboard/page.tsxcomponents/contact/ContactSection.tsxcomponents/dashboard/auth-overlay.tsxfeatures/landing/ai-demo.tsxfeatures/landing/cgpa-feature.tsxfeatures/landing/faq.tsxfeatures/landing/features.tsxfeatures/landing/hero.tsxfeatures/landing/stats.tsx
| useEffect(() => { | ||
| let i = 0; | ||
| const timer = setTimeout(() => { | ||
| const interval = setInterval(() => { | ||
| setTypedText(fullText.slice(0, i + 1)); | ||
| i++; | ||
| if (i >= fullText.length) { | ||
| clearInterval(interval); | ||
| setTimeout(() => { | ||
| setMessageSent(true); | ||
| setTypedText(""); | ||
| }, 200); | ||
| } | ||
| }, 15); | ||
| return () => clearInterval(interval); | ||
| }, 400); | ||
| return () => clearTimeout(timer); | ||
| }, []); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Interval and inner timeout leak on unmount. The effect's returned cleanup only clears the outer timer. The return () => clearInterval(interval) on Line 70 is returned from the setTimeout callback and is discarded, so once typing starts, interval (and the 200ms inner setTimeout) are never cleared. On unmount mid-typing—or on React StrictMode's double mount in dev—the interval keeps firing setTypedText/setMessageSent on an unmounted component.
🔒 Proposed fix: track and clear all timers in the effect cleanup
useEffect(() => {
let i = 0;
- const timer = setTimeout(() => {
- const interval = setInterval(() => {
+ let interval: ReturnType<typeof setInterval>;
+ let doneTimeout: ReturnType<typeof setTimeout>;
+ const timer = setTimeout(() => {
+ interval = setInterval(() => {
setTypedText(fullText.slice(0, i + 1));
i++;
if (i >= fullText.length) {
clearInterval(interval);
- setTimeout(() => {
+ doneTimeout = setTimeout(() => {
setMessageSent(true);
setTypedText("");
}, 200);
}
}, 15);
- return () => clearInterval(interval);
}, 400);
- return () => clearTimeout(timer);
+ return () => {
+ clearTimeout(timer);
+ clearInterval(interval);
+ clearTimeout(doneTimeout);
+ };
}, []);📝 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.
| useEffect(() => { | |
| let i = 0; | |
| const timer = setTimeout(() => { | |
| const interval = setInterval(() => { | |
| setTypedText(fullText.slice(0, i + 1)); | |
| i++; | |
| if (i >= fullText.length) { | |
| clearInterval(interval); | |
| setTimeout(() => { | |
| setMessageSent(true); | |
| setTypedText(""); | |
| }, 200); | |
| } | |
| }, 15); | |
| return () => clearInterval(interval); | |
| }, 400); | |
| return () => clearTimeout(timer); | |
| }, []); | |
| useEffect(() => { | |
| let i = 0; | |
| let interval: ReturnType<typeof setInterval>; | |
| let doneTimeout: ReturnType<typeof setTimeout>; | |
| const timer = setTimeout(() => { | |
| interval = setInterval(() => { | |
| setTypedText(fullText.slice(0, i + 1)); | |
| i++; | |
| if (i >= fullText.length) { | |
| clearInterval(interval); | |
| doneTimeout = setTimeout(() => { | |
| setMessageSent(true); | |
| setTypedText(""); | |
| }, 200); | |
| } | |
| }, 15); | |
| }, 400); | |
| return () => { | |
| clearTimeout(timer); | |
| clearInterval(interval); | |
| clearTimeout(doneTimeout); | |
| }; | |
| }, []); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@features/landing/hero.tsx` around lines 56 - 73, Update the useEffect timer
lifecycle so its cleanup tracks and clears the outer timer, the typing interval,
and the inner 200ms timeout. Do not return cleanup from inside the setTimeout
callback; retain timer handles in the effect scope and clear them all on
unmount, including during StrictMode remounts.
| <section | ||
| id="Home" | ||
| className="relative overflow-hidden scroll-mt-24 border-b border-border dark:bg-transparent" | ||
| className="relative overflow-hidden scroll-mt-24 border-b border-border dark:bg-transparent min-h-[calc(100vh-72px)] flex items-center w-full" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
CSS calc requires whitespace around minus operator calc(100vh-72px) invalid
💡 Result:
Yes, the CSS calc function requires whitespace around the plus (+) and minus (-) operators to be valid [1][2][3]. Therefore, calc(100vh-72px) is invalid syntax and will be discarded by the browser [4][2]. The requirement exists because the minus sign (-) can also be part of a negative number or a CSS identifier (e.g., in a unit or a keyword) [5][2]. Without whitespace, the browser may misinterpret the expression—for example, it might parse calc(100vh-72px) as a single value rather than a subtraction operation [2][3]. By contrast, the multiplication (*) and division (/) operators do not require whitespace, although adding it for consistency is permitted and recommended [1][6][7]. To make your expression valid, ensure there is a space on both sides of the minus operator: calc(100vh - 72px) [2][7]
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Values/calc
- 2: https://stackoverflow.com/questions/34419813/why-must-a-or-be-surrounded-with-whitespace-from-within-the-calc-method
- 3: https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Values/calc-sum
- 4: https://meyerweb.com/eric/thoughts/2012/04/10/whitespace-in-css-calculations/
- 5: https://lists.w3.org/Archives/Public/www-style/2012May/0404.html
- 6: https://css-tricks.com/almanac/functions/c/calc/
- 7: https://developer.mozilla.org/en-US/docs/Web/CSS/Guides/Values_and_units/Using_math_functions
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== occurrences ==\n'
rg -n 'min-h-\[calc\(100vh-72px\)\]|md:h-\[72px\]|h-16|72px' \
features app components -g '!**/node_modules/**'
printf '\n== hero snippet ==\n'
sed -n '100,125p' features/landing/hero.tsx
printf '\n== contact snippet ==\n'
sed -n '1,70p' components/contact/ContactSection.tsx
printf '\n== header/nav snippet candidates ==\n'
rg -n 'md:h-\[72px\]|h-16|header|navbar' app features components -g '!**/node_modules/**'Repository: imuniqueshiv/HyperLearningTech
Length of output: 9864
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== file list for likely header/layout files ==\n'
git ls-files | rg '(^app/|^features/|^components/).*(header|nav|navbar|layout|hero|contact|faq|features|stats|ai-demo|cgpa|creators)'
printf '\n== maybe shared shell/header component ==\n'
rg -n 'md:h-\[72px\]|h-16|min-h-\[calc\(100vh-72px\)\]' app features components -g '!**/node_modules/**'Repository: imuniqueshiv/HyperLearningTech
Length of output: 4356
Use calc(100vh_-_72px) and make the offset responsive.
min-h-[calc(100vh-72px)]won’t parse as a validcalc()value; change each occurrence tomin-h-[calc(100vh_-_72px)].- The navbar is
h-16belowmdandmd:h-[72px], so a fixed72pxoffset only matches desktop.
📍 Affects 9 files
features/landing/hero.tsx#L113-L113(this comment)app/(public)/cgpa-calculator/page.tsx#L73-L73app/(public)/creators/page.tsx#L85-L85components/contact/ContactSection.tsx#L41-L41features/landing/ai-demo.tsx#L120-L120features/landing/cgpa-feature.tsx#L17-L17features/landing/faq.tsx#L41-L41features/landing/features.tsx#L34-L34features/landing/stats.tsx#L86-L86
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@features/landing/hero.tsx` at line 113, Replace each affected min-height
expression with valid Tailwind calc syntax using calc(100vh_-_72px), and make
the navbar offset responsive by using the h-16 value below md and the 72px value
at md and above. Apply this consistently in features/landing/hero.tsx:113,
app/(public)/cgpa-calculator/page.tsx:73, app/(public)/creators/page.tsx:85,
components/contact/ContactSection.tsx:41, features/landing/ai-demo.tsx:120,
features/landing/cgpa-feature.tsx:17, features/landing/faq.tsx:41,
features/landing/features.tsx:34, and features/landing/stats.tsx:86.
stickytrack, allowing the modal to smoothly scroll with the user without overlapping.Summary by CodeRabbit
New Features
Bug Fixes
Style