Skip to content

Commit 711c5f8

Browse files
committed
fix: montage edit-mode reset and remaining all-mode nits (refs #337)
Montage: isEditMode was not scoped to isAllMode. Switching into All mode with it left on from single mode stranded the grid in edit mode with no way to turn it off (the toggle disables in All mode), so drag/resize handlers stayed live but no-op. Reset during render (React's "adjusting state when a prop changes" pattern, same idiom as LiveActivitySettingsDialog's useClampedNumberField) rather than in an Effect, which would paint one extra frame with the stale edit-mode UI. Montage: the kebab menu's hidden-monitors list regressed from the full monitor list to the group-filtered one. A monitor hidden while outside the active group filter became permanently un-hideable. Restored a separate, never-group-filtered `enabledMonitors` for the kebab. MonitorWidget: SingleMonitor built LiveMonitorPlayer without its profileId prop, so the go2rtc failure cache / MJPEG token resolution fell back to the globally-selected profile instead of the tile's owning one in All mode. One line per LiveMonitorPlayer call site (both branches - the test caught the first fix only covering one). EventsWidget: isAllMode was inferred from profiles.length > 1, which silently flips to single-mode behavior (no chips, no /all deep links) once a delete brings the All-mode scope down to one profile while mode is still 'all'. Uses scope?.mode === 'all' instead.
1 parent 60bf255 commit 711c5f8

6 files changed

Lines changed: 140 additions & 15 deletions

File tree

app/src/components/dashboard/widgets/EventsWidget.tsx

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,11 @@ export const EventsWidget = memo(function EventsWidget({
6464
const bandwidth = useBandwidthSettings();
6565
const scope = useProfileScope();
6666
const profiles = scope?.profiles ?? [];
67-
// profiles.length > 1 only happens in All mode (useProfileScope's single
68-
// mode always returns exactly one profile) - a chip-worthy signal with
69-
// no separate isAllMode branch needed.
70-
const isAllMode = profiles.length > 1;
67+
// scope.mode, not profiles.length > 1: a single remaining profile after
68+
// deleting down to one WHILE still in All mode must keep chips/deep-links
69+
// (profiles.length > 1 collapses to the single-mode branch there, refs
70+
// #337).
71+
const isAllMode = scope?.mode === 'all';
7172
const monitorIdFilter = monitorIds?.length ? monitorIds.join(',') : undefined;
7273
const refetchMs = refreshInterval ?? bandwidth.eventsWidgetInterval;
7374

app/src/components/dashboard/widgets/MonitorWidget.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ function SingleMonitor({ monitorId, objectFit, profileId }: { monitorId: string;
9191
<LiveMonitorPlayer
9292
monitor={monitor.Monitor}
9393
profile={currentProfile}
94+
profileId={profileId}
9495
className="w-full h-full"
9596
objectFit={objectFit}
9697
onProtocolChange={setProtocol}
@@ -100,6 +101,7 @@ function SingleMonitor({ monitorId, objectFit, profileId }: { monitorId: string;
100101
<LiveMonitorPlayer
101102
monitor={monitor.Monitor}
102103
profile={currentProfile}
104+
profileId={profileId}
103105
className="w-full h-full"
104106
objectFit={objectFit}
105107
onProtocolChange={setProtocol}

app/src/components/dashboard/widgets/__tests__/EventsWidget.test.tsx

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { describe, it, expect, vi, beforeEach } from 'vitest';
2-
import { render, screen, waitFor } from '@testing-library/react';
3-
import { MemoryRouter } from 'react-router-dom';
2+
import { render, screen, waitFor, fireEvent } from '@testing-library/react';
3+
import { MemoryRouter, Routes, Route } from 'react-router-dom';
44
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
55
import { EventsWidget } from '../EventsWidget';
66
import { useProfileScope } from '../../../../hooks/useProfileScope';
@@ -55,11 +55,11 @@ function clientFor(id: string) {
5555
return { profile: id } as unknown as import('../../../../api/client').ApiClient;
5656
}
5757

58-
function mockScope(profiles: Array<typeof profileA>) {
59-
const mode = profiles.length > 1 ? 'all' : 'single';
58+
function mockScope(profiles: Array<typeof profileA>, mode?: 'single' | 'all') {
59+
const resolvedMode = mode ?? (profiles.length > 1 ? 'all' : 'single');
6060
vi.mocked(useProfileScope).mockReturnValue({
61-
mode,
62-
profile: mode === 'single' ? profiles[0] : null,
61+
mode: resolvedMode,
62+
profile: resolvedMode === 'single' ? profiles[0] : null,
6363
profiles,
6464
settings: {},
6565
} as never);
@@ -87,6 +87,35 @@ describe('EventsWidget', () => {
8787
}));
8888
});
8989

90+
// profiles.length > 1 as the All-mode signal breaks the moment a delete
91+
// brings the scope down to one profile while still IN All mode (mode
92+
// stays 'all', but the count-based heuristic silently flips to single-mode
93+
// behavior): chips disappear and links stop deep-linking through /all
94+
// (refs #337, final fix wave).
95+
it('a single remaining profile in All mode still chips and deep-links via /all (refs #337)', async () => {
96+
mockScope([profileA], 'all');
97+
vi.mocked(getEvents).mockResolvedValue({
98+
events: [event('1', 'Front Door A', '2026-08-03 10:00:00')],
99+
} as never);
100+
101+
render(
102+
<QueryClientProvider client={new QueryClient({ defaultOptions: { queries: { retry: false } } })}>
103+
<MemoryRouter initialEntries={['/dashboard']}>
104+
<Routes>
105+
<Route path="/dashboard" element={<EventsWidget />} />
106+
<Route path="/all/events/:profileId/:eventId" element={<div data-testid="landed-all-route" />} />
107+
</Routes>
108+
</MemoryRouter>
109+
</QueryClientProvider>
110+
);
111+
112+
await waitFor(() => expect(screen.getByText('Front Door A')).toBeInTheDocument());
113+
expect(screen.getByTestId('widget-profile-chip')).toHaveTextContent('Home');
114+
115+
fireEvent.click(screen.getByText('Front Door A'));
116+
expect(screen.getByTestId('landed-all-route')).toBeInTheDocument();
117+
});
118+
90119
it('All mode: aggregates both profiles\' events with a profile chip per row (refs #337)', async () => {
91120
mockScope([profileA, profileB]);
92121
vi.mocked(getEvents).mockImplementation(async (client) => {

app/src/components/dashboard/widgets/__tests__/MonitorWidget.test.tsx

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@ vi.mock('../../../../api/monitors', () => ({
2222
getMonitors: vi.fn(),
2323
}));
2424
vi.mock('../../../monitors/LiveMonitorPlayer', () => ({
25-
LiveMonitorPlayer: () => <div data-testid="live-player" />,
25+
LiveMonitorPlayer: ({ profileId }: { profileId?: string }) => (
26+
<div data-testid="live-player" data-profile-id={profileId ?? ''} />
27+
),
2628
}));
2729
vi.mock('../../../monitors/MonitorHoverPreview', () => ({
2830
MonitorHoverPreview: ({ children }: { children: React.ReactNode }) => <>{children}</>,
@@ -92,5 +94,9 @@ describe('MonitorWidget', () => {
9294
expect((client as unknown as { profile: string }).profile).toBe(profileB.id);
9395

9496
await waitFor(() => expect(screen.getByTestId('widget-profile-chip')).toHaveTextContent('Work'));
97+
// LiveMonitorPlayer's own profileId prop (distinct from `profile`) scopes
98+
// its go2rtc failure cache / MJPEG token resolution to the OWNING
99+
// profile, not whichever profile is globally selected (refs #337).
100+
expect(screen.getByTestId('live-player')).toHaveAttribute('data-profile-id', profileB.id);
95101
});
96102
});

app/src/pages/Montage.tsx

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,16 @@ export default function Montage() {
124124
return counts;
125125
}, [scopedMonitors]);
126126

127+
// The full monitor list, NEVER group-filtered: the kebab menu's hidden-
128+
// monitors list must be able to un-hide any monitor regardless of which
129+
// group filter is currently active, or a monitor hidden while outside the
130+
// active group becomes permanently un-hideable (refs #337 single-mode
131+
// regression - `monitors` below is group-filtered).
132+
const enabledMonitors = useMemo(
133+
(): MonitorData[] => scopedMonitors.map((s) => ({ Monitor: s.item.Monitor, Monitor_Status: s.item.Monitor_Status })),
134+
[scopedMonitors]
135+
);
136+
127137
const monitors = useMemo((): MontageTileItem[] => {
128138
if (isAllMode) {
129139
// Group filter is current-profile-scoped (Monitors.tsx precedent) - All
@@ -187,6 +197,21 @@ export default function Montage() {
187197
// Edit mode state lifted to page level
188198
const [isEditMode, setIsEditMode] = useState(false);
189199

200+
// Editing is single-mode-only (the toggle is disabled in All mode below),
201+
// but isEditMode itself is not scoped to isAllMode - switching into All
202+
// mode with it left on from single mode stranded the grid in edit mode
203+
// with no way to turn it off (the toggle stays disabled), so drag/resize
204+
// handlers were live but no-op (refs #337). Reset during render rather
205+
// than in an Effect (React's documented "adjusting state when a prop
206+
// changes" pattern - see LiveActivitySettingsDialog's useClampedNumberField
207+
// for the same idiom): an Effect would paint one extra frame with the
208+
// stale edit-mode UI before correcting it.
209+
const [lastIsAllMode, setLastIsAllMode] = useState(isAllMode);
210+
if (isAllMode !== lastIsAllMode) {
211+
setLastIsAllMode(isAllMode);
212+
if (isAllMode) setIsEditMode(false);
213+
}
214+
190215
// Active saved layout name (persisted in settings)
191216
const activeLayoutName = bucket.activeLayoutName;
192217

@@ -583,7 +608,7 @@ export default function Montage() {
583608
</Button>
584609
<RefreshButton size="sm" className="h-8 sm:h-9" data-testid="montage-refresh-button" />
585610
<MontageKebabMenu
586-
monitors={monitors.map((m) => m.Monitor)}
611+
monitors={enabledMonitors.map((m) => m.Monitor)}
587612
hiddenMonitorIds={bucket.hiddenMonitorIds}
588613
onToggleVisibility={handleToggleMonitorVisibility}
589614
/>

app/src/pages/__tests__/Montage.test.tsx

Lines changed: 65 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,14 @@
88
* strips. Single-mode assertions guard the byte-identical requirement.
99
*/
1010
import { describe, expect, it, vi, beforeEach } from 'vitest';
11-
import { render, screen } from '@testing-library/react';
11+
import { render, screen, fireEvent } from '@testing-library/react';
1212
import Montage from '../Montage';
1313

1414
const useScopedMonitorsMock = vi.fn();
1515
const useCurrentProfileMock = vi.fn();
1616
const useProfileScopeMock = vi.fn();
1717
const useMontageGridMock = vi.fn();
18+
const useGroupFilterMock = vi.fn();
1819

1920
vi.mock('../../hooks/useScopedMonitors', () => ({
2021
useScopedMonitors: () => useScopedMonitorsMock(),
@@ -29,7 +30,7 @@ vi.mock('../../hooks/useProfileScope', () => ({
2930
}));
3031

3132
vi.mock('../../hooks/useGroupFilter', () => ({
32-
useGroupFilter: () => ({ isFilterActive: false, filteredMonitorIds: [], isFilterReady: true }),
33+
useGroupFilter: () => useGroupFilterMock(),
3334
}));
3435

3536
vi.mock('../../hooks/useMontageGroupState', () => ({
@@ -70,7 +71,9 @@ vi.mock('../../components/montage', async (importOriginal) => {
7071
...actual,
7172
GridLayoutControls: () => <div data-testid="grid-layout-controls-stub" />,
7273
FullscreenControls: () => <div data-testid="fullscreen-controls-stub" />,
73-
MontageKebabMenu: () => <div data-testid="montage-kebab-stub" />,
74+
MontageKebabMenu: ({ monitors }: { monitors: Array<{ Id: string; Name: string }> }) => (
75+
<div data-testid="montage-kebab-stub">{monitors.map((m) => m.Name).join(',')}</div>
76+
),
7477
MontageTileErrorBoundary: ({ children }: { children?: React.ReactNode }) => <>{children}</>,
7578
MontageScrollPad: () => null,
7679
useMontageGrid: () => useMontageGridMock(),
@@ -166,6 +169,8 @@ describe('Montage Page', () => {
166169
useCurrentProfileMock.mockReset();
167170
useProfileScopeMock.mockReset();
168171
useMontageGridMock.mockReset();
172+
useGroupFilterMock.mockReset();
173+
useGroupFilterMock.mockReturnValue({ isFilterActive: false, filteredMonitorIds: [], isFilterReady: true });
169174
useMontageGridMock.mockReturnValue({
170175
layout: [],
171176
gridCols: 2,
@@ -239,6 +244,63 @@ describe('Montage Page', () => {
239244
expect(toggle).toHaveAttribute('title', 'montage.edit_disabled_all_mode');
240245
});
241246

247+
// Stale edit mode from single mode left the All-mode grid in a
248+
// draggable-but-inert state (the toggle disables so it can never be turned
249+
// off): entering All mode must reset it (refs #337, final fix wave).
250+
it('resets edit mode when switching into All mode', () => {
251+
singleProfile();
252+
useScopedMonitorsMock.mockReturnValue({
253+
monitors: [{ profileId: 'profile-1', profileName: 'Home', item: monitor('1', 'Front Door') }],
254+
errors: [],
255+
isLoading: false,
256+
refetchProfile: vi.fn(),
257+
});
258+
259+
const { rerender } = render(<Montage />);
260+
fireEvent.click(screen.getByTestId('montage-edit-toggle'));
261+
expect(screen.getByTestId('montage-edit-toggle')).toHaveTextContent('montage.done_editing');
262+
263+
allMode([{ id: 'profile-1', name: 'Home' }, { id: 'profile-2', name: 'Office' }]);
264+
useScopedMonitorsMock.mockReturnValue({
265+
monitors: [{ profileId: 'profile-1', profileName: 'Home', item: monitor('1', 'Front Door') }],
266+
errors: [],
267+
isLoading: false,
268+
refetchProfile: vi.fn(),
269+
});
270+
rerender(<Montage />);
271+
272+
const toggle = screen.getByTestId('montage-edit-toggle');
273+
expect(toggle).toHaveTextContent('montage.edit_layout');
274+
expect(toggle).toBeDisabled();
275+
});
276+
277+
// A hidden monitor must stay un-hideable regardless of the active group
278+
// filter: the kebab's own list is a DIFFERENT list than the grid's (the
279+
// grid is group-filtered, the kebab must not be), else a monitor hidden
280+
// while outside the active group becomes permanently stuck (refs #337,
281+
// single-mode regression in the final fix wave).
282+
it('kebab lists every monitor even when the grid is narrowed by an active group filter', () => {
283+
singleProfile();
284+
useScopedMonitorsMock.mockReturnValue({
285+
monitors: [
286+
{ profileId: 'profile-1', profileName: 'Home', item: monitor('1', 'Front Door') },
287+
{ profileId: 'profile-1', profileName: 'Home', item: monitor('2', 'Back Door') },
288+
],
289+
errors: [],
290+
isLoading: false,
291+
refetchProfile: vi.fn(),
292+
});
293+
useGroupFilterMock.mockReturnValue({ isFilterActive: true, filteredMonitorIds: ['1'], isFilterReady: true });
294+
295+
render(<Montage />);
296+
297+
// Grid: narrowed to the active group (monitor 2 excluded).
298+
expect(screen.getByTestId('montage-monitor-1')).toBeInTheDocument();
299+
expect(screen.queryByTestId('montage-monitor-2')).not.toBeInTheDocument();
300+
// Kebab: still lists both, so the excluded one stays toggleable.
301+
expect(screen.getByTestId('montage-kebab-stub')).toHaveTextContent('Front Door,Back Door');
302+
});
303+
242304
it('All mode renders both profiles\' tiles with a profile chip each, composite-keyed', () => {
243305
allMode([{ id: 'profile-1', name: 'Home' }, { id: 'profile-2', name: 'Office' }]);
244306
useScopedMonitorsMock.mockReturnValue({

0 commit comments

Comments
 (0)