Feature/breadcrumb cleanup by LucHeart · Pull Request #182 · OpenShock/Frontend

LucHeart · 2026-03-31T23:05:36Z

No description provided.

Resolve conflicts: keep breadcrumb additions, adopt develop's refactored UI (empty states, grid layout, module script blocks). Remove duplicate imports and stale breadcrumb calls. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

cloudflare-workers-and-pages · 2026-03-31T23:07:46Z

Deploying openshockapp with Cloudflare Pages

Latest commit:	`3a994fa`
Status:	✅ Deploy successful!
Preview URL:	https://b6a6416c.openshockapp.pages.dev
Branch Preview URL:	https://feature-breadcrumb-cleanup.openshockapp.pages.dev

View logs

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 64a969dd50

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-03-31T23:11:55Z

src/routes/shares/public/[shareId=guid]/ControlView.svelte

+      {#if userState.self}
+        <div class="flex justify-end p-2">
+          <Button variant="outline" href={editUrl}><Pencil /> Edit</Button>


Restrict public-share edit CTA to the share owner

The new Edit button is rendered for any authenticated user (userState.self) on a public share, so logged-in visitors who do not own that share are still directed into the edit workflow. In that scenario, they can reach owner-only management screens and hit authorization failures on save calls, which is confusing and avoidable; the CTA should be gated by ownership (or equivalent edit permission), not just login state.

Useful? React with 👍 / 👎.

Copilot

Pull request overview

This PR refactors the app’s breadcrumb system from imperative breadcrumbs.push(...) calls to a declarative registerBreadcrumbs(() => [...]) API, and updates the header/breadcrumb UI to match the new model.

Changes:

Replace direct breadcrumb mutation with registerBreadcrumbs registrations across many routes/layouts.
Update breadcrumb rendering to use { label, href } entries and centralized state aggregation.
Add a “View” link on the public share edit page and adjust header layout to include a vertical divider next to breadcrumbs.

Reviewed changes

Copilot reviewed 28 out of 28 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/routes/shares/public/[shareId=guid]/+page.svelte	Registers breadcrumbs for public-share detail view and updates state to populate crumb label from fetched share data.
src/routes/Header.svelte	Layout tweak for header and breadcrumb placement; adds a vertical separator.
src/routes/Breadcrumb.svelte	Updates breadcrumb rendering for new `{label, href}` model and separator behavior.
src/routes/(app)/shockers/shared/+page.svelte	Registers “Shared Shockers” breadcrumbs via new API.
src/routes/(app)/shockers/own/+page.svelte	Replaces `breadcrumbs.push` usage with `registerBreadcrumbs`.
src/routes/(app)/shockers/logs/+page.svelte	Registers “Shocker Logs” breadcrumb.
src/routes/(app)/shockers/logs/[shockerId=guid]/+page.svelte	Registers breadcrumb trail for logs details page.
src/routes/(app)/shockers/[shockerId=guid]/edit/+page.svelte	Registers breadcrumbs for shocker edit page using shocker name when available.
src/routes/(app)/shares/user/+layout.svelte	Registers “User Shares” layout breadcrumb.
src/routes/(app)/shares/public/+page.svelte	Registers “Public Shares” breadcrumb.
src/routes/(app)/shares/public/[shareId=guid]/edit/+page.svelte	Registers breadcrumbs for public share edit and adds “View” button linking to the public page.
src/routes/(app)/settings/sessions/+page.svelte	Registers Settings → Sessions breadcrumb trail.
src/routes/(app)/settings/connections/+page.svelte	Registers Settings → Connections breadcrumb trail.
src/routes/(app)/settings/api-tokens/new/+page.svelte	Registers Settings → API Tokens → New breadcrumb trail.
src/routes/(app)/settings/api-tokens/+page.svelte	Registers Settings → API Tokens breadcrumb trail.
src/routes/(app)/settings/account/+page.svelte	Registers Settings → Account breadcrumb trail.
src/routes/(app)/profile/+page.svelte	Adds a new Profile route that currently only registers breadcrumbs.
src/routes/(app)/hubs/+page.svelte	Replaces `breadcrumbs.push` usage with `registerBreadcrumbs` for hubs page.
src/routes/(app)/hubs/[hubId=guid]/update/+page.svelte	Replaces `breadcrumbs.push` usage with `registerBreadcrumbs` for hub update flow.
src/routes/(app)/home/+page.svelte	Replaces `breadcrumbs.push` usage with `registerBreadcrumbs` for home page.
src/routes/(app)/admin/webhooks/+page.svelte	Registers “Webhooks” breadcrumb.
src/routes/(app)/admin/users/+page.svelte	Registers “Users” breadcrumb.
src/routes/(app)/admin/users/[userId]/+page.svelte	Registers breadcrumb trail for admin user detail page.
src/routes/(app)/admin/online-hubs/+page.svelte	Registers “Online Hubs” breadcrumb.
src/routes/(app)/admin/config/+page.svelte	Registers “Config” breadcrumb.
src/routes/(app)/admin/blacklists/+page.svelte	Registers “Blacklists” breadcrumb.
src/routes/(app)/admin/+layout.svelte	Registers “Admin” layout breadcrumb.
src/lib/state/breadcrumbs-state.svelte.ts	Introduces slot-based breadcrumb aggregation and the `registerBreadcrumbs` API.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-01T05:14:10Z

src/routes/Breadcrumb.svelte

+          {#if crumb.href && i < breadcrumbs.state.length - 1}
+            <Breadcrumb.Link href={crumb.href}>{crumb.label}</Breadcrumb.Link>
          {:else}


Breadcrumb links are rendered as plain values, but the href is passed through as a raw pathname. Since the app can be deployed with a non-empty SvelteKit paths.base (see prefixBase() usage elsewhere), these breadcrumb links will navigate to the wrong location unless the base path is prefixed. Consider applying prefixBase (or otherwise resolving the base) before passing crumb.href to Breadcrumb.Link.

Copilot · 2026-04-01T05:14:10Z

src/routes/Breadcrumb.svelte

+  <Breadcrumb.Root class="shrink-0">
    <Breadcrumb.List>
-      {#each breadcrumbs.state as crumb, index (crumb.href)}
+      {#each breadcrumbs.state as crumb, i (i)}


The breadcrumb list is keyed by array index ((i)), which can cause incorrect DOM reuse if breadcrumbs are inserted/removed (e.g., switching between routes that add/remove intermediate crumbs). Prefer a stable key derived from crumb identity (e.g., href+label) or add an explicit id to breadcrumb entries.

Suggested change

{#each breadcrumbs.state as crumb, i (i)}

{#each breadcrumbs.state as crumb, i (crumb.href || crumb.label)}

Copilot · 2026-04-01T05:14:11Z

src/lib/state/breadcrumbs-state.svelte.ts

+  onMount(() => () => {
+    const idx = _slots.findIndex((s) => s.id === id);
+    if (idx !== -1) _slots.splice(idx, 1);
+  });


registerBreadcrumbs removes its slot via an onMount teardown. Because the slot is pushed immediately when registerBreadcrumbs() is called, it’s safer to tie removal to component destruction (onDestroy or an $effect cleanup) so slots are cleaned up even if the component never mounts (SSR, early errors) and to avoid potential memory leaks/stale crumbs.

Copilot · 2026-04-01T05:14:11Z

src/routes/(app)/profile/+page.svelte

+  import { registerBreadcrumbs } from '$lib/state/breadcrumbs-state.svelte';
+
+  registerBreadcrumbs(() => [{ label: 'Profile' }]);
+</script>


This adds a new /profile route that renders no UI (only registers breadcrumbs). If users can navigate here from the Header menu, they’ll land on a blank page. Consider adding at least basic content/placeholder, or redirecting to an existing profile/settings page until the profile UI is implemented.

Suggested change

</script>

</script>

<main>

<h1>Profile</h1>

<p>Your profile page is coming soon.</p>

</main>

Copilot · 2026-04-01T05:14:11Z

src/routes/Header.svelte

  import { Button } from '$lib/components/ui/button';
  import * as DropdownMenu from '$lib/components/ui/dropdown-menu';
-  import { useSidebar } from '$lib/components/ui/sidebar';
+  import { Separator, useSidebar } from '$lib/components/ui/sidebar';


Header is importing Separator from the sidebar UI module, which applies sidebar-specific styling (e.g., bg-sidebar-border, mx-2). For a header divider, consider using the generic separator component ($lib/components/ui/separator) to avoid coupling header styling to sidebar theme tokens.

Suggested change

import { Separator, useSidebar } from '$lib/components/ui/sidebar';

import { useSidebar } from '$lib/components/ui/sidebar';

import { Separator } from '$lib/components/ui/separator';

LucHeart and others added 5 commits March 19, 2026 22:07

breadcrumb cleanup

66a12a6

fix format...

b1473b2

add breadcrumb stuff, and edit button to public shares

a99aa45

add share copy input

6b1fdf5

chatgpt-codex-connector bot reviewed Mar 31, 2026

View reviewed changes

LucHeart and others added 5 commits April 1, 2026 01:20

format

a5e2ee9

Merge branch 'develop' into feature/breadcrumb-cleanup

5e00660

yeet

0730ddd

Improve breadcrumb store

ec88a7c

Update +page.svelte

3a994fa

hhvrc requested a review from Copilot April 1, 2026 05:07

Copilot started reviewing on behalf of hhvrc April 1, 2026 05:08 View session

Copilot AI reviewed Apr 1, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/breadcrumb cleanup#182

Feature/breadcrumb cleanup#182
LucHeart wants to merge 10 commits intodevelopfrom
feature/breadcrumb-cleanup

LucHeart commented Mar 31, 2026

Uh oh!

cloudflare-workers-and-pages bot commented Mar 31, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector bot left a comment

Uh oh!

chatgpt-codex-connector bot Mar 31, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 1, 2026

Uh oh!

Copilot AI Apr 1, 2026

Uh oh!

Copilot AI Apr 1, 2026

Uh oh!

Copilot AI Apr 1, 2026

Uh oh!

Copilot AI Apr 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	{#each breadcrumbs.state as crumb, i (i)}
	{#each breadcrumbs.state as crumb, i (crumb.href \|\| crumb.label)}

	import { Separator, useSidebar } from '$lib/components/ui/sidebar';
	import { useSidebar } from '$lib/components/ui/sidebar';
	import { Separator } from '$lib/components/ui/separator';

Conversation

LucHeart commented Mar 31, 2026

Uh oh!

cloudflare-workers-and-pages bot commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deploying openshockapp with Cloudflare Pages

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cloudflare-workers-and-pages bot commented Mar 31, 2026 •

edited

Loading