mirror of
https://github.com/johannesjo/super-productivity.git
synced 2026-07-17 16:37:43 +00:00
fix(notes): persist fullscreen note when a resize closes the editor (#8434)
The fullscreen markdown editor is a detached CDK overlay opened with MatDialog's default closeOnNavigation. That maps to the overlay's disposeOnNavigation, which on any Location change (popstate/hashchange) disposes the overlay with NO result. Resizing the window across the mobile layout breakpoint fires such a navigation, so the editor was torn down and the in-flight note silently dropped. Open the dialog with closeOnNavigation: false and reinstate close-on- navigation by subscribing to the same Location signal, but close through the dialog's save path so the edit is persisted instead of discarded. This also keeps the Android back-button closing the editor (it routes through history.back -> popstate) and now saves on that path too. The listener is not tied to takeUntilDestroyed so it still fires after a breakpoint switch destroys this host mid-edit; the save then lands via the existing destroyed-host dispatch (#8432). Torn down in afterClosed. Covered by inline-markdown unit tests and a new note-resize e2e.
This commit is contained in:
parent
2a2b0b05c3
commit
5966ba5f43
3 changed files with 147 additions and 0 deletions
65
e2e/tests/note/detail-fullscreen-note-resize.spec.ts
Normal file
65
e2e/tests/note/detail-fullscreen-note-resize.spec.ts
Normal file
|
|
@ -0,0 +1,65 @@
|
|||
import { test, expect } from '../../fixtures/test.fixture';
|
||||
import { WorkViewPage } from '../../pages/work-view.page';
|
||||
import { cssSelectors } from '../../constants/selectors';
|
||||
|
||||
/**
|
||||
* Regression for issue #8434 (follow-up): a note edited in the fullscreen
|
||||
* markdown editor (outside focus mode) was lost when the window was resized.
|
||||
*
|
||||
* Crossing the mobile layout breakpoint fires a router navigation; MatDialog's
|
||||
* default closeOnNavigation closed the fullscreen editor with no result, so the
|
||||
* in-flight note was dropped. The fix opts out of closeOnNavigation and instead
|
||||
* closes through the save path, so the navigation persists the note.
|
||||
*/
|
||||
test.describe('Detail panel fullscreen note - resize', () => {
|
||||
const NOTE_TEXT = 'Note typed before a window resize 8434';
|
||||
|
||||
test('persists a fullscreen note edit when a resize crosses the mobile breakpoint', async ({
|
||||
page,
|
||||
testPrefix,
|
||||
taskPage,
|
||||
}) => {
|
||||
test.setTimeout(60000);
|
||||
const workViewPage = new WorkViewPage(page, testPrefix);
|
||||
await workViewPage.waitForTaskList();
|
||||
await workViewPage.addTask('Resize note task');
|
||||
|
||||
const task = page.locator('task').first();
|
||||
await expect(task).toBeVisible();
|
||||
await taskPage.openTaskDetail(task);
|
||||
|
||||
const detailPanel = page.locator(cssSelectors.DETAIL_PANEL);
|
||||
await expect(detailPanel).toBeVisible();
|
||||
|
||||
// Open the fullscreen markdown editor from the notes inline-markdown.
|
||||
await detailPanel
|
||||
.locator('inline-markdown button')
|
||||
.filter({ has: page.locator('mat-icon', { hasText: 'fullscreen' }) })
|
||||
.first()
|
||||
.click();
|
||||
|
||||
const dialog = page.locator('dialog-fullscreen-markdown');
|
||||
await expect(dialog).toBeVisible({ timeout: 5000 });
|
||||
|
||||
const textarea = dialog.locator('textarea');
|
||||
await textarea.click();
|
||||
await textarea.fill(NOTE_TEXT);
|
||||
|
||||
// Resize narrow, crossing the 600px breakpoint (desktop right-panel ->
|
||||
// mobile bottom sheet). This fires the navigation that used to drop the
|
||||
// note; the fix routes it through the save path instead.
|
||||
await page.setViewportSize({ width: 480, height: 800 });
|
||||
|
||||
// The fullscreen editor closes via the save path.
|
||||
await expect(dialog).not.toBeVisible({ timeout: 5000 });
|
||||
|
||||
// The note panel re-renders in the mobile bottom sheet, now backed by the
|
||||
// persisted task notes: the typed text must have survived the resize.
|
||||
await expect(detailPanel.locator('inline-markdown').first()).toContainText(
|
||||
NOTE_TEXT,
|
||||
{
|
||||
timeout: 5000,
|
||||
},
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
@ -11,6 +11,7 @@ import { of, Subject } from 'rxjs';
|
|||
import { TranslateModule } from '@ngx-translate/core';
|
||||
import { TaskSharedActions } from '../../root-store/meta/task-shared.actions';
|
||||
import { Log } from '../../core/log';
|
||||
import { Location } from '@angular/common';
|
||||
|
||||
describe('InlineMarkdownComponent', () => {
|
||||
let component: InlineMarkdownComponent;
|
||||
|
|
@ -1863,4 +1864,62 @@ describe('InlineMarkdownComponent', () => {
|
|||
expect(warnSpy).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('fullscreen editor save-and-close on navigation (#8434)', () => {
|
||||
let afterClosed$: Subject<unknown>;
|
||||
let closeSpy: jasmine.Spy;
|
||||
let unsubscribeSpy: jasmine.Spy;
|
||||
let locationCb: ((value: PopStateEvent) => void) | undefined;
|
||||
|
||||
beforeEach(() => {
|
||||
afterClosed$ = new Subject<unknown>();
|
||||
closeSpy = jasmine.createSpy('close');
|
||||
unsubscribeSpy = jasmine.createSpy('unsubscribe');
|
||||
locationCb = undefined;
|
||||
|
||||
// Capture the Location listener so a "navigation" can be simulated.
|
||||
const location = TestBed.inject(Location);
|
||||
spyOn(location, 'subscribe').and.callFake((cb: (value: PopStateEvent) => void) => {
|
||||
locationCb = cb;
|
||||
return { unsubscribe: unsubscribeSpy } as never;
|
||||
});
|
||||
|
||||
mockMatDialog.open.and.returnValue({
|
||||
afterClosed: () => afterClosed$.asObservable(),
|
||||
componentInstance: { close: closeSpy },
|
||||
} as never);
|
||||
fixture.componentRef.setInput('taskId', 'task-1');
|
||||
fixture.detectChanges();
|
||||
});
|
||||
|
||||
// The default closeOnNavigation disposes the overlay with no result on a
|
||||
// navigation, dropping the edit (#8434); we must opt out so we can close it
|
||||
// through the save path instead.
|
||||
it('opens the dialog with closeOnNavigation disabled', () => {
|
||||
component.openFullScreen();
|
||||
|
||||
const config = mockMatDialog.open.calls.mostRecent().args[1];
|
||||
expect(config?.closeOnNavigation).toBe(false);
|
||||
});
|
||||
|
||||
// A navigation while the editor is open — an Android back-button press, or
|
||||
// the involuntary route change a breakpoint-crossing window resize fires —
|
||||
// must close the dialog through its save path rather than silently drop it.
|
||||
it('closes the dialog via its save path on a Location change', () => {
|
||||
component.openFullScreen();
|
||||
expect(closeSpy).not.toHaveBeenCalled();
|
||||
|
||||
locationCb!({} as PopStateEvent);
|
||||
|
||||
expect(closeSpy).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('stops listening for navigations once the dialog has closed', () => {
|
||||
component.openFullScreen();
|
||||
|
||||
afterClosed$.next(undefined);
|
||||
|
||||
expect(unsubscribeSpy).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -39,6 +39,7 @@ import { TaskAttachmentService } from '../../features/tasks/task-attachment/task
|
|||
import { ResolveClipboardImagesDirective } from '../../core/clipboard-image/resolve-clipboard-images.directive';
|
||||
import { ClipboardPasteHandlerService } from '../../core/clipboard-image/clipboard-paste-handler.service';
|
||||
import { Store } from '@ngrx/store';
|
||||
import { Location } from '@angular/common';
|
||||
import { TaskSharedActions } from '../../root-store/meta/task-shared.actions';
|
||||
import { Log } from '../../core/log';
|
||||
import { handleListKeydown } from './markdown-toolbar.util';
|
||||
|
|
@ -72,6 +73,7 @@ export class InlineMarkdownComponent implements OnInit, OnDestroy {
|
|||
private _taskAttachmentService = inject(TaskAttachmentService);
|
||||
private _clipboardPasteHandler = inject(ClipboardPasteHandlerService);
|
||||
private _store = inject(Store);
|
||||
private _location = inject(Location);
|
||||
private _currentPastePlaceholder: string | null = null;
|
||||
private _isFullscreenDialogOpen = false;
|
||||
private _isDestroyed = false;
|
||||
|
|
@ -367,16 +369,37 @@ export class InlineMarkdownComponent implements OnInit, OnDestroy {
|
|||
height: '100vh',
|
||||
restoreFocus: true,
|
||||
autoFocus: 'textarea',
|
||||
// Opt out of MatDialog's default closeOnNavigation: it closes the dialog
|
||||
// with NO result on any navigation — including the involuntary route
|
||||
// change a window resize triggers when it crosses the mobile layout
|
||||
// breakpoint — so the in-flight note was silently dropped (#8434). We
|
||||
// reinstate the close-on-navigation below, but through the save path so
|
||||
// the edit (and Android back-button close) survive.
|
||||
closeOnNavigation: false,
|
||||
data: {
|
||||
content: currentContent,
|
||||
taskId,
|
||||
},
|
||||
});
|
||||
|
||||
// Reinstate close-on-navigation as a save-and-close. closeOnNavigation
|
||||
// (above) is the overlay's `disposeOnNavigation`, which on a Location
|
||||
// change (popstate/hashchange) DISPOSES the overlay with no result —
|
||||
// dropping the edit. We listen to the same Location signal but close
|
||||
// through the dialog's save path, so an Android back-button press or the
|
||||
// involuntary navigation a breakpoint-crossing resize triggers persists
|
||||
// the note instead of losing it. Not tied to takeUntilDestroyed so it
|
||||
// still fires after a breakpoint switch destroys this host mid-edit; torn
|
||||
// down in the afterClosed handler below.
|
||||
const locationSub = this._location.subscribe(() =>
|
||||
dialogRef.componentInstance?.close(),
|
||||
);
|
||||
|
||||
// Intentionally NOT torn down with takeUntilDestroyed: this MUST still fire
|
||||
// after the component is destroyed — see the `_isDestroyed` branch below.
|
||||
// afterClosed emits once then completes, so there is no leak.
|
||||
dialogRef.afterClosed().subscribe((res) => {
|
||||
locationSub.unsubscribe();
|
||||
this._isFullscreenDialogOpen = false;
|
||||
// DELETE resets the note to its default text; a string is the saved note.
|
||||
// A missing result (Close without saving) leaves the note untouched.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue