-
-
Notifications
You must be signed in to change notification settings - Fork 186
feat: update dependencies and improve floating header component #126
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR replaces Lucide icons with React Icons, standardizes URL param handling, and enhances the floating header’s scroll performance.
- Swaps out
lucide-react
icons forreact-icons/lu
aliases across multiple UI components - Refactors
Link
imports toNextLink
and uses the URL API to appendref=onur.dev
- Adds throttled scroll handling and an opacity calculation helper to the floating header
Reviewed Changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
src/components/ui/select.jsx | Replaced Lucide imports with react-icons/lu |
src/components/ui/dialog.jsx | Switched XIcon import to react-icons/lu |
src/components/ui/carousel.jsx | Updated arrow icons to react-icons/lu |
src/components/tailwind-indicator.js | Adjusted size and text classes for indicator |
src/components/submit-bookmark/drawer.js | Swapped SendIcon to react-icons/lu |
src/components/submit-bookmark/dialog.js | Swapped SendIcon to react-icons/lu |
src/components/side-menu.js | Switched RadioIcon to react-icons/lu |
src/components/navigation-link.js | Renamed Link to NextLink and updated icon imports |
src/components/mobile-drawer.js | Swapped CommandIcon to react-icons/lu |
src/components/menu-content.js | Renamed Link to NextLink |
src/components/list-item.js | Renamed Link to NextLink |
src/components/link.js | Used URL API to append ref param |
src/components/floating-header.js | Introduced throttled scroll, helper fn, constants, NextLink |
src/components/bookmark-list.jsx | Swapped ArrowDownIcon to react-icons/lu |
src/components/bookmark-card.js | Used URL API for bookmarks, swapped Link2Icon |
src/app/page.js | Renamed Link to NextLink |
src/app/layout.js | Replaced EyeIcon import and removed wrapper className |
src/app/journey/page.js | Tweaked timeline item positioning classes |
src/app/bookmarks/page.js | Renamed Link to NextLink |
Files not reviewed (1)
- package.json: Language not supported
Comments suppressed due to low confidence (2)
src/components/floating-header.js:35
- [nitpick] Calling
.toFixed(2)
returns a string, which is then coerced back to a number byMath.max
/Math.min
. Consider using a numeric rounding approach (e.g.,Math.round(value * 100) / 100
) to keep the value as a number and avoid implicit type conversions.
((scrollY - MOBILE_SCROLL_THRESHOLD * (MOBILE_SCROLL_THRESHOLD / (scrollY ** 2 / SCROLL_OPACITY_DIVISOR))) / SCROLL_OPACITY_DIVISOR).toFixed(2),
src/components/floating-header.js:29
- The new
calculateOpacity
helper encapsulates nontrivial math logic. Adding unit tests for this function and the throttled scroll behavior would help ensure UI consistency across edge cases.
const calculateOpacity = (scrollY) => {
<SideMenu> | ||
<MenuContent /> | ||
</SideMenu> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The original wrapper passed className="relative hidden lg:flex"
to SideMenu
. Removing these classes may change its positioning and responsive behavior. Consider reapplying necessary layout classes or moving them into the SideMenu
component to preserve the intended layout.
<SideMenu> | |
<MenuContent /> | |
</SideMenu> | |
<div className="relative hidden lg:flex"> | |
<SideMenu> | |
<MenuContent /> | |
</SideMenu> | |
</div> |
Copilot uses AI. Check for mistakes.
… and rate limit utilities
Description
Please provide a brief description of what this PR accomplishes.
If applicable, you should then provide a more detailed description including screenshots (if appropriate) to benefit
everyone.
Type of change
How do I test this?
Checklist
Did you remember to take care of the following?
bun install
– for the new dependenciesbun.lockb
file when there is a package addition or update