fix(396): mark active route in a default-collapsed group; fix mobile brand bar
Build ErsatzTV Image / CI image pin matches docker/ci (pull_request) Successful in 6s
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 7s
Build ErsatzTV Image / decisions.md append-only (pull_request) Successful in 6s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Has been cancelled
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Has been cancelled
Build ErsatzTV Image / Functional E2E (curl contracts) (pull_request) Has been cancelled
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been cancelled
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Has been cancelled
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Has been cancelled
Build ErsatzTV Image / CI image pin matches docker/ci (pull_request) Successful in 6s
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 7s
Build ErsatzTV Image / decisions.md append-only (pull_request) Successful in 6s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Has been cancelled
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Has been cancelled
Build ErsatzTV Image / Functional E2E (curl contracts) (pull_request) Has been cancelled
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been cancelled
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Has been cancelled
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Has been cancelled
Cold-review findings: - #1 (CONFIRMED): deep-linking/reloading into a route inside a default-collapsed labeled group (e.g. /app/settings, /app/libraries) left the expanded sidebar with no active indicator, violating "active route marked in both states". Now the group that contains the active route is force-shown (and its header shows expanded) WITHOUT persisting — navigating away reverts to the stored preference. Regression test added. - #2 (mobile): a persisted-collapsed state hid the brand logo+wordmark while the toggle is also hidden at <=980px, leaving an empty header. Restore the brand in the mobile query (collapse is desktop-only). Verified live (Playwright): /app/settings deep-link shows Settings active + aria-current, System auto-revealed, Media stays collapsed, nothing persisted. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -720,6 +720,22 @@ describe('ChicoryTV SPA scaffold', () => {
|
||||
expect(container.querySelector('.ctv-app-shell')).toHaveClass('ctv-app-shell-collapsed');
|
||||
expect(screen.getByRole('button', { name: 'Expand sidebar' })).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('reveals + marks the active route even when its group is default-collapsed, without persisting', () => {
|
||||
// Deep-link into a route inside the default-collapsed System group (expanded sidebar).
|
||||
window.history.replaceState(null, '', '/app/settings');
|
||||
|
||||
render(<App />);
|
||||
|
||||
// The active item is rendered and marked, and its group header shows expanded…
|
||||
const settingsLink = screen.getByRole('link', { name: 'Settings' });
|
||||
expect(settingsLink).toHaveAttribute('aria-current', 'page');
|
||||
expect(screen.getByRole('button', { name: 'System', expanded: true })).toBeInTheDocument();
|
||||
// …but a sibling default-collapsed group (Media) stays collapsed…
|
||||
expect(screen.queryByRole('link', { name: 'Libraries' })).not.toBeInTheDocument();
|
||||
// …and the reveal did NOT write a stored preference (it reverts on navigating away).
|
||||
expect(window.localStorage.getItem('ctv-sidebar-groups')).toBeNull();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -177,7 +177,14 @@ function Sidebar({
|
||||
// Accordions only apply in the expanded sidebar; the rail always shows every item
|
||||
// (separated by a hairline divider), so group-collapse is ignored there.
|
||||
const groupCollapsed = isGroupCollapsed(groupKey);
|
||||
const showItems = collapsed || !groupCollapsed;
|
||||
// Always reveal the group that contains the active route so its active item is visible +
|
||||
// highlighted in the expanded sidebar, even when the group's stored state is collapsed
|
||||
// (e.g. a fresh deep-link to /app/settings). This does NOT persist — navigating away
|
||||
// reverts to the stored preference (acceptance: "active route marked in both states").
|
||||
const containsActive =
|
||||
activeRoute != null && ids.includes(activeRoute.id);
|
||||
const expanded = containsActive || !groupCollapsed;
|
||||
const showItems = collapsed || expanded;
|
||||
|
||||
return (
|
||||
<Fragment key={groupKey}>
|
||||
@@ -187,7 +194,7 @@ function Sidebar({
|
||||
<button
|
||||
type="button"
|
||||
className="ctv-nav-group-header"
|
||||
aria-expanded={!groupCollapsed}
|
||||
aria-expanded={expanded}
|
||||
onClick={() => onToggleGroup(groupKey)}
|
||||
>
|
||||
<span>{label}</span>
|
||||
|
||||
@@ -3816,6 +3816,19 @@ body {
|
||||
display: none;
|
||||
}
|
||||
|
||||
/* The collapse feature is desktop-only (the nav is hidden here). If the sidebar was collapsed on
|
||||
desktop, don't let the rail's brand-hiding leave an empty header on mobile — restore the brand
|
||||
(#396). */
|
||||
.ctv-app-shell-collapsed .ctv-brand {
|
||||
padding: 0 14px;
|
||||
justify-content: flex-start;
|
||||
}
|
||||
|
||||
.ctv-app-shell-collapsed .ctv-brand img,
|
||||
.ctv-app-shell-collapsed .ctv-brand-wordmark {
|
||||
display: revert;
|
||||
}
|
||||
|
||||
.ctv-topbar {
|
||||
flex-wrap: wrap;
|
||||
height: auto;
|
||||
|
||||
Reference in New Issue
Block a user