MOB-03: Mobile-responsive CSS for core flows#564
Conversation
Sidebar slides off-screen on mobile (<640px) with overlay backdrop. Hamburger button in a new mobile top bar toggles the sidebar. Nav items close the mobile menu on click. Content padding reduced on mobile to prevent horizontal overflow. Closes partial #543
Single-column layout, full-width buttons with 44px min tap targets, reduced spacing, readable subtitle text. Onboarding steps and action tiles stack vertically on small screens. Partial #543
Stacked single-column layout, capped list height at 50vh, readable excerpts with 3-line clamp, full-width action buttons with 44px min tap targets, and compact detail panel on mobile. Partial #543
Full viewport modal on <640px, 48px min-height buttons, 44px close button, 16px input font to prevent iOS zoom, flexible textarea that fills available space. Save button placed at bottom for thumb reach. Partial #543
Single-column proposal cards, full-width approve/reject/execute buttons with 48px min tap targets, compact summary grid (2-col with hidden helpers), stacked meta/provenance, and full-width board filter on mobile. Partial #543
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
The ShellTopbar (workspace mode selector, command palette trigger) is hidden at <640px since the mobile hamburger topbar provides navigation. Prevents duplicate chrome and horizontal overflow. Partial #543
Self-review findingsVerified good
Issues found and fixed
Residual observations (non-blocking)
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces comprehensive mobile responsiveness across the Taskdeck web application, including a new mobile top bar with a hamburger menu, an off-canvas sidebar, and full-screen modal adjustments. Feedback focuses on improving the robustness of the mobile layout by using dynamic viewport units (dvh) to handle mobile browser toolbars, resolving a conflicting media query in HomeView.vue that causes inconsistent column counts, and addressing potential issues with background scrolling and hidden overflows.
| .td-home-card__stats { | ||
| grid-template-columns: repeat(2, minmax(0, 1fr)); | ||
| } |
There was a problem hiding this comment.
There's a conflicting media query definition for .td-home-card__stats. An existing rule at (max-width: 768px) sets grid-template-columns: 1fr;, but this new rule at (max-width: 640px) sets it back to repeat(2, minmax(0, 1fr));.
This causes an inconsistent layout progression: 2 columns on desktop, 1 on tablet, and then back to 2 on mobile. To ensure a consistent single-column layout on smaller screens, you should remove this block and let the style from the 768px breakpoint apply.
| height: 100%; | ||
| max-height: 100vh; |
There was a problem hiding this comment.
Using 100vh for height can cause issues on mobile browsers with dynamic toolbars (like Safari's address bar), potentially cutting off content at the bottom of the screen. The dvh (dynamic viewport height) unit is designed to solve this by adjusting to the visible viewport size.
Replacing height: 100% and max-height: 100vh with height: 100dvh will make the modal more robust on mobile devices.
height: 100dvh;
|
|
||
| .td-content { | ||
| padding: var(--td-space-4); | ||
| overflow-x: hidden; |
There was a problem hiding this comment.
Applying overflow-x: hidden can hide underlying layout issues where content exceeds its container's width. While it prevents horizontal scrolling, the content is simply clipped, which might not be the desired behavior.
It's generally better to identify and fix the root cause of the overflow. If this is intended as a temporary safeguard, please consider adding a code comment to explain its purpose.
| const workspace = useWorkspaceStore() | ||
|
|
||
| const sidebarCollapsed = ref(false) | ||
| const mobileOpen = ref(false) |
There was a problem hiding this comment.
When the mobile sidebar is open, the main page content behind the overlay remains scrollable. This can lead to a confusing user experience.
To improve this, you can prevent the <body> from scrolling when the sidebar is open. This is typically done by watching the mobileOpen state in the parent AppShell.vue component and toggling overflow: hidden on document.body.
Here's an example you could add to AppShell.vue:
<script setup>
// ... imports and other code
watch(() => sidebarRef.value?.mobileOpen, (isOpen) => {
document.body.style.overflow = isOpen ? 'hidden' : '';
});
onUnmounted(() => {
// Ensure style is reset if component is unmounted while open
document.body.style.overflow = '';
});
</script>| } | ||
|
|
||
| .td-inbox__list-panel { | ||
| max-height: 50vh; |
There was a problem hiding this comment.
Using vh units can be problematic on mobile browsers with dynamic UI elements like address bars. 50vh might represent more than 50% of the actual visible screen area, causing this panel to be larger than intended.
To ensure the layout is robust, consider using a dynamic viewport unit like dvh.
max-height: 50dvh;
Adversarial Code Review -- PR #564 (MOB-03: Mobile-responsive CSS)Build & Test Status
Findings[High] iOS
|
Add max-height: 100dvh as progressive enhancement after the 100vh fallback so the modal does not overflow behind iOS browser chrome on mobile Safari.
- Lock body scroll when mobile sidebar is open to prevent background content from scrolling behind the overlay. - Register Escape key handler via useEscapeStack so tablet users with keyboards can dismiss the drawer. - Add aria-hidden to the backdrop overlay for screen readers. - Restore body overflow on cleanup and unmount.
Replace hardcoded 11px with var(--td-font-xs) to avoid triggering iOS auto-zoom on text selection.
Final Assessment -- Post-Fix ReviewFixes Pushed (3 commits)
Verification
Remaining Items (not blocking merge, suitable for follow-ups)
OverallThe PR is well-structured -- CSS-only additive changes with consistent breakpoints, good tap target sizing (44-48px), and no desktop regression risk. The three fixes pushed address the most impactful issues (iOS viewport, scroll lock, accessibility). With those in place, this is ready to merge with the remaining items tracked as follow-ups. |
Removes conflicting 640px override that re-introduced 2 columns below the 768px 1fr rule. Fixes Gemini review finding.
Progressive enhancement: vh for older browsers, dvh for mobile browsers with dynamic chrome. Fixes Gemini review finding.
Documents that the clip is intentional to prevent horizontal scroll from wide content on narrow viewports. Fixes Gemini review finding.
1 participant
Summary
All changes use CSS-only
@media (max-width: 640px)queries in scoped<style>blocks. Desktop layouts are preserved -- no existing breakpoints were removed.Closes #543
Test plan
npm run typecheckpassesnpm run buildpassesnpx vitest --run-- all 1107 tests pass