Uh oh!
There was an error while loading. Please reload this page.
fix: Improve mobile layout and responsiveness across multiple pages - #211
Draft
hhvrc wants to merge 14 commits into
Draft
fix: Improve mobile layout and responsiveness across multiple pages#211hhvrc wants to merge 14 commits into
hhvrc wants to merge 14 commits into
Conversation
# Conflicts: # src/routes/(app)/profile/+page.svelte
Contributor
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 ?? ''); |
Comment on lines
+87
to
+91
| <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)}> |
Comment on lines
+70
to
+72
| <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
ghost
commented
Jul 3, 2026
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
Contributor
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.
Contributor
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 file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a huge PR, needs much consideration before merging...