Enhance iOS overscroll support with layout adjustments and safe area …#511
Enhance iOS overscroll support with layout adjustments and safe area …#511felix-schultz merged 1 commit intodevfrom
Conversation
Summary of ChangesHello @felix-schultz, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the application's layout and visual consistency across various devices, with a particular focus on iOS and desktop environments. The changes ensure that UI elements correctly adapt to safe areas, prevent content overflow, and maintain a polished appearance by refining how heights, padding, and background colors are applied throughout the chat and sidebar components. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request does a great job of enhancing the UI for iOS and desktop by addressing overscroll behavior and safe area insets. The layout adjustments, such as using flex-1 min-h-0 and 100dvh, are solid improvements for creating a more robust and consistent experience across devices.
I've added a few suggestions to further improve the code by using Tailwind CSS's arbitrary value syntax instead of inline styles for consistency. I also spotted a minor formatting issue. Overall, these are excellent changes that significantly improve the application's mobile experience.
| }} | ||
| > | ||
| <div className="flex items-center gap-2 text-sm font-medium"> | ||
| <div className="flex items-center gap-2 text-sm font-medium" style={{paddingTop: "var(--fl-safe-top, env(safe-area-inset-top, 0px))"}}> |
There was a problem hiding this comment.
To maintain a consistent styling approach with Tailwind CSS, consider using an arbitrary value for padding within the className prop instead of an inline style. This keeps styling declarations consolidated and improves readability.
| <div className="flex items-center gap-2 text-sm font-medium" style={{paddingTop: "var(--fl-safe-top, env(safe-area-inset-top, 0px))"}}> | |
| <div className="flex items-center gap-2 text-sm font-medium pt-[var(--fl-safe-top,env(safe-area-inset-top,0px))]"> |
| <div | ||
| className="bg-background px-2 pb-2 max-w-screen-lg w-full mx-auto" | ||
| style={{ | ||
| paddingBottom: | ||
| "calc(0.5rem + var(--fl-safe-bottom, env(safe-area-inset-bottom, 0px)))", | ||
| }} | ||
| > |
There was a problem hiding this comment.
For consistency with the project's use of Tailwind CSS, it's better to use arbitrary values in the className prop instead of inline styles. This also allows removing the redundant pb-2 class, as the padding is handled by the arbitrary value.
| <div | |
| className="bg-background px-2 pb-2 max-w-screen-lg w-full mx-auto" | |
| style={{ | |
| paddingBottom: | |
| "calc(0.5rem + var(--fl-safe-bottom, env(safe-area-inset-bottom, 0px)))", | |
| }} | |
| > | |
| <div className="bg-background px-2 max-w-screen-lg w-full mx-auto pb-[calc(0.5rem+var(--fl-safe-bottom,env(safe-area-inset-bottom,0px)))]"> |
| <div className="relative border-b border-border/20 bg-background/95 backdrop-blur-md"> | ||
| <div className="p-4"> | ||
| <div className="flex items-center justify-between mb-4"> | ||
| <div className="flex items-center justify-between mb-4" style={{paddingTop: "var(--fl-safe-top, env(safe-area-inset-top, 0px))"}}> |
There was a problem hiding this comment.
To maintain a consistent styling approach with Tailwind CSS, consider using an arbitrary value for padding within the className prop instead of an inline style. This keeps styling declarations consolidated.
| <div className="flex items-center justify-between mb-4" style={{paddingTop: "var(--fl-safe-top, env(safe-area-inset-top, 0px))"}}> | |
| <div className="flex items-center justify-between mb-4 pt-[var(--fl-safe-top,env(safe-area-inset-top,0px))]"> |
| </div> | ||
| <div> | ||
| <h2 className="text-lg font-semibold text-foreground"> | ||
| <h2 className="text-lg font-semibold text-foreground" > |
There was a problem hiding this comment.
There's an extra space before the closing > of the h2 tag. While this is valid JSX, it's best to remove it for code cleanliness and consistency.
| <h2 className="text-lg font-semibold text-foreground" > | |
| <h2 className="text-lg font-semibold text-foreground"> |
| "var(--fl-safe-bottom, env(safe-area-inset-bottom, 0px))", | ||
| }} | ||
| > | ||
| <FlowBackground |
There was a problem hiding this comment.
Bug: The SidebarInset component in the desktop app is missing bottom padding for the safe area, which will obscure UI elements on iOS devices with a home indicator.
Severity: MEDIUM
Suggested Fix
Re-add the bottom padding with safe area support to the SidebarInset component in apps/desktop/components/app-sidebar.tsx. This can be done by applying a style similar to the web app's implementation: paddingBottom: var(--fl-safe-bottom, env(safe-area-inset-bottom, 0px)).
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: apps/desktop/components/app-sidebar.tsx#L282
Potential issue: The `SidebarInset` component in the desktop app no longer provides
bottom padding to account for the safe area on mobile devices. Unlike the web app
version which retains this padding, the desktop app's change means that on most non-chat
pages (like Library, Settings, and Admin pages), content at the bottom will be obscured
by the home indicator or other system UI on iOS devices (e.g., iPhone X and newer). This
will make buttons or other interactive elements at the bottom of these pages
inaccessible to users on affected devices.
Did we get this right? 👍 / 👎 to inform future reviews.
This pull request focuses on improving layout consistency, safe area handling (especially for desktop and iOS), and overall visual polish across the chat and sidebar interfaces. The changes ensure that padding, height, and overflow behaviors are correct in various environments, and that the UI adapts smoothly to device safe areas.
Safe area and layout improvements:
min-h-screento the root layout container inapps/desktop/app/layout.tsxto ensure the app fills the viewport vertically.ChatInterface,ChatHistory) to addpaddingTopusing CSS variables for safe area insets, improving support for devices with notches or special display areas. [1] [2]paddingBottomthat includes the safe area inset, ensuring it isn't obscured on devices with bottom safe areas.Sidebar and main content adjustments:
IOSQuickMenuTriggerand adjusted sidebar and main content area heights and overflow to better fit the viewport and prevent unwanted scrolling or clipping, especially on iOS.100dvhand properly constrain overflow, ensuring the sidebar and main content fill the screen without causing body overflow.General background and overflow fixes:
bg-backgroundto thehtmlelement for consistent background color across the app.flex-1 min-h-0patterns, which provide more reliable flexbox sizing and overflow handling.These changes collectively provide a more robust and visually consistent experience across different devices and environments.…handling