Improve tablet icon arrangement and synchronize breakpoints#572
Improve tablet icon arrangement and synchronize breakpoints#572ngoiyaeric wants to merge 1 commit intomainfrom
Conversation
- Update mobile layout breakpoint from 768px to 1024px in Chat.tsx, Mapbox.tsx, ProfileToggle.tsx, and Header.tsx. - Update app/globals.css to use justify-content: space-evenly for icons on tablet screens (768px - 1024px) to ensure they are equidistant. - Migrate md: tailwind classes to lg: across multiple components to ensure a consistent transition to the desktop layout at 1024px. - Fixes #571. Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
WalkthroughThis PR systematically shifts responsive design breakpoints across the application from medium (768px) to large (1024px) Tailwind breakpoints, affecting layout padding, spacing, and mobile detection thresholds. Additionally, CSS updates modify mobile icons bar spacing from space-between to space-evenly with 10px gaps. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/chat-panel.tsx (1)
59-59:⚠️ Potential issue | 🟡 MinorFix 1024px breakpoint off-by-one in mobile detection.
Line 59 should use
< 1024(not<= 1024) to match Tailwindlgsemantics and avoid inconsistent behavior exactly at 1024px.Suggested fix
- setIsMobile(window.innerWidth <= 1024) + setIsMobile(window.innerWidth < 1024)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/chat-panel.tsx` at line 59, The mobile-detection uses setIsMobile(window.innerWidth <= 1024) which treats 1024px as mobile; change the comparison to use < 1024 so setIsMobile(window.innerWidth < 1024) to match Tailwind's lg breakpoint semantics and avoid off-by-one behavior exactly at 1024px in the ChatPanel component (search for setIsMobile call in chat-panel.tsx).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/map/mapbox-map.tsx`:
- Around line 288-291: Change the desktop-width check from a strict greater-than
to inclusive so the nav controls are added at exactly 1024px; update the
conditional that uses window.innerWidth (where navControlRef.current is created
and map.current.addControl is called) from `> 1024` to `>= 1024` so behavior
matches the `< 1024` mobile check used elsewhere (affecting navControlRef and
the map.addControl code path).
---
Outside diff comments:
In `@components/chat-panel.tsx`:
- Line 59: The mobile-detection uses setIsMobile(window.innerWidth <= 1024)
which treats 1024px as mobile; change the comparison to use < 1024 so
setIsMobile(window.innerWidth < 1024) to match Tailwind's lg breakpoint
semantics and avoid off-by-one behavior exactly at 1024px in the ChatPanel
component (search for setIsMobile call in chat-panel.tsx).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f1517dae-4795-4229-86e8-ccc8e9f73569
⛔ Files ignored due to path filters (1)
server.logis excluded by!**/*.log
📒 Files selected for processing (17)
app/globals.csscomponents/chat-panel.tsxcomponents/chat.tsxcomponents/copilot-display.tsxcomponents/copilot.tsxcomponents/footer.tsxcomponents/header.tsxcomponents/history-sidebar.tsxcomponents/map/mapbox-map.tsxcomponents/profile-toggle.tsxcomponents/search-results-image.tsxcomponents/search-results.tsxcomponents/search-skeleton.tsxcomponents/settings/components/user-management-form.tsxcomponents/ui/lottie-player.tsxcomponents/ui/toast.tsxcomponents/video-search-results.tsx
📜 Review details
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-20T10:04:04.963Z
Learnt from: ngoiyaeric
Repo: QueueLab/QCX PR: 0
File: :0-0
Timestamp: 2026-01-20T10:04:04.963Z
Learning: The application has a comprehensive credits and payment UI system with: (1) PurchaseCreditsPopup that shows after user authentication with a 7-day cooldown, (2) History panel on the left side containing CreditsDisplay at the top and ChatHistoryClient below, (3) TentTree icon in header that opens UsageSidebar on the right side showing usage history and billing options. All these components are coordinated through RootLayout and only render for authenticated users.
Applied to files:
components/header.tsx
🔇 Additional comments (18)
components/settings/components/user-management-form.tsx (1)
82-87: Breakpoint migration is consistent and correct for this form row.
lg:grid-cols-3andlg:col-span-2are aligned and preserve the intended 2/1 split starting at the 1024px breakpoint.components/ui/toast.tsx (1)
19-19: Breakpoint change looks correct and aligned with the PR objective.Switching to
lg:max-w-[420px]keeps toast viewport sizing in sync with the new 1024px responsive boundary.components/footer.tsx (1)
8-8: Breakpoint update looks correct.Line 8 cleanly shifts footer responsive padding to the
lgbreakpoint and is consistent with the PR’s 1024px transition objective.components/chat-panel.tsx (1)
193-193: Desktop padding breakpoint migration is aligned.Line 193 correctly moves the desktop container spacing trigger to
lg, matching the intended 1024px layout switch.components/profile-toggle.tsx (1)
17-17: Mobile threshold change is consistent withlgbreakpoints.Line 17 correctly updates the mobile check to
< 1024, which aligns with the new responsive boundary strategy.components/search-skeleton.tsx (1)
13-13: Skeleton grid breakpoint shift is correct.Line 13 appropriately delays the quarter-width layout to
lg, consistent with the PR breakpoint synchronization.components/search-results-image.tsx (1)
70-70: Image tile responsive breakpoint update looks good.Line 70 correctly migrates the 4-column trigger to
lg, aligning image cards with the new 1024px transition.components/history-sidebar.tsx (1)
27-27: History sidebar spacing update is consistent.Line 27 correctly shifts responsive bottom padding to
lgto match the 1024px breakpoint strategy.components/search-results.tsx (1)
27-27: Result-card breakpoint migration is applied consistently.Lines 27 and 56 correctly move both card variants to
lg:w-1/4, preventing mixed behavior between regular cards and the “View more” tile.Also applies to: 56-56
components/video-search-results.tsx (1)
79-79: Video tile breakpoint change is aligned with PR intent.Line 79 correctly shifts the 4-column video tile layout to
lg, matching the rest of the responsive transition updates.components/map/mapbox-map.tsx (1)
390-391: Initial zoom breakpoint update is consistent.Line 390 correctly treats
< 1024as compact/mobile behavior and aligns with the updated responsive breakpoint strategy.components/ui/lottie-player.tsx (1)
16-16: Responsive blur breakpoint change looks correct.Line 16 cleanly moves the overlay blur behavior to the
lgbreakpoint without changing component logic.components/copilot-display.tsx (1)
22-22: Padding breakpoint migration is good.Line 22 updates responsive padding to
lgconsistently with the rest of this PR.components/copilot.tsx (1)
115-115: Completed-state card padding update is consistent.Line 115 aligns the completed card’s responsive padding with the new
lgbreakpoint strategy.components/chat.tsx (2)
59-59: Mobile threshold update is aligned with the 1024px transition goal.Line 59 correctly updates the layout switch to
< 1024.
178-178: Desktop spacing breakpoint migration is correct.Line 178 updates
space-y/ptresponsive classes tolg:*consistently.components/header.tsx (1)
47-47: Header breakpoint synchronization is applied consistently.Lines 47, 69, and 90 correctly migrate visibility/layout behavior from
mdtolgwith no logic regressions in this component.Also applies to: 69-69, 90-90
app/globals.css (1)
150-150: Tablet icon spacing override is correctly scoped and implemented.Lines 162–163 apply the intended 768–1024px distribution behavior (
space-evenlywith gap), and Line 150 comment update stays aligned with that intent.Also applies to: 162-163
| if (window.innerWidth > 1024) { | ||
| navControlRef.current = new mapboxgl.NavigationControl() | ||
| map.current.addControl(navControlRef.current, 'top-left') | ||
| } |
There was a problem hiding this comment.
Use an inclusive 1024px desktop condition for navigation controls.
Line 288 uses > 1024, which excludes exactly 1024px. With components/chat.tsx Line 59 using < 1024 for mobile, 1024px is desktop layout but still misses map nav controls. Use >= 1024 to keep behavior consistent.
🔧 Proposed fix
- if (window.innerWidth > 1024) {
+ if (window.innerWidth >= 1024) {
navControlRef.current = new mapboxgl.NavigationControl()
map.current.addControl(navControlRef.current, 'top-left')
}📝 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.
| if (window.innerWidth > 1024) { | |
| navControlRef.current = new mapboxgl.NavigationControl() | |
| map.current.addControl(navControlRef.current, 'top-left') | |
| } | |
| if (window.innerWidth >= 1024) { | |
| navControlRef.current = new mapboxgl.NavigationControl() | |
| map.current.addControl(navControlRef.current, 'top-left') | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/map/mapbox-map.tsx` around lines 288 - 291, Change the
desktop-width check from a strict greater-than to inclusive so the nav controls
are added at exactly 1024px; update the conditional that uses window.innerWidth
(where navControlRef.current is created and map.current.addControl is called)
from `> 1024` to `>= 1024` so behavior matches the `< 1024` mobile check used
elsewhere (affecting navControlRef and the map.addControl code path).
2 participants
This PR addresses issue #571 by improving the icon arrangement on tablet devices and synchronizing the mobile-to-desktop layout transition at 1024px.
Key changes:
Chat.tsxand related components to use 1024px as the threshold for switching between the mobile (stacked) and desktop (split-screen) layouts.app/globals.cssto usejustify-content: space-evenlyfor the mobile icons bar when screen width is between 768px and 1024px, ensuring icons are equidistant as requested.md:responsive classes tolg:throughout the codebase to maintain UI consistency during the layout transition.PR created automatically by Jules for task 11839020964137083398 started by @ngoiyaeric
Summary by CodeRabbit
Release Notes