fix: Improve mobile layout and responsiveness across multiple pages - #211
fix: Improve mobile layout and responsiveness across multiple pages#211hhvrc wants to merge 14 commits into
Conversation
# Conflicts: # src/routes/(app)/profile/+page.svelte
There was a problem hiding this comment.
Pull request overview
This PR focuses on improving mobile responsiveness and reducing layout overflow issues by restructuring the global header/layout, standardizing page headers, and ensuring wide tables scroll horizontally instead of breaking the page.
Changes:
- Added a dedicated mobile header layout with a back/menu control, centered title, and user menu.
- Made the main content area the primary vertical scroll container and adjusted width handling to reduce viewport overflow.
- Updated tables and page headers to behave better on small screens (horizontal table scrolling; hide redundant page titles on mobile).
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/routes/Header.svelte | Adds mobile-specific header UI (back/menu + centered label) and refactors user menu into a snippet. |
| src/routes/+layout.svelte | Adjusts root container width and makes <main> vertically scrollable. |
| src/routes/(app)/shockers/own/+page.svelte | Hides the page <h1> on small screens to avoid duplicating the mobile header title. |
| src/routes/(app)/shares/user/outgoing/+page.svelte | Switches table wrapper to horizontal overflow scrolling. |
| src/routes/(app)/shares/user/invites/+page.svelte | Switches both invites tables to horizontal overflow scrolling. |
| src/routes/(app)/shares/user/incoming/+page.svelte | Switches table wrapper to horizontal overflow scrolling. |
| src/routes/(app)/shares/public/[shareId=guid]/edit/+page.svelte | Reworks header area layout for better small-screen stacking and spacing. |
| src/routes/(app)/settings/sessions/+page.svelte | Migrates to PageHeader for consistent responsive headers/actions. |
| src/routes/(app)/settings/connections/+page.svelte | Migrates to PageHeader and adjusts layout wrapper away from Card.Content. |
| src/routes/(app)/settings/api-tokens/new/+page.svelte | Makes the card full-width (with max width) for small screens. |
| src/routes/(app)/settings/api-tokens/+page.svelte | Migrates to PageHeader and simplifies structure around the DataTable. |
| src/routes/(app)/profile/+page.svelte | Hides the page <h1> on small screens. |
| src/routes/(app)/hubs/[hubId=guid]/update/+page.svelte | Wraps update-history table in a horizontal overflow container. |
| src/routes/(app)/home/+page.svelte | Adjusts grids to show 2 columns on small screens where appropriate. |
| src/lib/components/Table/DataTableTemplate.svelte | Changes DataTable wrapper overflow behavior (now horizontal scrolling). |
| src/lib/components/PageHeader.svelte | Updates header layout to stack on mobile and hides the title on small screens. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let sidebar = useSidebar(); | ||
|
|
||
| let crumbs = $derived(breadcrumbs.state); | ||
| let currentLabel = $derived(crumbs[crumbs.length - 1]?.label ?? ''); |
| <a | ||
| href={prefixBase(parent.href)} | ||
| class="text-muted-foreground hover:text-foreground flex size-8 shrink-0 items-center justify-center" | ||
| title="Back to {parent.label}" | ||
| > |
| <Button | ||
| variant="ghost" | ||
| class="size-8 shrink-0" | ||
| title="Toggle Sidebar" |
| </script> | ||
|
|
||
| <div class={cn('overflow-y-auto rounded-md border', className)}> | ||
| <div class={cn('overflow-x-auto rounded-md border', className)}> |
| <Button variant="outline" href={resolve('/login')}>Login <LogIn /></Button> | ||
| <Button variant="outline" href={resolve('/signup')}>Sign Up <UserPlus /></Button> | ||
| <div class="hidden sm:flex sm:flex-row"> |
Replaces the data table on the sessions page with a card-based layout that works well on both mobile and desktop. Each card shows device type icon, parsed user agent, IP, and relative timestamps. Current session is visually distinguished with a green tint and "This device" badge. Also tightens mobile padding in Container from px-4 to px-2 globally.
# Conflicts: # src/lib/components/Container.svelte # src/lib/components/PageHeader.svelte # src/routes/(app)/settings/api-tokens/+page.svelte # src/routes/(app)/settings/connections/+page.svelte # src/routes/(app)/settings/sessions/+page.svelte # src/routes/(app)/settings/sessions/data-table-actions.svelte # src/routes/(app)/shares/user/incoming/+page.svelte # src/routes/(app)/shares/user/invites/+page.svelte # src/routes/(app)/shares/user/outgoing/+page.svelte # src/routes/Header.svelte
|
Ready to review this PR? Stage has broken it down into 6 individual chapters for you: Chapters generated by Stage for commit 7790abf on Jul 3, 2026 2:49pm UTC. |
…matting - prettier: instrumentation.server.ts (unformatted on develop, fails ci-lint there too) and session-card.svelte - eslint: justify the deliberate this-alias in AlsContextManager.bind
There was a problem hiding this comment.
🟡 Not ready to approve
It introduces confirmed functional regressions (DataTable vertical scrolling behavior, mobile theme toggle availability, and session timestamp updating) that should be fixed before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Suppressed comments (4)
src/lib/components/Table/DataTableTemplate.svelte:84
- Switching the table wrapper from
overflow-y-autoto onlyoverflow-x-autoremoves vertical scrolling for large tables; pages that rely on the DataTable being scrollable within a flex viewport (e.g.src/routes/(app)/shockers/logs/+page.svelteusesclass="min-h-0 flex-1") can now expand the table and push pagination/footer out of view.
<div class={cn('overflow-x-auto rounded-md border', className)}>
src/routes/Header.svelte:93
- The new mobile back control is an icon-only link; it should have an accessible label for screen readers (the
titleattribute is not a reliable accessible name).
<a
href={prefixBase(parent.href)}
class="text-muted-foreground hover:text-foreground flex size-8 shrink-0 items-center justify-center"
title="Back to {parent.label}"
>
<ChevronLeft size={24} />
</a>
src/routes/Header.svelte:107
- The mobile header (
sm:hidden) no longer renders theLightSwitch, so users on small screens have no way to toggle theme (regression vs the previous unified header).
<span class="flex-1 truncate text-center text-sm font-semibold">{currentLabel}</span>
{@render userMenu()}
src/routes/(app)/settings/sessions/session-card.svelte:53
nowis defined as$derived(Temporal.Now.instant())with no reactive dependencies, so it will be computed once and never update; as a result the "Last seen / Created / Expires" relative timestamps will stay frozen even though the page sets an interval to refresh them.
const now = $derived(Temporal.Now.instant());
- Files reviewed: 19/19 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
There was a problem hiding this comment.
🟡 Not ready to approve
It introduces at least two confirmed functional issues (broken avatar alt interpolation and session relative timestamps not updating over time).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Suppressed comments (2)
src/routes/(app)/settings/sessions/session-card.svelte:53
nowis derived fromTemporal.Now.instant()without any reactive dependencies, so it will be computed once and the relative timestamps (created/last seen/expires) won’t update over time even though the parent page is trying to refresh them periodically.
const now = $derived(Temporal.Now.instant());
// Past events: durationBetween(now, pastInstant) → negative → "X ago"
const lastSeenText = $derived(
session.lastUsed ? formatElapsed(durationBetween(now, session.lastUsed)) : 'Never'
);
const createdText = $derived(formatElapsed(durationBetween(now, session.created)));
// Future event: durationBetween(now, futureInstant) → positive → "in X"
const expiresText = $derived(formatElapsed(durationBetween(now, session.expires)));
src/routes/Header.svelte:58
altis currently a quoted string, so it will render literally as "{userState.self.name}'s avatar" instead of interpolating the user name.
<img
class="inline-block h-8 rounded-full"
src={userState.self.avatar}
alt="{userState.self.name}'s avatar"
/>
- Files reviewed: 19/19 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ❌ Deployment failed View logs |
openshock-app | b669e65 | Aug 04 2026, 11:20 AM |
This is a huge PR, needs much consideration before merging...