fixed footer not sticking always to bottom when height is higher #2083
Conversation
…wport, now fixed that
Summary by CodeRabbit
WalkthroughUpdated the application layout in frontend/src/app/layout.tsx to use flexbox classes on the body, removed inline minHeight styling, and introduced a main wrapper with flex growth to adjust content height relative to header and footer. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNone found. Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![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
🧹 Nitpick comments (1)
frontend/src/app/layout.tsx (1)
67-67: Layout enhancement: add dvh support; Providers confirmed wrapper-free
- Verified that
Providersinfrontend/src/wrappers/provider.tsxonly composes React and context providers (Suspense,SessionProvider,HeroUIProvider,NextThemesProvider) and does not emit any DOM wrapper—your<main>and<Footer>remain direct children of<body>.- Optional improvement: leverage dynamic viewport height on supported browsers to avoid iOS Safari 100vh bugs.
Apply this enhancement if you’d like modern
dvhsupport:- className={`${geistSans.variable} ${geistMono.variable} antialiased flex flex-col min-h-screen`} + className={`${geistSans.variable} ${geistMono.variable} antialiased flex flex-col min-h-screen supports-[min-height:100dvh]:min-h-[100dvh]`}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
frontend/src/app/layout.tsx(1 hunks)
| <main className="flex-1 flex flex-col"> | ||
| {children} | ||
| </main> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Wrapping children in a flex-1
correctly fills vertical space; ensure no nested tags and consider adding an anchor id.- This solves the short-content gap and keeps the footer pinned.
- Verify no page-level components also declare a
<main>to avoid invalid nested mains. - Optional: add
id="main-content"(andmin-w-0to prevent overflow in flex layouts) to support “skip to content” links and avoid horizontal scroll issues.
Suggested tweak:
- <main className="flex-1 flex flex-col">
+ <main id="main-content" className="flex-1 flex flex-col min-w-0">
{children}
</main>Scan for other <main> usages under the app directory (excluding this layout) to avoid nesting:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find <main> occurrences in app routes except this layout file
rg -nP -C2 '<main\b' frontend/src/app | grep -v 'frontend/src/app/layout.tsx' || trueLength of output: 993
Avoid Nested <main> Elements & Enhance Accessibility
We’ve detected nested <main> tags in your routes, which results in invalid HTML structure. Please update as follows:
• Replace page-level/layout <main> in non-root files with a semantically appropriate container (e.g., <div role="main">).
- frontend/src/app/projects/dashboard/layout.tsx (line 19)
- frontend/src/app/global-error.tsx (line 31)
• Update the root layout (frontend/src/app/layout.tsx) to support skip links and prevent horizontal overflow:
- <main className="flex-1 flex flex-col">
+ <main id="main-content" className="flex-1 flex flex-col min-w-0">
{children}
</main>📝 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.
| <main className="flex-1 flex flex-col"> | |
| {children} | |
| </main> | |
| <main id="main-content" className="flex-1 flex flex-col min-w-0"> | |
| {children} | |
| </main> |
🤖 Prompt for AI Agents
In frontend/src/app/layout.tsx around lines 73-75, update the root layout to add
a skip-link and prevent horizontal overflow: keep this <main> as the single
document-level main, give it an id (e.g. "skip-to-content") and a class to
prevent horizontal scrolling (e.g. add overflow-x-hidden to the container), and
add a visually-hidden but keyboard-focusable skip link anchor before the header
that points to that id; also ensure all non-root pages
(frontend/src/app/projects/dashboard/layout.tsx line ~19 and
frontend/src/app/global-error.tsx line ~31) replace their page-level <main> with
a semantically equivalent container such as <div role="main"> so there are no
nested <main> elements.
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
frontend/src/app/global-error.tsx (1)
31-49: Fix mismatched closing tag: openedbut closed with .This is a parse/build error (also flagged by Biome). Close the opened div properly.
- </main> + </div>
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
frontend/src/app/global-error.tsx(1 hunks)frontend/src/app/layout.tsx(1 hunks)frontend/src/app/projects/dashboard/layout.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/app/layout.tsx
🧰 Additional context used
🪛 Biome (2.1.2)
frontend/src/app/global-error.tsx
[error] 31-31: Expected corresponding JSX closing tag for 'div'.
Opening tag
closing tag
(parse)
frontend/src/app/global-error.tsx
Outdated
| @@ -28,7 +28,7 @@ export const ERROR_CONFIGS: Record<string, ErrorDisplayProps> = { | |||
| export const ErrorDisplay: React.FC<ErrorDisplayProps> = ({ statusCode, title, message }) => { | |||
| const router = useRouter() | |||
| return ( | |||
| <main className="flex min-h-screen flex-col items-center bg-white pt-8 dark:bg-slate-900"> | |||
| <div role="main" className="flex min-h-screen flex-col items-center bg-white pt-8 dark:bg-slate-900"> | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Avoid multiple “main” landmarks; drop role="main" here.
The root layout now wraps children in a single
. Keeping role="main" here risks duplicate main landmarks and degrades a11y. Unless this component is guaranteed to render outside the root layout, remove the role.Proposed change:
- <div role="main" className="flex min-h-screen flex-col items-center bg-white pt-8 dark:bg-slate-900">
+ <div className="flex min-h-screen flex-col items-center bg-white pt-8 dark:bg-slate-900">To verify there’s only one main landmark per page, you can scan the repo:
🏁 Script executed:
#!/bin/bash
# All <main> elements
rg -nP -C2 '<main\b' --type=tsx --type=jsx --type=ts --type=js
# All role="main" occurrences
rg -nP -C2 'role\s*=\s*["\']main["\']' --type=tsx --type=jsx --type=ts --type=js
# Root main-content (should appear once in the app layout)
rg -nP -C2 'id\s*=\s*["\']main-content["\']' --type=tsx --type=jsx --type=ts --type=jsLength of output: 209
🏁 Script executed:
#!/usr/bin/env bash
# Search for <main> elements in TSX/JSX/TS/JS files
rg -n '<main\b' -g '*.{tsx,jsx,ts,js}'
# Search for role="main" occurrences
rg -n 'role="main"' -g '*.{tsx,jsx,ts,js}'
# Search for id="main-content" occurrences
rg -n 'id="main-content"' -g '*.{tsx,jsx,ts,js}'Length of output: 610
Remove redundant role="main" landmarks
Scan results show one native <main id="main-content"> (root layout) but two extra ARIA mains:
- frontend/src/app/global-error.tsx:31
- frontend/src/app/projects/dashboard/layout.tsx:19
Keeping these risks duplicate landmarks and degrades a11y. At minimum, drop the role="main" from your error component:
- <div role="main" className="flex min-h-screen flex-col items-center bg-white pt-8 dark:bg-slate-900">
+ <div className="flex min-h-screen flex-col items-center bg-white pt-8 dark:bg-slate-900">Also review the dashboard layout’s role="main"—either remove it or refactor to a suitable landmark (e.g., <section role="region">).
📝 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.
| <div role="main" className="flex min-h-screen flex-col items-center bg-white pt-8 dark:bg-slate-900"> | |
| <div className="flex min-h-screen flex-col items-center bg-white pt-8 dark:bg-slate-900"> |
🧰 Tools
🪛 Biome (2.1.2)
[error] 31-31: Expected corresponding JSX closing tag for 'div'.
Opening tag
closing tag
(parse)
🤖 Prompt for AI Agents
In frontend/src/app/global-error.tsx around line 31, the error component adds a
redundant ARIA landmark via role="main"; remove the role="main" attribute from
that div so it no longer duplicates the native <main id="main-content">
landmark, and then review frontend/src/app/projects/dashboard/layout.tsx (around
line 19) to either remove its role="main" or change it to an appropriate
landmark (for example use a semantic <section> with role="region" if needed) to
avoid duplicate main landmarks and preserve accessibility.
| <div role="main" className="flex-1 p-4">{children}</div> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Don’t use role="main" inside pages; add min-w-0 to avoid flex overflow.
- A11y: The app’s root layout already defines the single main region. Using role="main" here creates duplicate main landmarks.
- Layout: In a flex row with a sidebar, add min-w-0 on the content pane to prevent unintended horizontal overflow.
- <div role="main" className="flex-1 p-4">{children}</div>
+ <div className="flex-1 min-w-0 p-4">{children}</div>📝 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.
| <div role="main" className="flex-1 p-4">{children}</div> | |
| </div> | |
| <div className="flex-1 min-w-0 p-4">{children}</div> | |
| </div> |
🤖 Prompt for AI Agents
In frontend/src/app/projects/dashboard/layout.tsx around lines 19 to 20, remove
the duplicate role="main" attribute from the content div and add the Tailwind
utility min-w-0 to its className to prevent flex-induced horizontal overflow;
ensure the className becomes the original classes plus min-w-0 and do not add
any additional landmark roles since the app root already provides the main
region.
Davda-James
force-pushed
the
fix/ui-footer
branch
from
69917a1 to
c46847a
Compare
|
@kasya any reviews on this ? |
DId you check your solution works locally? For me it didn't make any difference 🤷♂️ |
|
|
Closing due to a missed deadline. |
Footer was not sticking to bottom if height of screen is large
Resolves #2033
What was the actual bug/issue ?
the has [minHeight: 100vh] set inline, but the layout is not using a flexbox or grid to ensure the footer is always at the bottom when content is short.
The footer itself uses mt-auto, which only works if a parent uses flex with flex-col and min-h-screen (or min-h-[100vh].
Fix
Made the a flex container with flex-col and min-h-screen (or min-h-[100vh]) ensuring the main content area ({children}) grows to fill available space.