Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,6 @@
let collapsed = $state(false);
let mobileOpen = $state(false);
let isDesktop = $state(true);
// Closed to start with, leaving Manage Hackathon alone under the heading: a
// participant spine plus one way in, not a second nav half again as long as
// the first on every page whether or not the organiser came to manage
// anything. Ten organiser entries sat flat under the heading before this.
let manageOpen = $state(false);

// `collapsed` is a desktop-only preference (persisted below); on a narrow
// viewport the drawer must always render fully expanded regardless of it.
Expand All @@ -66,30 +61,13 @@
// Manage Pages.
const activeId = $derived(activeNavId($page.url.pathname, [...items, ...manageItems]));

// The hub stays on the rail whatever the fold state and carries the
// disclosure for the rest — it is how the section is entered.
const manageHubItem = $derived(manageItems.find((i) => i.id === 'manage:hackathon'));
const manageSubItems = $derived(manageItems.filter((i) => i.id !== 'manage:hackathon'));

// The hub counts as inside, not just the screens under it: opening it is how
// an organiser goes looking for the rest.
const insideManage = $derived(activeId?.startsWith('manage:') ?? false);

// `membership.role` is sourced from casbin. It is absent for a global admin who
// never joined, hence the second argument.
const badge = $derived(hackathonRoleBadge(membership ?? undefined, isGlobalAdmin));

$effect(() => {
if (typeof localStorage === 'undefined') return;
collapsed = localStorage.getItem('sidebar-collapsed') === 'true';
manageOpen = localStorage.getItem('sidebar-manage-open') === 'true';
});

// Entering the section opens it, rather than pinning it open while you are in
// there: deriving the fold state from the route instead made the chevron a
// no-op on every Manage page, which reads as a broken control.
$effect(() => {
if (insideManage) manageOpen = true;
});

$effect(() => {
Expand Down Expand Up @@ -129,11 +107,6 @@
collapsed = !collapsed;
remember('sidebar-collapsed', collapsed);
}

function toggleManage() {
manageOpen = !manageOpen;
remember('sidebar-manage-open', manageOpen);
}
</script>

<!-- Off-canvas on mobile, so the hackathon's nav needs its own trigger. The bar
Expand Down Expand Up @@ -239,10 +212,10 @@
<SidebarNavSection {items} {activeId} collapsed={effectiveCollapsed} />

<!-- Organiser-only. The heading is what makes the difference legible, so
unlike the section above this one needs it. Manage Hackathon is
always drawn; the ten screens it leads to are disclosed from it,
which is why they are not a second nav twice the length of the
participant spine on every page.
unlike the section above this one needs it. Flat, not folded: every
entry manageNav returns is drawn plainly, so this is a fixed-height
list rather than one whose height depends on whether the section
happens to be open.

Guarded here rather than left to SidebarNavSection, which keeps a
labelled empty section on purpose so a role chip has somewhere to
Expand All @@ -256,13 +229,10 @@
{#if manageItems.length > 0}
<SidebarNavSection
label="Manage"
parentItem={manageHubItem}
items={manageSubItems}
items={manageItems}
{activeId}
collapsed={effectiveCollapsed}
accent="tertiary"
open={manageOpen}
onToggle={toggleManage}
/>
{/if}
</nav>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { cleanup, fireEvent, render, screen } from "@testing-library/svelte"
import { cleanup, render, screen } from "@testing-library/svelte"
import { tick } from "svelte"
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"

Expand All @@ -17,11 +17,14 @@ const OWNER = 1
const MEMBER = 2

/*
* The disclosure rule, as against how one section renders
* (SidebarNavSection.test.ts): Manage Hackathon is always on the rail, the ten
* screens under it are not, arriving anywhere in the section brings them out,
* and the chevron works wherever you are — main's earlier version derived the
* state from the route and so dead-ended on every Manage page.
* The Manage section is a flat, always-expanded list (see
* SidebarNavSection.test.ts for how one section renders): every organiser
* entry manageNav returns is drawn on every page, not disclosed behind a
* fold. An earlier version folded the section under Manage Hackathon and
* force-opened it on every /manage/* route, which made the sidebar taller
* than its fixed, viewport-relative height and forced it to scroll — flat
* avoids that, since the rendered height no longer depends on which page
* you're on.
*/

const setPath = (pathname: string) =>
Expand Down Expand Up @@ -49,17 +52,16 @@ async function renderAt(pathname: string, overrides = {}) {
const hub = () => screen.queryByRole("link", { name: "Manage Hackathon" })
const subEntry = () => screen.queryByRole("link", { name: "Manage Teams" })

// Every entry this branch has that main does not. They are the reason the fold
// exists here at all — ten organiser rows under a five-row participant spine —
// and the reason to name them one by one is that losing any of them to the port
// is the failure this whole change had to avoid.
// Every entry this branch has that main does not — named individually so a
// future refactor that drops one turns red rather than merely shrinking a
// count nobody reads back. "New Phase" is deliberately not here: it's a tile
// on the manage hub page, not one of manageNav's sidebar entries.
const OURS_ONLY = [
"Prizes",
"Deadlines",
"Manage Forms",
"Notifications",
"Invitation Links",
"New Phase",
]

afterEach(cleanup)
Expand All @@ -78,7 +80,7 @@ beforeEach(() => {
})

// jsdom ships no matchMedia, and the component asks for one on mount to tell a
// desktop rail from the mobile drawer. Desktop is where the disclosure lives.
// desktop rail from the mobile drawer.
window.matchMedia = (query: string) =>
({
matches: true,
Expand All @@ -93,56 +95,38 @@ beforeEach(() => {
})

describe("HackathonSidebar's Manage section", () => {
it("shows only Manage Hackathon on a participant page", async () => {
it("shows every Manage entry on a participant page, not just the hub", async () => {
await renderAt("/my/hackathon/h1/overview")

expect(hub()).toBeInTheDocument()
expect(subEntry()).toBeNull()
})

// The hub counts as entering the section: whoever opens it is looking for what
// it leads to.
it("shows the rest once Manage Hackathon itself is open", async () => {
await renderAt("/my/hackathon/h1/manage")

expect(subEntry()).toBeInTheDocument()
})

it("shows the rest on one of the screens under it", async () => {
it("shows every Manage entry on one of the screens under it too", async () => {
await renderAt("/my/hackathon/h1/teams/manage")

expect(hub()).toBeInTheDocument()
expect(subEntry()).toBeInTheDocument()
})

// Nested under /manage, so `activeNavId`'s longest-prefix match keeps the hub
// lit — which is also what counts as "inside" and brings the section out.
it("counts the edit form nested under it as inside the section", async () => {
// lit as the current page even though the list beneath it is flat.
it("counts the edit form nested under it as the hub's own active page", async () => {
await renderAt("/my/hackathon/h1/manage/edit")

expect(subEntry()).toBeInTheDocument()
expect(hub()).toHaveAttribute("aria-current", "page")
})

it("opens from the chevron on a participant page", async () => {
// The regression this reverts: a fold control that force-opened on every
// Manage page anyway, so it existed only to make the section taller than
// the sidebar's fixed height on the pages it mattered on.
it("draws no fold control at all", async () => {
await renderAt("/my/hackathon/h1/overview")

await fireEvent.click(
screen.getByRole("button", { name: /Show Manage Hackathon/ }),
)

expect(subEntry()).toBeInTheDocument()
})

// The regression main hit: pinned open inside Manage, this click did nothing.
it("closes again from the chevron while inside the section", async () => {
await renderAt("/my/hackathon/h1/manage")

await fireEvent.click(
screen.getByRole("button", { name: /Hide Manage Hackathon/ }),
)

expect(subEntry()).toBeNull()
expect(hub()).toBeInTheDocument()
expect(
screen.queryByRole("button", { name: /Manage Hackathon/ }),
).toBeNull()
})

it("gives a plain member no Manage section at all", async () => {
Expand All @@ -155,16 +139,16 @@ describe("HackathonSidebar's Manage section", () => {
})

// The port's own failure mode, asserted by name rather than by count: main's
// Manage section has none of these, and folding the section is exactly the
// kind of change that could drop one without anything turning red.
// Manage section has none of these, and a future refactor of this list is
// exactly the kind of change that could drop one without anything turning red.
it("keeps every entry this branch has that main does not", async () => {
await renderAt("/my/hackathon/h1/manage")

for (const label of OURS_ONLY) {
expect(
screen.queryByRole("link", { name: label }),
`"${label}" is one of the organiser entries main's Manage panel does ` +
`not have — the port must not lose it`,
`not have — it must not be lost`,
).toBeInTheDocument()
}
})
Expand Down
Loading