fix: Enhance RecentProjects component layout and add scrollbar hiding styles #16
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request enhances the visual layout of the RecentProjects component and introduces a global utility class for hiding scrollbars. The changes primarily focus on making the icon displays more responsive and removing the previous overlapping effect for a cleaner presentation.
- Refactored icon container layouts with responsive sizing and wrapping capabilities
- Added gap spacing and flex-wrap support for better icon arrangement across screen sizes
- Introduced a scrollbar-hide utility class in global styles for cross-browser scrollbar hiding
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| components/RecentProjects.tsx | Updated icon layout from overlapping design to responsive grid with breakpoint-specific sizing and flex-wrap support |
| app/globals.css | Added scrollbar-hide utility class with webkit and standard scrollbar hiding rules |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| {lngIconLts.map((icon, index) => ( | ||
| <div | ||
| key={icon} | ||
| className="border border-white/[2.0] rounded-full bg-black w-7 h-7 sm:w-8 sm:h-8 lg:w-9 lg:h-9 xl:w-10 xl:h-10 flex items-center justify-center flex-shrink-0" | ||
| style={{ zIndex: 13 - index }} | ||
| > | ||
| <img src={icon} alt={icon} className="p-1.5 sm:p-2" /> | ||
| </div> | ||
| <div className="flex justify-center items-center"> | ||
| {toolsIconLts.map((icon, index) => ( | ||
| <div | ||
| key={icon} | ||
| className="border border-white/[2.0] rounded-full bg-black lg:w-10 lg:h-10 w-8 h-8 flex items-center justify-center" | ||
| style={{ | ||
| zIndex: 13 - index, | ||
| transform: `translateX(-${7.2 * index}px)`, | ||
| }} | ||
| > | ||
| <img src={icon} alt={icon} className="p-2" /> | ||
| </div> | ||
| ))} | ||
| ))} |
There was a problem hiding this comment.
Using icon file paths as React keys is fragile and could lead to bugs if the same icon appears multiple times in the array or if paths change. Consider using the index parameter or a combination of icon path and index for the key prop.
| {toolsIconLts.map((icon, index) => ( | ||
| <div | ||
| key={icon} | ||
| className="border border-white/[2.0] rounded-full bg-black w-7 h-7 sm:w-8 sm:h-8 lg:w-9 lg:h-9 xl:w-10 xl:h-10 flex items-center justify-center flex-shrink-0" | ||
| style={{ zIndex: 13 - index }} | ||
| > | ||
| <img src={icon} alt={icon} className="p-1.5 sm:p-2" /> | ||
| </div> | ||
| </div> | ||
| ))} |
There was a problem hiding this comment.
Using icon file paths as React keys is fragile and could lead to bugs if the same icon appears multiple times in the array or if paths change. Consider using the index parameter or a combination of icon path and index for the key prop.
| className="border border-white/[2.0] rounded-full bg-black w-7 h-7 sm:w-8 sm:h-8 lg:w-9 lg:h-9 xl:w-10 xl:h-10 flex items-center justify-center flex-shrink-0" | ||
| style={{ zIndex: 13 - index }} | ||
| > | ||
| <img src={icon} alt={icon} className="p-1.5 sm:p-2" /> |
There was a problem hiding this comment.
The alt text for the icon image uses the icon path (URL) as the value, which is not descriptive for screen readers. Consider using a more descriptive alternative text that indicates what the icon represents, such as the technology name (e.g., "Python", "React", etc.).
| className="border border-white/[2.0] rounded-full bg-black w-7 h-7 sm:w-8 sm:h-8 lg:w-9 lg:h-9 xl:w-10 xl:h-10 flex items-center justify-center flex-shrink-0" | ||
| style={{ zIndex: 13 - index }} | ||
| > | ||
| <img src={icon} alt={icon} className="p-1.5 sm:p-2" /> |
There was a problem hiding this comment.
The alt text for the icon image uses the icon path (URL) as the value, which is not descriptive for screen readers. Consider using a more descriptive alternative text that indicates what the tool represents, such as the tool name (e.g., "VSCode", "Jupyter", etc.).
|
|
||
| /* Hide scrollbar while maintaining scroll functionality */ | ||
| .scrollbar-hide { | ||
| -ms-overflow-style: none; | ||
| scrollbar-width: none; | ||
| } | ||
| .scrollbar-hide::-webkit-scrollbar { | ||
| display: none; | ||
| } |
There was a problem hiding this comment.
The scrollbar-hide utility class is defined but not used anywhere in the codebase. Consider either using this class where it's needed or removing it to keep the codebase clean and maintainable.
| /* Hide scrollbar while maintaining scroll functionality */ | |
| .scrollbar-hide { | |
| -ms-overflow-style: none; | |
| scrollbar-width: none; | |
| } | |
| .scrollbar-hide::-webkit-scrollbar { | |
| display: none; | |
| } |
This pull request improves the visual presentation and layout of the RecentProjects component and adds a utility class for hiding scrollbars while preserving scroll functionality. The most important changes are grouped below.
UI and Layout Enhancements in RecentProjects:
flex-wrap,gap, andmin-w-0classes, making the icon display more consistent across screen sizes.Global Styles:
.scrollbar-hideutility class inapp/globals.cssto hide scrollbars across browsers while maintaining scroll functionality.