diff --git a/frontend/package-lock.json b/frontend/package-lock.json index 33ad29cb6695..2747008aa164 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -46,8 +46,8 @@ "@github/webauthn-json": "^2.1.1", "@hocuspocus/provider": "^3.4.4", "@hotwired/stimulus": "^3.2.2", - "@hotwired/turbo": "^8.0.20", - "@hotwired/turbo-rails": "^8.0.20", + "@hotwired/turbo": "^8.0.23", + "@hotwired/turbo-rails": "^8.0.23", "@knowledgecode/delegate": "^0.10.3", "@kolkov/ngx-gallery": "^2.0.1", "@mantine/core": "^9.0.1", @@ -146,7 +146,7 @@ "@types/dragula": "^3.7.5", "@types/flot": "^0.0.36", "@types/hammerjs": "^2.0.36", - "@types/hotwired__turbo": "^8.0.5", + "@types/hotwired__turbo": "^8.0.10", "@types/jquery": "^3.5.33", "@types/jqueryui": "^1.12.24", "@types/lodash": "^4.17.23", @@ -5356,19 +5356,20 @@ "integrity": "sha512-eGeIqNOQpXoPAIP7tC1+1Yc1yl1xnwYqg+3mzqxyrbE5pg5YFBZcA6YoTiByJB6DKAEsiWtl6tjTJS4IYtbB7A==" }, "node_modules/@hotwired/turbo": { - "version": "8.0.20", - "resolved": "https://registry.npmjs.org/@hotwired/turbo/-/turbo-8.0.20.tgz", - "integrity": "sha512-IilkH/+h92BRLeY/rMMR3MUh1gshIfdra/qZzp/Bl5FmiALD/6sQZK/ecxSbumeyOYiWr/JRI+Au1YQmkJGnoA==", + "version": "8.0.23", + "resolved": "https://registry.npmjs.org/@hotwired/turbo/-/turbo-8.0.23.tgz", + "integrity": "sha512-GZ7cijxEZ6Ig71u7rD6LHaRv/wcE/hNsc+nEfiWOkLNqUgLOwo5MNGWOy5ZV9ZUDSiQx1no7YxjTNnT4O6//cQ==", "engines": { "node": ">= 18" } }, "node_modules/@hotwired/turbo-rails": { - "version": "8.0.20", - "resolved": "https://registry.npmjs.org/@hotwired/turbo-rails/-/turbo-rails-8.0.20.tgz", - "integrity": "sha512-4aYkYF9XMKL7ZZPfgElq15+60osZOwMwhztE4myKQYEzCPvaPUxwZH301tOrBNtWUwOD+TNOm1Hrpeaq22RX9A==", + "version": "8.0.23", + "resolved": "https://registry.npmjs.org/@hotwired/turbo-rails/-/turbo-rails-8.0.23.tgz", + "integrity": "sha512-iBILwda3qmQC7FYM70+4s6kEQ7Fx9dJ6+yGxjPyrz9a5JDx1+y7OAA5TA7GGVOZJoicMLrKGdFDNorl40X35lw==", + "license": "MIT", "dependencies": { - "@hotwired/turbo": "^8.0.20", + "@hotwired/turbo": "^8.0.23", "@rails/actioncable": ">=7.0" } }, @@ -9819,9 +9820,9 @@ } }, "node_modules/@types/hotwired__turbo": { - "version": "8.0.5", - "resolved": "https://registry.npmjs.org/@types/hotwired__turbo/-/hotwired__turbo-8.0.5.tgz", - "integrity": "sha512-lbbSGRg2QvyJJlF4LK4wVovQggUjLrMxpvgj66DFWt11GzdBBEjY2GQLoqFWfmc4AdBNPsSSw+l90DozZFtDfQ==", + "version": "8.0.10", + "resolved": "https://registry.npmjs.org/@types/hotwired__turbo/-/hotwired__turbo-8.0.10.tgz", + "integrity": "sha512-ZnNDmfE2mwvbpmq55ntbTwP82Y4U+g3lcGMTWvS+Vo5LEuO6YQrWVhCmrTae3IH19/nNcHvzbNuroNWEwRylFg==", "dev": true, "license": "MIT" }, @@ -29198,16 +29199,16 @@ "integrity": "sha512-eGeIqNOQpXoPAIP7tC1+1Yc1yl1xnwYqg+3mzqxyrbE5pg5YFBZcA6YoTiByJB6DKAEsiWtl6tjTJS4IYtbB7A==" }, "@hotwired/turbo": { - "version": "8.0.20", - "resolved": "https://registry.npmjs.org/@hotwired/turbo/-/turbo-8.0.20.tgz", - "integrity": "sha512-IilkH/+h92BRLeY/rMMR3MUh1gshIfdra/qZzp/Bl5FmiALD/6sQZK/ecxSbumeyOYiWr/JRI+Au1YQmkJGnoA==" + "version": "8.0.23", + "resolved": "https://registry.npmjs.org/@hotwired/turbo/-/turbo-8.0.23.tgz", + "integrity": "sha512-GZ7cijxEZ6Ig71u7rD6LHaRv/wcE/hNsc+nEfiWOkLNqUgLOwo5MNGWOy5ZV9ZUDSiQx1no7YxjTNnT4O6//cQ==" }, "@hotwired/turbo-rails": { - "version": "8.0.20", - "resolved": "https://registry.npmjs.org/@hotwired/turbo-rails/-/turbo-rails-8.0.20.tgz", - "integrity": "sha512-4aYkYF9XMKL7ZZPfgElq15+60osZOwMwhztE4myKQYEzCPvaPUxwZH301tOrBNtWUwOD+TNOm1Hrpeaq22RX9A==", + "version": "8.0.23", + "resolved": "https://registry.npmjs.org/@hotwired/turbo-rails/-/turbo-rails-8.0.23.tgz", + "integrity": "sha512-iBILwda3qmQC7FYM70+4s6kEQ7Fx9dJ6+yGxjPyrz9a5JDx1+y7OAA5TA7GGVOZJoicMLrKGdFDNorl40X35lw==", "requires": { - "@hotwired/turbo": "^8.0.20", + "@hotwired/turbo": "^8.0.23", "@rails/actioncable": ">=7.0" } }, @@ -31910,9 +31911,9 @@ } }, "@types/hotwired__turbo": { - "version": "8.0.5", - "resolved": "https://registry.npmjs.org/@types/hotwired__turbo/-/hotwired__turbo-8.0.5.tgz", - "integrity": "sha512-lbbSGRg2QvyJJlF4LK4wVovQggUjLrMxpvgj66DFWt11GzdBBEjY2GQLoqFWfmc4AdBNPsSSw+l90DozZFtDfQ==", + "version": "8.0.10", + "resolved": "https://registry.npmjs.org/@types/hotwired__turbo/-/hotwired__turbo-8.0.10.tgz", + "integrity": "sha512-ZnNDmfE2mwvbpmq55ntbTwP82Y4U+g3lcGMTWvS+Vo5LEuO6YQrWVhCmrTae3IH19/nNcHvzbNuroNWEwRylFg==", "dev": true }, "@types/http-errors": { diff --git a/frontend/package.json b/frontend/package.json index acd1c25ea036..8b0d52b4c0be 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -24,7 +24,7 @@ "@types/dragula": "^3.7.5", "@types/flot": "^0.0.36", "@types/hammerjs": "^2.0.36", - "@types/hotwired__turbo": "^8.0.5", + "@types/hotwired__turbo": "^8.0.10", "@types/jquery": "^3.5.33", "@types/jqueryui": "^1.12.24", "@types/lodash": "^4.17.23", @@ -97,8 +97,8 @@ "@github/webauthn-json": "^2.1.1", "@hocuspocus/provider": "^3.4.4", "@hotwired/stimulus": "^3.2.2", - "@hotwired/turbo": "^8.0.20", - "@hotwired/turbo-rails": "^8.0.20", + "@hotwired/turbo": "^8.0.23", + "@hotwired/turbo-rails": "^8.0.23", "@knowledgecode/delegate": "^0.10.3", "@kolkov/ngx-gallery": "^2.0.1", "@mantine/core": "^9.0.1", diff --git a/frontend/src/app/core/setup/globals/openproject.ts b/frontend/src/app/core/setup/globals/openproject.ts index 6a4f06601e45..d0755d1b31e4 100644 --- a/frontend/src/app/core/setup/globals/openproject.ts +++ b/frontend/src/app/core/setup/globals/openproject.ts @@ -56,12 +56,11 @@ export class OpenProject { return this.pageState === 'submitted'; } - /** Globally setable variable whether any of the EditFormComponent - * contain changes. - * Necessary to show a data loss warning on beforeunload when clicking - * on a link out of the Angular app (ie: main side menu) - * */ - public editFormsContainModelChanges:boolean; + public get pageHasUnsavedChanges():boolean { + return this.pageWasEdited || this.editFormsContainUnsavedChanges(); + } + + public editFormsContainUnsavedChanges:() => boolean = () => false; public getPluginContext():Promise { return firstValueFrom(this.pluginContext.values$()); @@ -103,9 +102,9 @@ export class OpenProject { window.localStorage.setItem(key, newValue); } else { const value = window.localStorage.getItem(key); - return value === null ? undefined : value; + return value ?? undefined; } - } catch (e) { + } catch { console.error('Failed to access your browsers local storage. Is your local database corrupted?'); } } diff --git a/frontend/src/app/features/work-packages/components/wp-relations/wp-relations.component.ts b/frontend/src/app/features/work-packages/components/wp-relations/wp-relations.component.ts index 24ba6141e126..50f2c6c3d5a0 100644 --- a/frontend/src/app/features/work-packages/components/wp-relations/wp-relations.component.ts +++ b/frontend/src/app/features/work-packages/components/wp-relations/wp-relations.component.ts @@ -34,7 +34,7 @@ import { ApiV3Service } from 'core-app/core/apiv3/api-v3.service'; import { WorkPackageRelationsService } from './wp-relations.service'; import { PathHelperService } from 'core-app/core/path-helper/path-helper.service'; import { TurboRequestsService } from 'core-app/core/turbo/turbo-requests.service'; -import { renderStreamMessage } from '@hotwired/turbo'; +import { type FrameElement, renderStreamMessage, type TurboSubmitEndEvent } from '@hotwired/turbo'; import { HalEventsService } from 'core-app/features/hal/services/hal-events.service'; @Component({ @@ -52,7 +52,7 @@ export class WorkPackageRelationsComponent extends UntilDestroyedMixin implement @Input() public workPackage:WorkPackageResource; - @ViewChild('frameElement') readonly relationTurboFrame:ElementRef; + @ViewChild('frameElement') readonly relationTurboFrame:ElementRef; turboFrameSrc:string; @@ -96,12 +96,9 @@ export class WorkPackageRelationsComponent extends UntilDestroyedMixin implement document.addEventListener('turbo:submit-end', this.turboFrameListener); } - private async updateFrontendData(event:CustomEvent) { + private async updateFrontendData(event:TurboSubmitEndEvent) { if (event) { - // A turbo:submit-end event *has* a `formSubmission` property, but I do not - // know how to avoid the eslint type warning. Please if you know, fix it. - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - const form = event.detail.formSubmission.formElement as HTMLFormElement; + const form = event.detail.formSubmission.formElement; const updateWorkPackage = !!form.dataset?.updateWorkPackage; if (updateWorkPackage) { diff --git a/frontend/src/app/features/work-packages/components/wp-reminder-modal/wp-reminder.modal.ts b/frontend/src/app/features/work-packages/components/wp-reminder-modal/wp-reminder.modal.ts index c7d5d4a2bb15..dd79f8b0bb5b 100644 --- a/frontend/src/app/features/work-packages/components/wp-reminder-modal/wp-reminder.modal.ts +++ b/frontend/src/app/features/work-packages/components/wp-reminder-modal/wp-reminder.modal.ts @@ -10,6 +10,7 @@ import { Observable } from 'rxjs'; import { map } from 'rxjs/operators'; import { ApiV3Service } from 'core-app/core/apiv3/api-v3.service'; import { CollectionResource } from 'core-app/features/hal/resources/collection-resource'; +import type { FrameElement, TurboSubmitEndEvent } from '@hotwired/turbo'; @Component({ templateUrl: './wp-reminder.modal.html', @@ -23,7 +24,7 @@ export class WorkPackageReminderModalComponent extends OpModalComponent implemen readonly actions$ = inject(ActionsService); readonly apiV3Service = inject(ApiV3Service); - @ViewChild('frameElement') frameElement:ElementRef; + @ViewChild('frameElement') frameElement:ElementRef; // Hide close button so it's not duplicated in primer (WP#51699) showCloseButton = false; @@ -88,12 +89,10 @@ export class WorkPackageReminderModalComponent extends OpModalComponent implemen this.frameSrc = url.toString(); } - private turboSubmitEndListener(event:CustomEvent) { - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + private turboSubmitEndListener(event:TurboSubmitEndEvent) { const { fetchResponse } = event.detail; - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - if (fetchResponse.succeeded) { + if (fetchResponse?.succeeded) { this.closeMe(); this.onClose(); } diff --git a/frontend/src/app/features/work-packages/components/wp-share-modal/wp-share.modal.ts b/frontend/src/app/features/work-packages/components/wp-share-modal/wp-share.modal.ts index 5cb321d37309..d4152be4a743 100644 --- a/frontend/src/app/features/work-packages/components/wp-share-modal/wp-share.modal.ts +++ b/frontend/src/app/features/work-packages/components/wp-share-modal/wp-share.modal.ts @@ -5,6 +5,7 @@ import { WorkPackageResource } from 'core-app/features/hal/resources/work-packag import { PathHelperService } from 'core-app/core/path-helper/path-helper.service'; import { ActionsService } from 'core-app/core/state/actions/actions.service'; import { shareModalUpdated } from 'core-app/features/work-packages/components/wp-share-modal/sharing.actions'; +import { type FrameElement } from '@hotwired/turbo'; @Component({ templateUrl: './wp-share.modal.html', @@ -17,7 +18,7 @@ export class WorkPackageShareModalComponent extends OpModalComponent implements readonly pathHelper = inject(PathHelperService); readonly actions$ = inject(ActionsService); - @ViewChild('frameElement') frameElement:ElementRef|undefined; + @ViewChild('frameElement') frameElement:ElementRef|undefined; // Hide close button so it's not duplicated in primer (WP#51699) showCloseButton = false; diff --git a/frontend/src/app/shared/components/fields/edit/edit-form/edit-form.component.spec.ts b/frontend/src/app/shared/components/fields/edit/edit-form/edit-form.component.spec.ts new file mode 100644 index 000000000000..ba9897600737 --- /dev/null +++ b/frontend/src/app/shared/components/fields/edit/edit-form/edit-form.component.spec.ts @@ -0,0 +1,118 @@ +//-- copyright +// OpenProject is an open source project management software. +// Copyright (C) the OpenProject GmbH +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License version 3. +// +// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +// Copyright (C) 2006-2013 Jean-Philippe Lang +// Copyright (C) 2010-2013 the ChiliProject Team +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either version 2 +// of the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +// +// See COPYRIGHT and LICENSE files for more details. +//++ + +import { TestBed } from '@angular/core/testing'; +import { StateService, Transition, TransitionService } from '@uirouter/core'; +import * as Turbo from '@hotwired/turbo'; +import { ConfigurationService } from 'core-app/core/config/configuration.service'; +import { I18nService } from 'core-app/core/i18n/i18n.service'; +import { EditingPortalService } from 'core-app/shared/components/fields/edit/editing-portal/editing-portal-service'; +import { EditFieldHandler } from 'core-app/shared/components/fields/edit/editing-portal/edit-field-handler'; +import { EditFormRoutingService } from 'core-app/shared/components/fields/edit/edit-form/edit-form-routing.service'; +import { EditFormComponent } from 'core-app/shared/components/fields/edit/edit-form/edit-form.component'; +import { GlobalEditFormChangesTrackerService } from 'core-app/shared/components/fields/edit/services/global-edit-form-changes-tracker/global-edit-form-changes-tracker.service'; +import { vi } from 'vitest'; + +describe('EditFormComponent', () => { + let onBeforeCallback:(transition:Transition) => unknown; + + afterEach(() => { + vi.restoreAllMocks(); + }); + + beforeEach(async () => { + await TestBed + .configureTestingModule({ + declarations: [ + EditFormComponent, + ], + providers: [ + { + provide: TransitionService, + useValue: { + onBefore: vi.fn((_criteria:unknown, callback:(transition:Transition) => unknown) => { + onBeforeCallback = callback; + return vi.fn(); + }), + }, + }, + { provide: ConfigurationService, useValue: { warnOnLeavingUnsaved: vi.fn().mockReturnValue(true) } }, + { provide: EditingPortalService, useValue: {} }, + { provide: StateService, useValue: {} }, + { provide: I18nService, useValue: { t: vi.fn().mockReturnValue('Leave edit mode?') } }, + { provide: EditFormRoutingService, useValue: { blockedTransition: vi.fn().mockReturnValue(true) } }, + { + provide: GlobalEditFormChangesTrackerService, + useValue: { + addToActiveForms: vi.fn(), + removeFromActiveForms: vi.fn(), + }, + }, + ], + }) + .compileComponents(); + }); + + it('restores a canceled browser Back transition without navigating forward', () => { + const fixture = TestBed.createComponent(EditFormComponent); + const component = fixture.componentInstance; + const urlRouterUpdate = vi.fn(); + const transition = { + options: vi.fn().mockReturnValue({ source: 'url' }), + from: vi.fn().mockReturnValue({ name: 'work-packages.partitioned.split' }), + params: vi.fn().mockReturnValue({ workPackageId: '46' }), + router: { + stateService: { + href: vi.fn().mockReturnValue('/work_packages/details/46/overview'), + }, + urlRouter: { + update: urlRouterUpdate, + }, + }, + } as unknown as Transition; + const turboPush = vi.spyOn( + Turbo.session.history, + 'push', + ).mockImplementation(() => undefined); + const historyForward = vi.spyOn(window.history, 'forward'); + const confirm = vi.spyOn(window, 'confirm').mockReturnValue(false); + const cancel = vi.spyOn(component, 'cancel'); + + component.activeFields = { + description: {} as EditFieldHandler, + }; + + expect(onBeforeCallback(transition)).toBe(false); + expect(confirm).toHaveBeenCalledWith('Leave edit mode?'); + expect(cancel).not.toHaveBeenCalled(); + expect(turboPush).toHaveBeenCalledOnce(); + expect(turboPush.mock.calls[0][0].pathname).toBe('/work_packages/details/46/overview'); + expect(urlRouterUpdate).toHaveBeenCalledWith(true); + expect(historyForward).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/app/shared/components/fields/edit/edit-form/edit-form.component.ts b/frontend/src/app/shared/components/fields/edit/edit-form/edit-form.component.ts index 5897a8faa793..83e5e54dc997 100644 --- a/frontend/src/app/shared/components/fields/edit/edit-form/edit-form.component.ts +++ b/frontend/src/app/shared/components/fields/edit/edit-form/edit-form.component.ts @@ -46,6 +46,7 @@ import { EditFormRoutingService } from 'core-app/shared/components/fields/edit/e import { ResourceChangesetCommit } from 'core-app/shared/components/fields/edit/services/hal-resource-editing.service'; import { GlobalEditFormChangesTrackerService } from 'core-app/shared/components/fields/edit/services/global-edit-form-changes-tracker/global-edit-form-changes-tracker.service'; import { firstValueFrom } from 'rxjs'; +import * as Turbo from '@hotwired/turbo'; @Component({ selector: 'edit-form,[edit-form]', @@ -103,6 +104,7 @@ export class EditFormComponent extends EditForm implements OnInit, // that's not within the edit mode. if (!this.editFormRouting || this.editFormRouting.blockedTransition(transition)) { if (requiresConfirmation && !window.confirm(confirmText)) { + this.undoCanceledBrowserBackTransition(transition); return false; } @@ -113,6 +115,31 @@ export class EditFormComponent extends EditForm implements OnInit, }); } + private undoCanceledBrowserBackTransition(transition:Transition) { + if (transition.options().source !== 'url') { + return; + } + + const fromUrl = transition + .router + .stateService + .href(transition.from(), transition.params('from')); + + if (!fromUrl) { + return; + } + + // Restore the canceled Back URL without firing a real forward navigation, + // which would make Turbo restore a stale snapshot of the split view. + Turbo.session + .history + .push(new URL(fromUrl, window.location.origin)); + + // Keep UI-Router from replacing the restored browser history entry while + // it rolls back the aborted Back navigation. + transition.router.urlRouter.update(true); + } + ngOnInit() { this.editMode = this.initializeEditMode; this.globalEditFormChangesTrackerService.addToActiveForms(this); diff --git a/frontend/src/app/shared/components/fields/edit/modal-with-turbo-content/modal-with-turbo-content.directive.ts b/frontend/src/app/shared/components/fields/edit/modal-with-turbo-content/modal-with-turbo-content.directive.ts index cb2192c2fb21..b28678ef00d4 100644 --- a/frontend/src/app/shared/components/fields/edit/modal-with-turbo-content/modal-with-turbo-content.directive.ts +++ b/frontend/src/app/shared/components/fields/edit/modal-with-turbo-content/modal-with-turbo-content.directive.ts @@ -34,6 +34,7 @@ import { ToastService } from 'core-app/shared/components/toaster/toast.service'; import { ApiV3Service } from 'core-app/core/apiv3/api-v3.service'; import { I18nService } from 'core-app/core/i18n/i18n.service'; import { ResourceChangeset } from 'core-app/shared/components/fields/changeset/resource-changeset'; +import type { TurboBeforeFrameRenderEvent, TurboSubmitEndEvent } from '@hotwired/turbo'; @Directive({ selector: '[opModalWithTurboContent]', @@ -78,7 +79,7 @@ export class ModalWithTurboContentDirective implements AfterViewInit, OnDestroy .removeEventListener('cancelModalWithTurboContent', this.cancelListenerBound); } - private contextBasedListener(event:CustomEvent) { + private contextBasedListener(event:TurboSubmitEndEvent) { if (this.resource.id === 'new') { void this.propagateSuccessfulCreate(event); } else { @@ -86,10 +87,8 @@ export class ModalWithTurboContentDirective implements AfterViewInit, OnDestroy } } - private preserveSegmentAttributes(event:CustomEvent) { - const turboEvent = event as CustomEvent<{ newFrame?:HTMLElement }>; - - const element = turboEvent.detail?.newFrame?.querySelector('segmented-control'); + private preserveSegmentAttributes(event:TurboBeforeFrameRenderEvent) { + const element = event.detail?.newFrame?.querySelector('segmented-control'); if (!element) return; const connectedCallback = Object.getOwnPropertyDescriptor( @@ -114,13 +113,11 @@ export class ModalWithTurboContentDirective implements AfterViewInit, OnDestroy this.cancel.emit(); } - private async propagateSuccessfulCreate(event:CustomEvent) { - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + private async propagateSuccessfulCreate(event:TurboSubmitEndEvent) { const { fetchResponse } = event.detail; - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - if (fetchResponse.succeeded) { - // eslint-disable-next-line @typescript-eslint/no-unsafe-argument,@typescript-eslint/no-unsafe-member-access + if (fetchResponse?.succeeded) { + if (!fetchResponse.response.body) return; const JSONresponse:unknown = await this.extractJSONFromResponse(fetchResponse.response.body); this.successfulCreate.emit(JSONresponse); @@ -130,12 +127,10 @@ export class ModalWithTurboContentDirective implements AfterViewInit, OnDestroy } } - private propagateSuccessfulUpdate(event:CustomEvent) { - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + private propagateSuccessfulUpdate(event:TurboSubmitEndEvent) { const { fetchResponse } = event.detail; - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - if (fetchResponse.succeeded) { + if (fetchResponse?.succeeded) { this.halEvents.push( this.resource as WorkPackageResource, { eventType: 'updated' }, diff --git a/frontend/src/app/shared/components/fields/edit/services/global-edit-form-changes-tracker/global-edit-form-changes-tracker.service.spec.ts b/frontend/src/app/shared/components/fields/edit/services/global-edit-form-changes-tracker/global-edit-form-changes-tracker.service.spec.ts index adf0d829c612..23da08df375d 100644 --- a/frontend/src/app/shared/components/fields/edit/services/global-edit-form-changes-tracker/global-edit-form-changes-tracker.service.spec.ts +++ b/frontend/src/app/shared/components/fields/edit/services/global-edit-form-changes-tracker/global-edit-form-changes-tracker.service.spec.ts @@ -1,20 +1,40 @@ import { TestBed } from '@angular/core/testing'; import { EditFormComponent } from 'core-app/shared/components/fields/edit/edit-form/edit-form.component'; +import { OpenProject } from 'core-app/core/setup/globals/openproject'; import { GlobalEditFormChangesTrackerService } from './global-edit-form-changes-tracker.service'; describe('GlobalEditFormChangesTrackerService', () => { let service:GlobalEditFormChangesTrackerService; - const createForm = (changed?:boolean) => ({ + let originalOpenProject:OpenProject; + const createForm = ( + changed?:boolean, + inFlight = false, + resourceId:string|null = '1', + editing = Boolean(changed) || resourceId === null, + ) => ({ + editing, + resource: { + id: resourceId, + }, change: { + inFlight, isEmpty: () => !changed, }, } as EditFormComponent); beforeEach(() => { + originalOpenProject = window.OpenProject; + window.OpenProject = new OpenProject(); TestBed.configureTestingModule({}); service = TestBed.inject(GlobalEditFormChangesTrackerService); }); + afterEach(() => { + // eslint-disable-next-line @typescript-eslint/dot-notation + service['abortController'].abort(); + window.OpenProject = originalOpenProject; + }); + it('should be created', () => { expect(service).toBeTruthy(); }); @@ -60,6 +80,67 @@ describe('GlobalEditFormChangesTrackerService', () => { expect(service.thereAreFormsWithUnsavedChanges).toBe(true); }); + it('should report no changes when a changed form is no longer editing', () => { + const form = createForm(true, false, '1', false); + + service.addToActiveForms(form); + + expect(service.thereAreFormsWithUnsavedChanges).toBe(false); + }); + + it('should report no changes when one form is editing without changes', () => { + const form = { + ...createForm(), + editing: true, + } as EditFormComponent; + + service.addToActiveForms(form); + + expect(service.thereAreFormsWithUnsavedChanges).toBe(false); + }); + + it('should report changes when an unchanged form tracks a new resource', () => { + const form = createForm(false, false, null); + + service.addToActiveForms(form); + + expect(service.thereAreFormsWithUnsavedChanges).toBe(true); + }); + + it('should report no changes when a new resource form is no longer editing', () => { + const form = createForm(false, false, null, false); + + service.addToActiveForms(form); + + expect(service.thereAreFormsWithUnsavedChanges).toBe(false); + }); + + it('should report no changes when the only changed form is being saved', () => { + const form = createForm(true, true); + + service.addToActiveForms(form); + + expect(service.thereAreFormsWithUnsavedChanges).toBe(false); + }); + + it('should report no changes when a new resource is being saved', () => { + const form = createForm(false, true, null); + + service.addToActiveForms(form); + + expect(service.thereAreFormsWithUnsavedChanges).toBe(false); + }); + + it('should report changes when another form has unsaved changes while one is being saved', () => { + const savingForm = createForm(true, true); + const changedForm = createForm(true); + + service.addToActiveForms(savingForm); + service.addToActiveForms(changedForm); + + expect(service.thereAreFormsWithUnsavedChanges).toBe(true); + }); + it('should report forms with changes when multiple form have changes', () => { const form = createForm(true); const form2 = createForm(true); @@ -72,13 +153,92 @@ describe('GlobalEditFormChangesTrackerService', () => { expect(service.thereAreFormsWithUnsavedChanges).toBe(true); }); - it('should call thereAreFormsWithUnsavedChangesSpy on beforeunload', () => { - const thereAreFormsWithUnsavedChangesSpy = vi.spyOn(service, 'thereAreFormsWithUnsavedChanges', 'get'); + it('should prevent beforeunload when a tracked form has changes', () => { + const form = createForm(true); + const event = new Event('beforeunload', { cancelable: true }); + + service.addToActiveForms(form); + window.dispatchEvent(event); + + expect(event.defaultPrevented).toBe(true); + }); + + it('should not prevent beforeunload when the page was submitted', () => { + const form = createForm(true); + const event = new Event('beforeunload', { cancelable: true }); + + window.OpenProject.pageState = 'submitted'; + service.addToActiveForms(form); + window.dispatchEvent(event); + + expect(event.defaultPrevented).toBe(false); + }); + + it('registers an OpenProject callback for edit form changes', () => { + const form = createForm(true); + + service.addToActiveForms(form); + + expect(window.OpenProject.editFormsContainUnsavedChanges()).toBe(true); + }); + + it('should prevent turbo:before-render for restoration visits when a tracked form has changes', () => { + const form = createForm(true); + const event = new Event('turbo:before-render', { cancelable: true }); + + service.addToActiveForms(form); + document.dispatchEvent(event); + + expect(event.defaultPrevented).toBe(true); + }); + + it('should not prevent turbo:before-render when no forms have changes', () => { + const event = new Event('turbo:before-render', { cancelable: true }); + + document.dispatchEvent(event); + + expect(event.defaultPrevented).toBe(false); + }); + + it('should not prevent turbo:before-render after a non-restore turbo:visit', () => { + const form = createForm(true); + + service.addToActiveForms(form); + document.dispatchEvent(new CustomEvent('turbo:visit', { detail: { url: 'http://example.com', action: 'advance' } })); + + const event = new Event('turbo:before-render', { cancelable: true }); + document.dispatchEvent(event); + + expect(event.defaultPrevented).toBe(false); + }); + + it('should prevent turbo:before-render after a restore turbo:visit', () => { + const form = createForm(true); + + service.addToActiveForms(form); + document.dispatchEvent(new CustomEvent('turbo:visit', { detail: { url: 'http://example.com', action: 'restore' } })); + + const event = new Event('turbo:before-render', { cancelable: true }); + document.dispatchEvent(event); + + expect(event.defaultPrevented).toBe(true); + }); + + it('should still block renders when a prior turbo:before-visit was canceled', () => { + const form = createForm(true); + + service.addToActiveForms(form); - window.onbeforeunload = vi.fn(); + const canceledVisit = new CustomEvent('turbo:before-visit', { + detail: { url: 'http://example.com' }, + cancelable: true, + }); + canceledVisit.preventDefault(); + document.dispatchEvent(canceledVisit); - window.dispatchEvent(new Event('beforeunload')); + const event = new Event('turbo:before-render', { cancelable: true }); + document.dispatchEvent(event); - expect(thereAreFormsWithUnsavedChangesSpy).toHaveBeenCalled(); + expect(event.defaultPrevented).toBe(true); }); }); diff --git a/frontend/src/app/shared/components/fields/edit/services/global-edit-form-changes-tracker/global-edit-form-changes-tracker.service.ts b/frontend/src/app/shared/components/fields/edit/services/global-edit-form-changes-tracker/global-edit-form-changes-tracker.service.ts index 359dff8b6f0b..c5b8ea06881e 100644 --- a/frontend/src/app/shared/components/fields/edit/services/global-edit-form-changes-tracker/global-edit-form-changes-tracker.service.ts +++ b/frontend/src/app/shared/components/fields/edit/services/global-edit-form-changes-tracker/global-edit-form-changes-tracker.service.ts @@ -1,6 +1,7 @@ import { Injectable, inject } from '@angular/core'; import { EditFormComponent } from 'core-app/shared/components/fields/edit/edit-form/edit-form.component'; import { I18nService } from 'core-app/core/i18n/i18n.service'; +import isNewResource from 'core-app/features/hal/helpers/is-new-resource'; @Injectable({ providedIn: 'root', @@ -9,20 +10,51 @@ export class GlobalEditFormChangesTrackerService { private i18nService = inject(I18nService); private activeForms = new Map(); + private abortController = new AbortController(); + private visitApproved = false; get thereAreFormsWithUnsavedChanges() { - return Array.from(this.activeForms.keys()).some((form) => !form.change.isEmpty()); + return Array + .from(this.activeForms.keys()) + .some((form) => ( + form.editing + && !form.change.inFlight + && (isNewResource(form.resource) || !form.change.isEmpty()) + )); } constructor() { - // Global beforeunload hook to show a data loss warn - // when the user clicks on a link out of the Angular app + const { signal } = this.abortController; + + window.OpenProject.editFormsContainUnsavedChanges = () => this.thereAreFormsWithUnsavedChanges; + + // turbo:visit fires after a visit starts (canceled visits never + // reach it) and carries the visit action. Restoration visits + // have action "restore"; link clicks have "advance"/"replace". + document.addEventListener('turbo:visit', (event) => { + const { action } = (event as CustomEvent<{ action:string }>).detail; + this.visitApproved = action !== 'restore'; + }, { signal }); + + // Block Turbo restoration renders that would clobber Angular's + // DOM while an edit form is active. For restoration visits + // visitApproved is false, so the guard fires. + document.addEventListener('turbo:before-render', (event) => { + if (!this.visitApproved && this.thereAreFormsWithUnsavedChanges) { + event.preventDefault(); + } + }, { signal }); + + // Show a data loss warning when the user closes the tab or + // navigates away from the Angular app entirely. window.addEventListener('beforeunload', (event) => { - if (this.thereAreFormsWithUnsavedChanges) { + if (!window.OpenProject.pageWasSubmitted && this.thereAreFormsWithUnsavedChanges) { + const message = this.i18nService.t('js.work_packages.confirm_edit_cancel'); + event.preventDefault(); - event.returnValue = this.i18nService.t('js.work_packages.confirm_edit_cancel'); + event.returnValue = message; } - }); + }, { signal }); } public addToActiveForms(form:EditFormComponent) { diff --git a/frontend/src/stimulus/controllers/beforeunload.controller.spec.ts b/frontend/src/stimulus/controllers/beforeunload.controller.spec.ts new file mode 100644 index 000000000000..282133eb8b6e --- /dev/null +++ b/frontend/src/stimulus/controllers/beforeunload.controller.spec.ts @@ -0,0 +1,128 @@ +/* + * -- copyright + * OpenProject is an open source project management software. + * Copyright (C) the OpenProject GmbH + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License version 3. + * + * OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: + * Copyright (C) 2006-2013 Jean-Philippe Lang + * Copyright (C) 2010-2013 the ChiliProject Team + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + * + * See COPYRIGHT and LICENSE files for more details. + * ++ + */ + +import { vi } from 'vitest'; +import { OpenProject } from 'core-app/core/setup/globals/openproject'; +import { BeforeunloadController } from './beforeunload.controller'; + +describe('BeforeunloadController', () => { + let originalOpenProject:OpenProject; + let controller:BeforeunloadController; + + beforeEach(() => { + originalOpenProject = window.OpenProject; + window.OpenProject = new OpenProject(); + vi.stubGlobal('I18n', { t: vi.fn().mockReturnValue('Leave page?') }); + controller = Object.create(BeforeunloadController.prototype) as BeforeunloadController; + }); + + afterEach(() => { + window.OpenProject = originalOpenProject; + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + }); + + function turboBeforeVisit(url = 'http://example.com/projects') { + return new CustomEvent('turbo:before-visit', { + detail: { url }, + cancelable: true, + }); + } + + function handle(event:Event) { + controller.handleEvent(event); + + return event; + } + + it('shows confirm when Angular edit forms have unsaved changes', () => { + const confirm = vi.spyOn(window, 'confirm').mockReturnValue(false); + + window.OpenProject.editFormsContainUnsavedChanges = () => true; + const event = handle(turboBeforeVisit()); + + expect(confirm).toHaveBeenCalledWith('Leave page?'); + expect(event.defaultPrevented).toBe(true); + }); + + it('shows confirm when pageState is edited', () => { + window.OpenProject.pageState = 'edited'; + const confirm = vi.spyOn(window, 'confirm').mockReturnValue(false); + + const event = handle(turboBeforeVisit()); + + expect(confirm).toHaveBeenCalledWith('Leave page?'); + expect(event.defaultPrevented).toBe(true); + }); + + it('does not show confirm when nothing is dirty', () => { + const confirm = vi.spyOn(window, 'confirm').mockReturnValue(false); + + const event = handle(turboBeforeVisit()); + + expect(confirm).not.toHaveBeenCalled(); + expect(event.defaultPrevented).toBe(false); + }); + + it('does not prevent navigation when user accepts confirm', () => { + const confirm = vi.spyOn(window, 'confirm').mockReturnValue(true); + + window.OpenProject.editFormsContainUnsavedChanges = () => true; + const event = handle(turboBeforeVisit()); + + expect(confirm).toHaveBeenCalledWith('Leave page?'); + expect(event.defaultPrevented).toBe(false); + }); + + it('only checks pageWasEdited for native beforeunload', () => { + window.OpenProject.editFormsContainUnsavedChanges = () => true; + const confirm = vi.spyOn(window, 'confirm').mockReturnValue(false); + const event = new Event('beforeunload', { cancelable: true }); + + handle(event); + + expect(confirm).not.toHaveBeenCalled(); + expect(event.defaultPrevented).toBe(false); + }); + + it('resets pageState to pristine on turbo:render', () => { + window.OpenProject.pageState = 'edited'; + + handle(new Event('turbo:render')); + + expect(window.OpenProject.pageState).toBe('pristine'); + }); + + it('sets pageState to submitted on form submit', () => { + handle(new Event('submit')); + + expect(window.OpenProject.pageState).toBe('submitted'); + }); +}); diff --git a/frontend/src/stimulus/controllers/beforeunload.controller.ts b/frontend/src/stimulus/controllers/beforeunload.controller.ts index 983aecbd0d1c..833bfb7de6ac 100644 --- a/frontend/src/stimulus/controllers/beforeunload.controller.ts +++ b/frontend/src/stimulus/controllers/beforeunload.controller.ts @@ -21,11 +21,11 @@ export class BeforeunloadController extends ApplicationController { this.abortController.abort(); } - handleEvent(evt:BeforeUnloadEvent|TurboBeforeVisitEvent|CustomEvent) { + handleEvent(evt:Event) { switch (evt.type) { case 'beforeunload': case 'turbo:before-visit': - this.beforeunloadHandler(evt as BeforeUnloadEvent|TurboBeforeVisitEvent); + this.beforeunloadHandler(evt); break; case 'turbo:submit-end': case 'turbo:load': @@ -41,7 +41,11 @@ export class BeforeunloadController extends ApplicationController { } private beforeunloadHandler(evt:BeforeUnloadEvent|TurboBeforeVisitEvent) { - if (window.OpenProject.pageState !== 'edited') { + const hasUnsavedChanges = evt.type === 'turbo:before-visit' + ? window.OpenProject.pageHasUnsavedChanges + : window.OpenProject.pageWasEdited; + + if (!hasUnsavedChanges) { return; } diff --git a/frontend/src/stimulus/controllers/dynamic/overview/project-life-cycle-form.controller.ts b/frontend/src/stimulus/controllers/dynamic/overview/project-life-cycle-form.controller.ts index c157574be1a1..16552dc061de 100644 --- a/frontend/src/stimulus/controllers/dynamic/overview/project-life-cycle-form.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/overview/project-life-cycle-form.controller.ts @@ -35,6 +35,7 @@ import { debounce, DebouncedFunc, } from 'lodash'; +import { type TurboBeforeMorphAttributeEvent } from '@hotwired/turbo'; export default class ProjectLifeCycleFormController extends FormPreviewController { private timezoneService:TimezoneService; @@ -128,7 +129,7 @@ export default class ProjectLifeCycleFormController extends FormPreviewControlle this.previewForm(); } - preventValueMorphingActiveElement(event:CustomEvent<{ attributeName:string }>) { + preventValueMorphingActiveElement(event:TurboBeforeMorphAttributeEvent) { const target = event.target as HTMLInputElement; const { attributeName } = event.detail; const isActiveElement = this.highlightedField?.id === target?.id; diff --git a/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/editor.controller.ts b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/editor.controller.ts index 2436608f2ef7..6d3823d1f1d1 100644 --- a/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/editor.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/work-packages/activities-tab/editor.controller.ts @@ -36,6 +36,7 @@ import type AutoScrollingController from './auto-scrolling.controller'; import BaseController from './base.controller'; import type PollingController from './polling.controller'; import type StemsController from './stems.controller'; +import type { TurboSubmitEndEvent, TurboSubmitStartEvent } from '@hotwired/turbo'; export default class EditorController extends BaseController { static outlets = [ @@ -157,8 +158,8 @@ export default class EditorController extends BaseController { const handlers = { beforeUnload: () => { void this.rescueEditorContent(); }, - turboSubmitStart: (event:Event) => { void this.handleTurboSubmitStart(event); }, - turboSubmitEnd: (event:Event) => { void this.handleTurboSubmitEnd(event); }, + turboSubmitStart: (event:TurboSubmitStartEvent) => { void this.handleTurboSubmitStart(event); }, + turboSubmitEnd: (event:TurboSubmitEndEvent) => { void this.handleTurboSubmitEnd(event); }, }; document.addEventListener('beforeunload', handlers.beforeUnload, { signal }); @@ -271,16 +272,16 @@ export default class EditorController extends BaseController { } } - private handleTurboSubmitStart(_event:Event) { + private handleTurboSubmitStart(_event:TurboSubmitStartEvent) { this.setCKEditorReadonlyMode(true); } - private handleTurboSubmitEnd(event:Event) { - const formSubmitResponse = (event as CustomEvent<{ fetchResponse:{ succeeded:boolean; response:{ headers:Headers } } }>).detail.fetchResponse; + private handleTurboSubmitEnd(event:TurboSubmitEndEvent) { + const formSubmitResponse = event.detail.fetchResponse; this.setCKEditorReadonlyMode(false); - if (formSubmitResponse.succeeded) { + if (formSubmitResponse?.succeeded) { // extract server timestamp from response headers in order to be in sync with the server this.pollingOutlet.setLastServerTimestampViaHeaders(formSubmitResponse.response.headers); diff --git a/frontend/src/stimulus/controllers/dynamic/work-packages/dialog/preview.controller.ts b/frontend/src/stimulus/controllers/dynamic/work-packages/dialog/preview.controller.ts index 30023fa505a1..47143eb48cbe 100644 --- a/frontend/src/stimulus/controllers/dynamic/work-packages/dialog/preview.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/work-packages/dialog/preview.controller.ts @@ -29,16 +29,9 @@ */ import { Controller } from '@hotwired/stimulus'; +import type { FrameElement, TurboBeforeFrameRenderEvent } from '@hotwired/turbo'; import { Idiomorph } from 'idiomorph'; -interface TurboBeforeFrameRenderEventDetail { - render:(currentElement:HTMLElement, newElement:HTMLElement) => void; -} - -interface HTMLTurboFrameElement extends HTMLElement { - src:string; -} - export abstract class DialogPreviewController extends Controller { static targets = [ 'form', @@ -52,7 +45,7 @@ export abstract class DialogPreviewController extends Controller { declare readonly initialValueInputTargets:HTMLInputElement[]; declare readonly touchedFieldInputTargets:HTMLInputElement[]; - protected frameMorphRenderer:(event:CustomEvent) => void; + protected frameMorphRenderer:(event:TurboBeforeFrameRenderEvent) => void; protected targetFieldName:string; protected touchedFields:Set; @@ -73,17 +66,17 @@ export abstract class DialogPreviewController extends Controller { // new ids, the ids referenced by `aria-describedby` are stale. This makes // caption and validation message unaccessible for screen readers and other // assistive technologies. This is why morph cannot be used here. - this.frameMorphRenderer = (event:CustomEvent) => { - const target = event.target as HTMLTurboFrameElement; + this.frameMorphRenderer = (event:TurboBeforeFrameRenderEvent) => { + const target = event.target as FrameElement; const requestUrl = new URL(target.src || '', window.location.origin); // Do not replace the angular datepicker unless the schedule_manually flag is changed. const schedulingChanged = requestUrl.searchParams.has('schedule_manually'); - event.detail.render = (currentElement:HTMLElement, newElement:HTMLElement) => { + event.detail.render = (currentElement, newElement) => { Idiomorph.morph(currentElement, newElement, { ignoreActiveValue: this.ignoreActiveValueWhenMorphing(), callbacks: { - beforeNodeMorphed: (oldNode:Element, newNode:Element) => { + beforeNodeMorphed: (oldNode, newNode) => { // In case the element is an OpenProject custom dom element, prevent morphing and // replace the angular tag with the new version. if (oldNode.tagName?.startsWith('OPCE-')) { @@ -106,15 +99,13 @@ export abstract class DialogPreviewController extends Controller { } }); - const turboFrame = this.formTarget.closest('turbo-frame') as HTMLTurboFrameElement; - turboFrame.addEventListener('turbo:before-frame-render', this.frameMorphRenderer); + const turboFrame = this.formTarget.closest('turbo-frame'); + turboFrame?.addEventListener('turbo:before-frame-render', this.frameMorphRenderer); } disconnect() { - const turboFrame = this.formTarget.closest('turbo-frame') as HTMLTurboFrameElement; - if (turboFrame) { - turboFrame.removeEventListener('turbo:before-frame-render', this.frameMorphRenderer); - } + const turboFrame = this.formTarget.closest('turbo-frame'); + turboFrame?.removeEventListener('turbo:before-frame-render', this.frameMorphRenderer); } protected cancel():void { @@ -147,7 +138,7 @@ export abstract class DialogPreviewController extends Controller { } const previewUrl = `${form.action}/preview?${new URLSearchParams(wpParams).toString()}`; - const turboFrame = this.formTarget.closest('turbo-frame') as HTMLTurboFrameElement; + const turboFrame = this.formTarget.closest('turbo-frame'); if (turboFrame) { turboFrame.src = previewUrl; diff --git a/frontend/src/turbo/action-menu-morph-remount.ts b/frontend/src/turbo/action-menu-morph-remount.ts index 29266102da88..5c4d64795ba0 100644 --- a/frontend/src/turbo/action-menu-morph-remount.ts +++ b/frontend/src/turbo/action-menu-morph-remount.ts @@ -41,15 +41,9 @@ * morph, frame morph, and full-page morph alike. */ -interface TurboMorphElementDetail { - currentElement:HTMLElement; - newElement:HTMLElement; -} - export function registerActionMenuMorphRemount():void { - document.addEventListener('turbo:morph-element', (event:Event) => { - const { detail } = event as CustomEvent; - const currentElement = detail?.currentElement; + document.addEventListener('turbo:morph-element', (event) => { + const currentElement = event.detail?.currentElement; if (!currentElement?.matches('action-menu:has(include-fragment[src])')) { return; } diff --git a/frontend/src/turbo/constants.ts b/frontend/src/turbo/constants.ts deleted file mode 100644 index ee6cb991a0d8..000000000000 --- a/frontend/src/turbo/constants.ts +++ /dev/null @@ -1,22 +0,0 @@ -export const TURBO_EVENTS:string[] = [ - 'turbo:before-cache', - 'turbo:before-fetch-request', - 'turbo:before-fetch-response', - 'turbo:before-frame-morph', - 'turbo:before-frame-render', - 'turbo:before-prefetch', - 'turbo:before-render', - 'turbo:before-stream-render', - 'turbo:before-visit', - 'turbo:click', - 'turbo:fetch-request-error', - 'turbo:frame-load', - 'turbo:frame-render', - 'turbo:load', - 'turbo:morph', - 'turbo:reload', - 'turbo:render', - 'turbo:submit-end', - 'turbo:submit-start', - 'turbo:visit' -]; diff --git a/frontend/src/turbo/helpers.spec.ts b/frontend/src/turbo/helpers.spec.ts new file mode 100644 index 000000000000..e37f98d1b4a5 --- /dev/null +++ b/frontend/src/turbo/helpers.spec.ts @@ -0,0 +1,84 @@ +import * as Turbo from '@hotwired/turbo'; +import { TurboHelpers } from './helpers'; + +describe('TurboHelpers.showProgressBar / hideProgressBar', () => { + const progressBar = (Turbo.session.adapter as Turbo.BrowserAdapter).progressBar; + /* eslint-disable @typescript-eslint/no-unsafe-assignment,@typescript-eslint/no-empty-function */ + let setValueSpy:ReturnType; + let showSpy:ReturnType; + let hideSpy:ReturnType; + let savedDelay:number; + + beforeEach(() => { + vi.useFakeTimers(); + setValueSpy = vi.spyOn(progressBar, 'setValue').mockImplementation(() => {}); + showSpy = vi.spyOn(progressBar, 'show').mockImplementation(() => {}); + hideSpy = vi.spyOn(progressBar, 'hide').mockImplementation(() => {}); + savedDelay = Turbo.config.drive.progressBarDelay; + Turbo.config.drive.progressBarDelay = 200; + }); + /* eslint-enable @typescript-eslint/no-unsafe-assignment,@typescript-eslint/no-empty-function */ + + afterEach(() => { + TurboHelpers.hideProgressBar(); + Turbo.config.drive.progressBarDelay = savedDelay; + vi.restoreAllMocks(); + vi.useRealTimers(); + }); + + it('sets value to 0 immediately', () => { + TurboHelpers.showProgressBar(); + + expect(setValueSpy).toHaveBeenCalledWith(0); + }); + + it('does not call show() before delay elapses', () => { + TurboHelpers.showProgressBar(); + vi.advanceTimersByTime(199); + + expect(showSpy).not.toHaveBeenCalled(); + }); + + it('calls show() after delay elapses', () => { + TurboHelpers.showProgressBar(); + vi.advanceTimersByTime(200); + + expect(showSpy).toHaveBeenCalledOnce(); + }); + + it('does not create multiple timeouts when called twice', () => { + TurboHelpers.showProgressBar(); + TurboHelpers.showProgressBar(); + vi.advanceTimersByTime(200); + + expect(showSpy).toHaveBeenCalledOnce(); + }); + + it('sets value to 1 and calls hide()', () => { + TurboHelpers.hideProgressBar(); + + expect(setValueSpy).toHaveBeenCalledWith(1); + expect(hideSpy).toHaveBeenCalledOnce(); + }); + + it('clears pending timeout so show() is never called', () => { + TurboHelpers.showProgressBar(); + vi.advanceTimersByTime(100); + TurboHelpers.hideProgressBar(); + vi.advanceTimersByTime(200); + + expect(showSpy).not.toHaveBeenCalled(); + }); + + it('handles full show → delay → hide cycle', () => { + TurboHelpers.showProgressBar(); + vi.advanceTimersByTime(200); + + expect(showSpy).toHaveBeenCalledOnce(); + + TurboHelpers.hideProgressBar(); + + expect(setValueSpy).toHaveBeenCalledWith(1); + expect(hideSpy).toHaveBeenCalledOnce(); + }); +}); diff --git a/frontend/src/turbo/helpers.ts b/frontend/src/turbo/helpers.ts index 84845167533c..59865d766b34 100644 --- a/frontend/src/turbo/helpers.ts +++ b/frontend/src/turbo/helpers.ts @@ -1,12 +1,28 @@ import * as Turbo from '@hotwired/turbo'; export namespace TurboHelpers { + let progressBarTimeout:number | undefined; + + function getProgressBar():Turbo.ProgressBar { + return (Turbo.session.adapter as Turbo.BrowserAdapter).progressBar; + } + export function showProgressBar() { - Turbo.session.adapter.formSubmissionStarted(); + const progressBar = getProgressBar(); + progressBar.setValue(0); + progressBarTimeout ??= window.setTimeout(() => { + progressBar.show(); + }, Turbo.config.drive.progressBarDelay); } export function hideProgressBar() { - Turbo.session.adapter.formSubmissionFinished(); + const progressBar = getProgressBar(); + progressBar.setValue(1); + progressBar.hide(); + if (progressBarTimeout != null) { + window.clearTimeout(progressBarTimeout); + progressBarTimeout = undefined; + } } export function scrubScriptElements(element:HTMLElement|DocumentFragment) { diff --git a/frontend/src/turbo/setup.ts b/frontend/src/turbo/setup.ts index 26916d8151b0..926704eece86 100644 --- a/frontend/src/turbo/setup.ts +++ b/frontend/src/turbo/setup.ts @@ -8,7 +8,7 @@ import { registerInputCaptionStreamAction } from './input-caption-stream-action' import { addTurboGlobalListeners } from './turbo-global-listeners'; import { applyTurboNavigationPatch } from './turbo-navigation-patch'; import { debugLog, whenDebugging } from 'core-app/shared/helpers/debug_output'; -import { TURBO_EVENTS } from './constants'; +import { getTurboEvents } from './utils'; import { StreamActions } from '@hotwired/turbo'; import { addTurboAngularWrapper } from 'core-turbo/turbo-angular-wrapper'; import { registerActionMenuMorphRemount } from './action-menu-morph-remount'; @@ -21,7 +21,7 @@ Turbo.start(); // Register logging of events whenDebugging(() => { - TURBO_EVENTS + getTurboEvents() .filter((name) => name !== 'turbo:before-stream-render') .forEach((name:string) => { document.addEventListener(name, (event) => { diff --git a/frontend/src/turbo/turbo-event-listeners.ts b/frontend/src/turbo/turbo-event-listeners.ts index a4e40c22d2bd..9c5e619757ca 100644 --- a/frontend/src/turbo/turbo-event-listeners.ts +++ b/frontend/src/turbo/turbo-event-listeners.ts @@ -10,8 +10,8 @@ export function addTurboEventListeners() { // it will leave an overflow:hidden attribute on the body, which prevents scrolling on the page. // // Also, we will dispatch a custom `dialog:close` event when the dialog is closed. - document.addEventListener('turbo:submit-end', (event:CustomEvent) => { - const { detail: { success }, target } = event as { detail:{ success:boolean }; target:EventTarget }; + document.addEventListener('turbo:submit-end', (event) => { + const { detail: { success }, target } = event; if (success && target instanceof HTMLFormElement) { const dialog = target.closest('dialog')!; @@ -28,7 +28,7 @@ export function addTurboEventListeners() { // Append turbo nonce for drive requests document.addEventListener('turbo:before-fetch-request', (event) => { // Turbo Drive does not send a referrer like turbolinks used to, so let's simulate it here - const headers = event.detail.fetchOptions.headers as Record; + const { headers } = event.detail.fetchOptions; headers['Turbo-Referrer'] = window.location.href; headers['X-Turbo-Nonce'] = document.getElementsByName('csp-nonce')[0]?.getAttribute('content') ?? ''; }); diff --git a/frontend/src/turbo/utils.ts b/frontend/src/turbo/utils.ts new file mode 100644 index 000000000000..e53a65cd18cb --- /dev/null +++ b/frontend/src/turbo/utils.ts @@ -0,0 +1,67 @@ +//-- copyright +// OpenProject is an open source project management software. +// Copyright (C) the OpenProject GmbH +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License version 3. +// +// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +// Copyright (C) 2006-2013 Jean-Philippe Lang +// Copyright (C) 2010-2013 the ChiliProject Team +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either version 2 +// of the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +// +// See COPYRIGHT and LICENSE files for more details. +//++ + +import { TurboGlobalEventHandlersEventMap } from '@hotwired/turbo'; + +type TurboEvent = keyof TurboGlobalEventHandlersEventMap; + +// Compile-time guard: TypeScript errors if any TurboEvent key is absent from the array +function allTurboEvents( + events:T & ([TurboEvent] extends [T[number]] ? unknown : never), +):readonly TurboEvent[] { + return events; +} + +export function getTurboEvents():readonly TurboEvent[] { + return allTurboEvents([ + 'turbo:before-cache', + 'turbo:before-fetch-request', + 'turbo:before-fetch-response', + 'turbo:before-frame-morph', + 'turbo:before-frame-render', + 'turbo:before-morph-attribute', + 'turbo:before-morph-element', + 'turbo:before-prefetch', + 'turbo:before-render', + 'turbo:before-stream-render', + 'turbo:before-visit', + 'turbo:click', + 'turbo:fetch-request-error', + 'turbo:frame-load', + 'turbo:frame-missing', + 'turbo:frame-render', + 'turbo:load', + 'turbo:morph-element', + 'turbo:morph', + 'turbo:reload', + 'turbo:render', + 'turbo:submit-end', + 'turbo:submit-start', + 'turbo:visit', + ] as const); +} diff --git a/frontend/src/typings/shims.d.ts b/frontend/src/typings/shims.d.ts index 6d5a1540d01d..5c26c463da38 100644 --- a/frontend/src/typings/shims.d.ts +++ b/frontend/src/typings/shims.d.ts @@ -25,33 +25,6 @@ declare module 'observable-array'; declare module 'dom-autoscroller'; declare module 'core-vendor/enjoyhint'; -declare module '@hotwired/turbo' { - interface BrowserAdapter { - formSubmissionStarted:() => void; - formSubmissionFinished:() => void; - } - - export const session:{ - drive:boolean; - adapter:BrowserAdapter; - }; - - export const config:{ - drive:{ progressBarDelay:number } - }; - - export const navigator:{ - submitForm:(form:HTMLFormElement, submitter?:HTMLElement) => void; - }; - - export interface StreamElement { - templateElement:HTMLTemplateElement; - templateContent:DocumentFragment; - } - - export function start():void; -} - declare global { const _:typeof TLodash; const I18n:I18n; diff --git a/spec/features/work_packages/cancel_editing_spec.rb b/spec/features/work_packages/cancel_editing_spec.rb index ac74b5baba0b..081f61ff1461 100644 --- a/spec/features/work_packages/cancel_editing_spec.rb +++ b/spec/features/work_packages/cancel_editing_spec.rb @@ -72,6 +72,8 @@ def move_to_home_page(alert: true) find(".op-logo--link").click end + expect(page).to have_current_path("/", ignore_query: true) + expect(page).to have_no_css("#wp-new-inline-edit--field-subject") expect(page).to have_css("#projects-menu", text: "All projects") end @@ -152,15 +154,18 @@ def move_to_home_page(alert: true) # Try to move back to list, expect warning page.execute_script("window.history.back()") + expect(wp_page).to have_alert_dialog wp_page.dismiss_alert_dialog! + description.expect_active! # Now cancel the field description.cancel_by_click + description.expect_inactive! # Now we should be able to get back to list page.execute_script("window.history.back()") - expect(wp_page.has_alert_dialog?).to be false + wait_for { wp_page.has_alert_dialog? }.to be false end context "when user does not want to be warned" do diff --git a/spec/features/work_packages/table/queries/query_history_spec.rb b/spec/features/work_packages/table/queries/query_history_spec.rb index d2ef31ae002a..97bc3a4eebf3 100644 --- a/spec/features/work_packages/table/queries/query_history_spec.rb +++ b/spec/features/work_packages/table/queries/query_history_spec.rb @@ -121,6 +121,7 @@ wp_table.expect_title(version_query.name) wp_table.expect_work_package_listed work_package_3 filters.expect_filter_count 2 + filters.ensure_open filters.expect_filter_by("Status", "open", nil) filters.expect_filter_by("Version", "is (OR)", version.name) @@ -128,7 +129,7 @@ wp_table.expect_title(assignee_query.name) wp_table.expect_work_package_listed work_package_2 - filters.open + filters.ensure_open filters.expect_filter_by("Status", "open", nil) filters.expect_filter_by("Assignee", "is (OR)", user.name) @@ -138,14 +139,14 @@ wp_table.expect_work_package_listed work_package_1 wp_table.expect_work_package_listed work_package_2 wp_table.expect_work_package_listed work_package_3 - filters.open + filters.ensure_open filters.expect_filter_by("Status", "open", nil) page.execute_script("window.history.forward()") wp_table.expect_title(assignee_query.name) wp_table.expect_work_package_listed work_package_2 - filters.open + filters.ensure_open filters.expect_filter_by("Status", "open", nil) filters.expect_filter_by("Assignee", "is (OR)", user.name) @@ -153,7 +154,7 @@ wp_table.expect_title(version_query.name) wp_table.expect_work_package_listed work_package_3 - filters.open + filters.ensure_open filters.expect_filter_by("Status", "open", nil) filters.expect_filter_by("Version", "is (OR)", version.name) @@ -161,7 +162,7 @@ wp_table.expect_title(version_query.name) wp_table.expect_no_work_package_listed - filters.open + filters.ensure_open filters.expect_filter_by("Status", "open", nil) filters.expect_filter_by("Version", "is (OR)", version.name) filters.expect_filter_by("Assignee", "is (OR)", user.name) diff --git a/spec/support/components/work_packages/filters.rb b/spec/support/components/work_packages/filters.rb index bcf7f3fc86c1..c1aceb952e6e 100644 --- a/spec/support/components/work_packages/filters.rb +++ b/spec/support/components/work_packages/filters.rb @@ -55,6 +55,14 @@ def open! expect_open end + def ensure_open + SeleniumHubWaiter.wait + expect_loaded + return if page.has_selector?(filters_selector, visible: :visible, wait: false) + + open + end + def expect_filter_count(num) expect(filter_button).to have_css(".badge", text: num, wait: 10) end