Skip to content

fix: PR Analytics layout on tablet/desktop(ipad pro)#359

Open
SaishWadnere wants to merge 4 commits into
Priyanshu-byte-coder:mainfrom
SaishWadnere:fix/issue-334-fix-pr-analytics-ui
Open

fix: PR Analytics layout on tablet/desktop(ipad pro)#359
SaishWadnere wants to merge 4 commits into
Priyanshu-byte-coder:mainfrom
SaishWadnere:fix/issue-334-fix-pr-analytics-ui

Conversation

@SaishWadnere
Copy link
Copy Markdown

Summary

This code sets up a layout that switches from 4 columns on small screens (mobile) to 2 columns on medium-and-up screens

Closes #334

Type of Change

  • Bug fix
  • New feature
  • Documentation update
  • Refactor / code cleanup

Changes Made

Before
<div className="grid grid-cols-2 md:grid-cols-4 gap-4">

After
<div className="grid grid-cols-4 md:grid-cols-2 gap-4">

How to Test

Steps for the reviewer to verify this works:

  1. Open your app in the browser, right-click anywhere, and select Inspect.
  2. Click the Device Toolbar icon (the phone/tablet icon in the top-left of the inspector).
  3. Select iPad Pro from the top dimensions dropdown.
  4. The Test: Verify that the cards are now sitting perfectly in a balanced grid without clipping text or overflowing.

Screenshots (if UI change)

For screen width smaller that is 768px
Screenshot 2026-05-19 142200

For screen width larger than 768px
Screenshot 2026-05-19 142216

For Ipad pro
Screenshot 2026-05-19 142229

Checklist

  • Linked issue in summary
  • npm run lint passes locally
  • No TypeScript errors (npm run type-check)
  • Self-reviewed the diff
  • Added/updated tests if applicable

@vercel
Copy link
Copy Markdown

vercel Bot commented May 19, 2026

@SaishWadnere is attempting to deploy a commit to the PRIYANSHU DOSHI's projects Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your first PR on DevTrack! 🎉

A maintainer will review it within 48 hours. While you wait:

  • Make sure CI is passing (type-check + lint)
  • Double-check the PR description is filled out and the issue is linked
  • Feel free to ask questions in Discussions if you need help

@Priyanshu-byte-coder
Copy link
Copy Markdown
Owner

Hi @SaishWadnere — the fix has the breakpoints inverted, which makes mobile worse not better.

Current (correct for mobile):

grid-cols-2 md:grid-cols-4

This gives 2 columns on mobile, 4 on tablet/desktop — which is correct.

Your change:

grid-cols-4 md:grid-cols-2

This gives 4 columns on mobile (very narrow cells) and 2 on tablet+ — the opposite of what's wanted.

The real issue for iPad Pro cramping is the md: breakpoint is too small (768px). Fix by using lg: instead:

<div className="grid grid-cols-2 lg:grid-cols-4 gap-4">

lg: triggers at 1024px, which gives iPads 2 columns and larger desktops 4. Please update and push.

Copy link
Copy Markdown
Owner

@Priyanshu-byte-coder Priyanshu-byte-coder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change is wrong — grid-cols-4 lg:grid-cols-4 forces 4 columns on all screen sizes including mobile, making the stat cards too narrow to read.

The correct fix for the iPad Pro layout issue is to keep 2 columns on mobile and small screens, and use 4 columns only on large screens:

className="grid grid-cols-2 lg:grid-cols-4 gap-4"

Also rebase on main — the base changed after recent PRs merged.

@SaishWadnere SaishWadnere force-pushed the fix/issue-334-fix-pr-analytics-ui branch from 51a19e5 to 058d2f0 Compare May 19, 2026 15:54
Copy link
Copy Markdown
Owner

@Priyanshu-byte-coder Priyanshu-byte-coder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1. PR description doesn't match diff — description says changing grid-cols-2 md:grid-cols-4 to grid-cols-4 md:grid-cols-2 but actual diff is md:grid-cols-5lg:grid-cols-4. Fix the description.

2. Layout orphan — 5 stat cards in a 4-column grid leaves 1 card alone on the last row at lg+ widths. Consider lg:grid-cols-5 (keeps 1 row) or restructure to 4 cards if avgFirstReviewHours is being removed.

3. Conflicts with #394 — both touch the same grid class on the same line in PRMetrics.tsx. Coordinate or rebase after #394 resolves.

@SaishWadnere
Copy link
Copy Markdown
Author

SaishWadnere commented May 20, 2026

Hey @Priyanshu-byte-coder,

Got it. I will work on changes.

One detail: on mobile (grid-cols-2), the 5th card still ends up alone on the bottom row (screenshot attached). Do you want me to leave it as-is for mobile, or should I make one column for mobile?

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] PR Analytics metric cards squished/deformed on iPad Pro layout (1024px)

2 participants