mobileFirstDesing changes - #11
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughRemoves the ChangesProject Card Modal Feature
Mobile Hamburger Navigation
Hero/AboutSection/CardComponents removal and responsive CSS polish
Sequence Diagram(s)sequenceDiagram
participant User
participant ProjectCardWithModal
participant ProjectCardComponent
participant ProjectModal
User->>ProjectCardComponent: clicks "Read More →"
ProjectCardComponent->>ProjectCardWithModal: onReadMore()
ProjectCardWithModal->>ProjectModal: renders (isOpen=true)
ProjectModal->>ProjectModal: lock scroll, record focused element
ProjectModal->>ProjectModal: auto-focus first focusable control
User->>ProjectModal: Escape / overlay click / close button
ProjectModal->>ProjectCardWithModal: onClose()
ProjectCardWithModal->>ProjectModal: unmount (isOpen=false)
ProjectModal->>ProjectModal: restore scroll + return focus
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: one or more packages not found in the registry. 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 |
khankadir193
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/Navbar.js (1)
101-107: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winClose the mobile menu when the resume link is activated.
This is the only action inside
.nav-linksthat does not callsetIsMenuOpen(false), so on small screens the panel stays open after the download/new tab is triggered. Add the same close behavior here to avoid leaving the current page occluded when the user comes back.Proposed fix
<a href="/cv/Kadir-CV.pdf" download="Kadir-CV.pdf" className="download-resume-btn" + onClick={() => setIsMenuOpen(false)} target="_blank" rel="noopener noreferrer" >🤖 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/Navbar.js` around lines 101 - 107, The resume link inside Navbar’s .nav-links does not close the mobile menu, unlike the other navigation actions. Update the resume anchor in Navbar so it also calls setIsMenuOpen(false) when activated, matching the behavior used by the other links and ensuring the menu closes on small screens.
🤖 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 `@app/components/ProjectCardWithModal.jsx`:
- Around line 9-10: The modal cleanup in ProjectCardWithModal currently resets
document.body.style.overflow to an empty string, which can overwrite an existing
inline overflow value. Update the modal open/close effect and its cleanup to
capture the previous body overflow value before setting it, then restore that
saved value on cleanup instead of hardcoding ''. Use the existing overlayRef and
previousFocusRef area to locate the modal lifecycle logic and apply the same
restoration when the modal closes.
- Around line 12-42: The modal focus handling in ProjectCardWithModal only
closes on Escape and does not trap Tab navigation, so keyboard focus can escape
behind the dialog. Update the existing handleKeyDown/useEffect logic to
intercept Tab and Shift+Tab, cycle focus among the dialog’s focusable elements
inside overlayRef.current, and keep focus contained until onClose runs. Use the
existing overlayRef, handleKeyDown, and previousFocusRef setup to implement the
trap and restore focus on cleanup.
In `@app/components/ProjectCardWithModal.module.css`:
- Around line 11-21: The stylesheet’s keyframe names in
ProjectCardWithModal.module.css use camelCase and violate the configured
keyframes-name-pattern rule, so rename the animation and matching `@keyframes`
identifiers to the required stylelint-compliant format. Update every reference
in the module, including the overlay fade-in usage and the other keyframes block
noted in the review, so the animation names stay consistent and lint passes
cleanly.
In `@app/globals.css`:
- Line 614: The `@keyframes` definition for navbarPulse violates the project’s
kebab-case Stylelint rule, so rename this keyframe in app/globals.css to a
kebab-case name and update every matching animation reference to use the same
new name. Make sure the change is applied consistently wherever the keyframe is
defined or consumed so the stylesheet still compiles and passes lint.
In `@app/sections/ContactSection.module.css`:
- Around line 274-276: The .infoText rule in ContactSection.module.css uses a
deprecated wrap property, so replace word-break: break-word with overflow-wrap:
anywhere to preserve the same mobile-friendly long-text wrapping behavior.
Update the existing .infoText selector only, keeping the rest of the contact
text styling unchanged.
---
Outside diff comments:
In `@app/Navbar.js`:
- Around line 101-107: The resume link inside Navbar’s .nav-links does not close
the mobile menu, unlike the other navigation actions. Update the resume anchor
in Navbar so it also calls setIsMenuOpen(false) when activated, matching the
behavior used by the other links and ensuring the menu closes on small screens.
🪄 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: 40a1db79-b109-4e3d-86f6-88780b8ab67a
📒 Files selected for processing (19)
app/About.jsapp/About.module.cssapp/CardComponents.jsapp/Footer.module.cssapp/Hero.jsapp/Hero.module.cssapp/HomeClient.jsxapp/Navbar.jsapp/SectionComponents.jsapp/SectionComponents.module.cssapp/components/ProjectCardWithModal.jsxapp/components/ProjectCardWithModal.module.cssapp/globals.cssapp/sections/AboutSection.jsapp/sections/ContactSection.module.cssapp/sections/EducationSection.module.cssapp/sections/ExperienceSection.module.cssapp/sections/ProjectsSection.jsapp/sections/SkillsSection.module.css
💤 Files with no reviewable changes (4)
- app/sections/AboutSection.js
- app/Hero.js
- app/CardComponents.js
- app/Hero.module.css
| const overlayRef = useRef(null); | ||
| const previousFocusRef = useRef(null); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Restore the previous body overflow value on cleanup.
Closing the modal always writes '' back to document.body.style.overflow. If some other state had already set a non-default inline overflow, this cleanup silently breaks it.
Proposed fix
function ProjectModal({ project, onClose }) {
const overlayRef = useRef(null);
const previousFocusRef = useRef(null);
+ const previousOverflowRef = useRef('');
useEffect(() => {
previousFocusRef.current = document.activeElement;
+ previousOverflowRef.current = document.body.style.overflow;
document.addEventListener('keydown', handleKeyDown);
document.body.style.overflow = 'hidden';
@@
return () => {
document.removeEventListener('keydown', handleKeyDown);
- document.body.style.overflow = '';
+ document.body.style.overflow = previousOverflowRef.current;
previousFocusRef.current?.focus();
};
}, [handleKeyDown]);Also applies to: 24-41
🤖 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/components/ProjectCardWithModal.jsx` around lines 9 - 10, The modal
cleanup in ProjectCardWithModal currently resets document.body.style.overflow to
an empty string, which can overwrite an existing inline overflow value. Update
the modal open/close effect and its cleanup to capture the previous body
overflow value before setting it, then restore that saved value on cleanup
instead of hardcoding ''. Use the existing overlayRef and previousFocusRef area
to locate the modal lifecycle logic and apply the same restoration when the
modal closes.
| const handleKeyDown = useCallback((e) => { | ||
| if (e.key === 'Escape') { | ||
| onClose(); | ||
| } | ||
| }, [onClose]); | ||
|
|
||
| const handleOverlayClick = useCallback((e) => { | ||
| if (e.target === overlayRef.current) { | ||
| onClose(); | ||
| } | ||
| }, [onClose]); | ||
|
|
||
| useEffect(() => { | ||
| previousFocusRef.current = document.activeElement; | ||
|
|
||
| document.addEventListener('keydown', handleKeyDown); | ||
| document.body.style.overflow = 'hidden'; | ||
|
|
||
| const focusable = overlayRef.current?.querySelector( | ||
| 'button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])' | ||
| ); | ||
| if (focusable) { | ||
| focusable.focus(); | ||
| } | ||
|
|
||
| return () => { | ||
| document.removeEventListener('keydown', handleKeyDown); | ||
| document.body.style.overflow = ''; | ||
| previousFocusRef.current?.focus(); | ||
| }; | ||
| }, [handleKeyDown]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep focus trapped inside the dialog.
This only handles Escape. Tab / Shift+Tab can still move focus to elements behind the modal, so keyboard users can leave the active dialog while it is open.
Proposed fix
const handleKeyDown = useCallback((e) => {
if (e.key === 'Escape') {
onClose();
+ return;
}
+
+ if (e.key === 'Tab' && overlayRef.current) {
+ const focusable = Array.from(
+ overlayRef.current.querySelectorAll(
+ 'button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])'
+ )
+ );
+ if (focusable.length === 0) return;
+
+ const first = focusable[0];
+ const last = focusable[focusable.length - 1];
+
+ if (e.shiftKey && document.activeElement === first) {
+ e.preventDefault();
+ last.focus();
+ } else if (!e.shiftKey && document.activeElement === last) {
+ e.preventDefault();
+ first.focus();
+ }
+ }
}, [onClose]);📝 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.
| const handleKeyDown = useCallback((e) => { | |
| if (e.key === 'Escape') { | |
| onClose(); | |
| } | |
| }, [onClose]); | |
| const handleOverlayClick = useCallback((e) => { | |
| if (e.target === overlayRef.current) { | |
| onClose(); | |
| } | |
| }, [onClose]); | |
| useEffect(() => { | |
| previousFocusRef.current = document.activeElement; | |
| document.addEventListener('keydown', handleKeyDown); | |
| document.body.style.overflow = 'hidden'; | |
| const focusable = overlayRef.current?.querySelector( | |
| 'button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])' | |
| ); | |
| if (focusable) { | |
| focusable.focus(); | |
| } | |
| return () => { | |
| document.removeEventListener('keydown', handleKeyDown); | |
| document.body.style.overflow = ''; | |
| previousFocusRef.current?.focus(); | |
| }; | |
| }, [handleKeyDown]); | |
| const handleKeyDown = useCallback((e) => { | |
| if (e.key === 'Escape') { | |
| onClose(); | |
| return; | |
| } | |
| if (e.key === 'Tab' && overlayRef.current) { | |
| const focusable = Array.from( | |
| overlayRef.current.querySelectorAll( | |
| 'button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])' | |
| ) | |
| ); | |
| if (focusable.length === 0) return; | |
| const first = focusable[0]; | |
| const last = focusable[focusable.length - 1]; | |
| if (e.shiftKey && document.activeElement === first) { | |
| e.preventDefault(); | |
| last.focus(); | |
| } else if (!e.shiftKey && document.activeElement === last) { | |
| e.preventDefault(); | |
| first.focus(); | |
| } | |
| } | |
| }, [onClose]); | |
| const handleOverlayClick = useCallback((e) => { | |
| if (e.target === overlayRef.current) { | |
| onClose(); | |
| } | |
| }, [onClose]); | |
| useEffect(() => { | |
| previousFocusRef.current = document.activeElement; | |
| document.addEventListener('keydown', handleKeyDown); | |
| document.body.style.overflow = 'hidden'; | |
| const focusable = overlayRef.current?.querySelector( | |
| 'button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])' | |
| ); | |
| if (focusable) { | |
| focusable.focus(); | |
| } | |
| return () => { | |
| document.removeEventListener('keydown', handleKeyDown); | |
| document.body.style.overflow = ''; | |
| previousFocusRef.current?.focus(); | |
| }; | |
| }, [handleKeyDown]); |
🤖 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/components/ProjectCardWithModal.jsx` around lines 12 - 42, The modal
focus handling in ProjectCardWithModal only closes on Escape and does not trap
Tab navigation, so keyboard focus can escape behind the dialog. Update the
existing handleKeyDown/useEffect logic to intercept Tab and Shift+Tab, cycle
focus among the dialog’s focusable elements inside overlayRef.current, and keep
focus contained until onClose runs. Use the existing overlayRef, handleKeyDown,
and previousFocusRef setup to implement the trap and restore focus on cleanup.
| animation: overlayFadeIn 0.2s ease; | ||
| } | ||
|
|
||
| @keyframes overlayFadeIn { | ||
| from { | ||
| opacity: 0; | ||
| } | ||
| to { | ||
| opacity: 1; | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Rename the keyframes to match the enforced stylelint pattern.
The current camelCase names trip the configured keyframes-name-pattern rule, so this stylesheet will not lint cleanly.
Proposed fix
.overlay {
@@
- animation: overlayFadeIn 0.2s ease;
+ animation: overlay-fade-in 0.2s ease;
}
-@keyframes overlayFadeIn {
+@keyframes overlay-fade-in {
@@
.modal {
@@
- animation: modalSlideUp 0.25s ease;
+ animation: modal-slide-up 0.25s ease;
}
-@keyframes modalSlideUp {
+@keyframes modal-slide-up {Also applies to: 35-47
🧰 Tools
🪛 Stylelint (17.13.0)
[error] 14-14: Expected keyframe name "overlayFadeIn" to be kebab-case (keyframes-name-pattern)
(keyframes-name-pattern)
🤖 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/components/ProjectCardWithModal.module.css` around lines 11 - 21, The
stylesheet’s keyframe names in ProjectCardWithModal.module.css use camelCase and
violate the configured keyframes-name-pattern rule, so rename the animation and
matching `@keyframes` identifiers to the required stylelint-compliant format.
Update every reference in the module, including the overlay fade-in usage and
the other keyframes block noted in the review, so the animation names stay
consistent and lint passes cleanly.
Source: Linters/SAST tools
| .nav-links li a { | ||
| padding: 0.3rem 0.4rem; | ||
| font-size: 0.7rem; | ||
| @keyframes navbarPulse { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Rename the keyframe to satisfy the configured Stylelint rule.
Line 614 introduces @keyframes navbarPulse, but Stylelint already flags it because the project enforces kebab-case keyframe names. Rename the keyframe and its corresponding animation references together, otherwise this change will keep failing lint.
🧰 Tools
🪛 Stylelint (17.13.0)
[error] 614-614: Expected keyframe name "navbarPulse" to be kebab-case (keyframes-name-pattern)
(keyframes-name-pattern)
🤖 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/globals.css` at line 614, The `@keyframes` definition for navbarPulse
violates the project’s kebab-case Stylelint rule, so rename this keyframe in
app/globals.css to a kebab-case name and update every matching animation
reference to use the same new name. Make sure the change is applied consistently
wherever the keyframe is defined or consumed so the stylesheet still compiles
and passes lint.
Source: Linters/SAST tools
| .infoText { | ||
| font-size: 0.9rem; | ||
| word-break: break-word; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- Target file around lines 250-290 ---'
nl -ba app/sections/ContactSection.module.css | sed -n '250,290p'
echo
echo '--- Search for word-break / overflow-wrap usage ---'
rg -n "word-break|overflow-wrap" app/sections -g '*.css' || true
echo
echo '--- Search for stylelint config and rules mentioning word-break ---'
git ls-files | rg '(^|/)(stylelint|\.stylelintrc|stylelint\.config)|package\.json$|\.css$' || true
rg -n "word-break|overflow-wrap|stylelint" . -g '.stylelintrc*' -g 'stylelint.config.*' -g 'package.json' -g '*.js' -g '*.cjs' -g '*.mjs' -g '*.json' || trueRepository: khankadir193/Portfolio-Next-Js
Length of output: 249
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- Target file around lines 250-290 ---'
sed -n '250,290p' app/sections/ContactSection.module.css | sed '=' | sed 'N;s/\n/\t/'
echo
echo '--- Search for word-break / overflow-wrap usage in CSS ---'
rg -n "word-break|overflow-wrap" app -g '*.css' || true
echo
echo '--- Find stylelint config / package.json ---'
git ls-files | rg '(^|/)(\.stylelintrc(\..*)?|stylelint\.config\..*|package\.json)$'
echo
echo '--- Search for related lint rules/config references ---'
rg -n "word-break|overflow-wrap|stylelint" . -g '.stylelintrc*' -g 'stylelint.config.*' -g 'package.json' -g '*.js' -g '*.cjs' -g '*.mjs' -g '*.json' || trueRepository: khankadir193/Portfolio-Next-Js
Length of output: 1177
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,260p' package.json | sed '=' | sed 'N;s/\n/\t/'
echo
echo '--- stylelint mentions in package.json ---'
rg -n "stylelint|lint" package.json || trueRepository: khankadir193/Portfolio-Next-Js
Length of output: 907
🌐 Web query:
MDN word-break break-word deprecated overflow-wrap anywhere
💡 Result:
The CSS declaration word-break: break-word is deprecated [1][2]. It is widely recommended to use overflow-wrap: anywhere as its modern, standard replacement [3][2]. ### Status and Recommendation - Deprecation: The break-word keyword for the word-break property is deprecated [1][4][5]. Modern CSS specifications and browser standards discourage its use in new projects [2]. - Preferred Alternative: overflow-wrap: anywhere is the current standard approach to achieve the desired wrapping behavior [3][2]. It instructs the browser to break otherwise unbreakable strings (like long URLs or words) at arbitrary points if no other valid break opportunities exist [6]. ### Why the Change? Historically, word-break: break-word was introduced as a non-standard value to address text overflow issues [1][7]. Its behavior was eventually superseded by the standardization of overflow-wrap [6]. Using overflow-wrap: anywhere is preferred because it provides more predictable behavior across modern browsers and aligns with current CSS specifications [6][3]. ### Key Differences to Note While word-break: break-word and overflow-wrap: anywhere are often used to solve the same problem (preventing text overflow), they can differ in how they influence the layout's "intrinsic size" calculation [8][9]. Specifically, overflow-wrap: anywhere takes the soft wrap opportunities introduced by the break into account when calculating the min-content size of a container [6][9], whereas older implementations of similar properties might have handled this differently [8]. By transitioning to overflow-wrap: anywhere, developers ensure their code adheres to modern standards while maintaining consistent, robust text wrapping [3][2].
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Properties/word-break
- 2: https://web-platform-dx.github.io/web-features-explorer/features/word-break-break-word/
- 3:
word-break: break-word;vsoverflow-wrap: anywhere;mdn/content#15676 - 4: https://github.com/mdn/content/blob/main/files/en-us/web/css/reference/properties/word-break/index.md
- 5: [css-text-3] Normatively disallow break-word w3c/csswg-drafts#4087
- 6: https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Properties/overflow-wrap
- 7: [css-text-3] Implement or unship word-break:break-word w3c/csswg-drafts#2390
- 8: https://stackoverflow.com/questions/66724509/overflow-wrap-break-word-vs-word-break-break-word
- 9: https://stackoverflow.com/questions/56618668/do-overflow-wrap-break-word-and-word-break-break-word-ever-behave-differen
Use a standard wrap rule for long contact text.
word-break: break-word is deprecated here; switch to overflow-wrap: anywhere to keep the mobile wrapping behavior without relying on the legacy value.
Proposed fix
.infoText {
font-size: 0.9rem;
- word-break: break-word;
+ overflow-wrap: anywhere;
+ word-break: normal;
}📝 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.
| .infoText { | |
| font-size: 0.9rem; | |
| word-break: break-word; | |
| .infoText { | |
| font-size: 0.9rem; | |
| overflow-wrap: anywhere; | |
| word-break: normal; |
🧰 Tools
🪛 Stylelint (17.13.0)
[error] 276-276: Deprecated keyword "break-word" for property "word-break" (declaration-property-value-keyword-no-deprecated)
(declaration-property-value-keyword-no-deprecated)
🤖 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/sections/ContactSection.module.css` around lines 274 - 276, The .infoText
rule in ContactSection.module.css uses a deprecated wrap property, so replace
word-break: break-word with overflow-wrap: anywhere to preserve the same
mobile-friendly long-text wrapping behavior. Update the existing .infoText
selector only, keeping the rest of the contact text styling unchanged.
Source: Linters/SAST tools
2 participants
Summary by CodeRabbit
New Features
Bug Fixes