-
Notifications
You must be signed in to change notification settings - Fork 75
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
Fix/improve-responsiveness-projectCards #132
Conversation
merging fix/improve-responsiveness-projectCards to dev, to push the code from dev branch directly.
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.
Thank you!
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.
❌ Changes requested. Reviewed everything up to ef6a694 in 10 minutes and 18 seconds
More details
- Looked at
22
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
0
drafted comments based on config settings.
Workflow ID: wflow_XnkGh8PaCYxw4T9s
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
@@ -45,7 +45,7 @@ export default function Projects({ isWorkspaceEnabled }: ProjectsProps) { | |||
{[...Array(5).keys()].map((_, index) => ( | |||
<div key={index} className="flex flex-col"> | |||
<Skeleton className="h-8 w-1/3 mb-4" /> | |||
<div className="grid gap-4 grid-cols-1 md:grid-cols-2 lg:grid-cols-3"> | |||
<div className="grid gap-4 grid-cols-1 max-[768px]:grid-cols-1 md:grid-cols-1 lg:grid-cols-2 min-[1441px]:grid-cols-3"> |
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 custom media queries max-[768px]
and min-[1441px]
are not standard Tailwind CSS syntax. Consider using Tailwind's built-in responsive utilities like sm:
, md:
, lg:
, xl:
, and 2xl:
for better maintainability and consistency.
<div className="grid gap-4 grid-cols-1 max-[768px]:grid-cols-1 md:grid-cols-1 lg:grid-cols-2 min-[1441px]:grid-cols-3"> | |
<div className="grid gap-4 grid-cols-1 sm:grid-cols-1 md:grid-cols-1 lg:grid-cols-2 2xl:grid-cols-3"> |
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.
Good point, but this was intentional. We will migrate this to flex-wrap eventually
This PR solves the issue #108
Previously cards looked like this:
Afterwards: https://drive.google.com/file/d/1wcUXduDfv_EDpQ7kJ_IjP30uK2gbI2pQ/view?usp=sharing
Important
Improve responsiveness of project cards in
projects.tsx
by adjusting grid layout for different screen sizes.projects.tsx
for project cards.grid-cols
configuration tomax-[768px]:grid-cols-1
,md:grid-cols-1
,lg:grid-cols-2
,min-[1441px]:grid-cols-3
for better responsiveness.This description was created by for ef6a694. It will automatically update as commits are pushed.