fix: footer layout and styles for improved responsiveness#2330
Conversation
- Added a footer spacer div to enhance layout structure. - Changed footer display to grid for better alignment on larger screens. - Adjusted footer spacer visibility for mobile and desktop views.
WalkthroughFooter markup adds a spacer div between user notice and copyright. CSS switches footer layout to a 3-column grid on desktop, introduces .footer-spacer visibility rules, and removes several flex-based styles to align with the grid approach. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 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 comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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 (4)
emhttp/plugins/dynamix/include/DefaultPageLayout/Footer.php (2)
59-59: Hide spacer from assistive tech and remove non-breaking spaceThe spacer is purely presentational. Prevent it from being announced by screen readers and avoid using non-breaking space for layout.
- <div class="footer-spacer"> </div> + <div class="footer-spacer" aria-hidden="true"></div>
59-59: Optional: Avoid extra spacer markup by placing items via CSS GridYou can keep only two elements and place the right block into the third grid column, removing the need for the spacer node. If you prefer to keep the current approach for simplicity, feel free to skip.
- Markup change (remove spacer):
- <div class="footer-spacer" aria-hidden="true"></div>
- CSS additions (outside this file) to explicitly place columns:
@media (min-width: 768px) { #footer { display: grid; grid-template-columns: auto minmax(0, 1fr) auto; } .footer-left { grid-column: 1; } #copyright { grid-column: 3; justify-self: end; } }emhttp/plugins/dynamix/styles/default-base.css (2)
704-706: Make spacer non-interactiveNot critical, but disabling pointer events on the spacer avoids it intercepting clicks in the empty middle column.
.footer-spacer { - display: none; /* Hidden by default on mobile */ + display: none; /* Hidden by default on mobile */ + pointer-events: none; }
738-740: Explicitly place spacer in the middle column (clarity/robustness)Auto-placement will put the second child in column 2 today, but making intent explicit reduces coupling to DOM order.
@media (min-width: 768px) { .footer-left { justify-content: flex-start; } .footer-spacer { - display: block; /* Show on desktop */ + display: block; /* Show on desktop */ + grid-column: 2; }Optionally also add (outside these lines) for explicit placement:
@media (min-width: 768px) { .footer-left { grid-column: 1; } #copyright { grid-column: 3; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
emhttp/plugins/dynamix/include/DefaultPageLayout/Footer.php(1 hunks)emhttp/plugins/dynamix/styles/default-base.css(3 hunks)
🔇 Additional comments (2)
emhttp/plugins/dynamix/styles/default-base.css (2)
703-703: No functional changeThis appears to be a structural/formatting-only delta within .footer-left. Nothing to action.
737-737: No functional changeThis looks like a brace/whitespace change from the previous block. Nothing to action.
| display: grid; | ||
| grid-template-columns: auto 1fr auto; | ||
| align-items: center; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use minmax(0, 1fr) to prevent overflow in the center track
Grid tracks with plain 1fr can respect the min-content size of children, causing unexpected overflow/wrapping. Using minmax(0, 1fr) ensures the middle track can shrink as intended.
- display: grid;
- grid-template-columns: auto 1fr auto;
+ display: grid;
+ grid-template-columns: auto minmax(0, 1fr) auto;Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In emhttp/plugins/dynamix/styles/default-base.css around lines 676 to 678, the
grid-template-columns uses "auto 1fr auto" which allows the center column to
grow based on min-content and can cause overflow; replace the middle track with
a shrinkable track by changing it to "auto minmax(0, 1fr) auto" so the center
column can correctly shrink and prevent overflow/wrapping.
3 participants
Summary by CodeRabbit