diff --git a/frontend/src/global_styles/content/modules/_backlogs.sass b/frontend/src/global_styles/content/modules/_backlogs.sass index 01a6729b2c3d..928bba7162f2 100644 --- a/frontend/src/global_styles/content/modules/_backlogs.sass +++ b/frontend/src/global_styles/content/modules/_backlogs.sass @@ -44,6 +44,10 @@ container-name: backlogsListsContainer container-type: inline-size +// Display the loading indicator while the #backlogs_container is loading +body:has(turbo-frame#backlogs_container[busy]) #global-loading-indicator + display: block !important + .op-sprint-planning-container display: flex flex-direction: row diff --git a/frontend/src/turbo/turbo-navigation-patch.spec.ts b/frontend/src/turbo/turbo-navigation-patch.spec.ts index 619d253f6d0d..b0e44c0d683f 100644 --- a/frontend/src/turbo/turbo-navigation-patch.spec.ts +++ b/frontend/src/turbo/turbo-navigation-patch.spec.ts @@ -26,7 +26,7 @@ //++ import * as Turbo from '@hotwired/turbo'; -import { applyTurboNavigationPatch } from './turbo-navigation-patch'; +import { applyTurboNavigationPatch, stripStaleBusyState } from './turbo-navigation-patch'; // The FrameController prototype the patch reaches into. Internal Turbo API, // not part of the public types, hence the casts. @@ -60,3 +60,31 @@ describe('turbo-navigation-patch (tripwire for hotwired/turbo#1300)', () => { expect(String(patched)).toContain('ownerDocument'); }); }); + +describe('stripStaleBusyState', () => { + it('removes busy and aria-busy from every busy turbo-frame in the subtree', () => { + const root = document.createElement('div'); + root.innerHTML = ` + +
+ + `; + + stripStaleBusyState(root); + + expect(root.querySelector('#a')!.hasAttribute('busy')).toBe(false); + expect(root.querySelector('#a')!.hasAttribute('aria-busy')).toBe(false); + expect(root.querySelector('#b')!.hasAttribute('busy')).toBe(false); + expect(root.querySelector('#b')!.hasAttribute('aria-busy')).toBe(false); + expect(root.querySelector('#c')!.hasAttribute('busy')).toBe(false); + }); + + it('leaves unrelated content untouched', () => { + const root = document.createElement('div'); + root.innerHTML = '

content

'; + + stripStaleBusyState(root); + + expect(root.querySelector('p')!.textContent).toBe('content'); + }); +}); diff --git a/frontend/src/turbo/turbo-navigation-patch.ts b/frontend/src/turbo/turbo-navigation-patch.ts index d8e80b5ac461..9e6b1119a4fc 100644 --- a/frontend/src/turbo/turbo-navigation-patch.ts +++ b/frontend/src/turbo/turbo-navigation-patch.ts @@ -30,6 +30,20 @@ // part of the public type surface, so every access below is untyped. import * as Turbo from '@hotwired/turbo'; +// Turbo marks a turbo-frame `busy`/`aria-busy` for the duration of a +// navigation, but for a form-submission-driven navigation it doesn't clear +// that state until well after `proposeVisitIfNavigatedWithAction` below has +// already cloned the frame into the history-cache snapshot -- so the clone +// freezes the frame as permanently busy. `turbo:before-cache` fires too late +// to fix this: by then the clone is already a detached copy, disconnected +// from the live DOM. Exported for direct unit testing. +export function stripStaleBusyState(root: ParentNode): void { + root.querySelectorAll('turbo-frame[busy]').forEach((frame) => { + frame.removeAttribute('busy'); + frame.removeAttribute('aria-busy'); + }); +} + /** * Workaround for https://github.com/hotwired/turbo/issues/1300: with a * ``, the URL advances but the @@ -41,10 +55,13 @@ import * as Turbo from '@hotwired/turbo'; * ships in a release we depend on, drop this patch in favor of it. * * This is a verbatim fork of `FrameController.proposeVisitIfNavigatedWithAction` - * with a single divergence: the history snapshot is built from the live - * document (`frame.ownerDocument.documentElement.outerHTML`) instead of the - * fetch response (`await fetchResponse.responseHTML`), so the restored snapshot - * reflects the fully rendered page rather than the bare frame fragment. + * with two divergences: + * - The history snapshot is built from the live document + * (`frame.ownerDocument.documentElement.outerHTML`) instead of the fetch + * response (`await fetchResponse.responseHTML`), so the restored snapshot + * reflects the fully rendered page rather than the bare frame fragment. + * - `stripStaleBusyState` clears any frame left stuck `busy` in that snapshot + * (see its own comment above). * * Pinned to @hotwired/turbo 8.0.x. `turbo-navigation-patch.spec.ts` is a * tripwire: it fails if Turbo renames the patched method or stops sourcing the @@ -57,6 +74,16 @@ export function applyTurboNavigationPatch() { if (this.action) { const pageSnapshot = Turbo.PageSnapshot.fromElement(frame).clone(); + + // Form submission cycle: + // Form marked busy, frame marked busy -> Request starts -> Request Completes -> proposeVisit called -> + // it caches the pageSnapshot which is still marked busy -> Issues a visit -> visitCachedSnapshot + // will update the stale pageSnapshot with the current previousFrameElement to keep a fresh copy. + // But the pageSnapshot busy attribute is untouched. Ideally visitCacheSnapshot would remove the + // busy attribute from the pageSnapshot, but since that is not happening, the busy state is stripped + // from the pageSnapshot here. + stripStaleBusyState(pageSnapshot.element); + const { visitCachedSnapshot } = frame.delegate; // Kept `async` to mirror upstream's method shape even though the body no diff --git a/modules/backlogs/spec/features/backlogs/filters_spec.rb b/modules/backlogs/spec/features/backlogs/filters_spec.rb index f58490d09cdc..94efce696779 100644 --- a/modules/backlogs/spec/features/backlogs/filters_spec.rb +++ b/modules/backlogs/spec/features/backlogs/filters_spec.rb @@ -96,6 +96,21 @@ def create_sprint_wp(subject:, status: status_a) = create(:work_package, project backlogs_page.expect_inbox_work_package_count(1) end + it "shows the loading spinner on the backlogs container while the subject quick search is in flight" do + expect(page).to have_no_css("turbo-frame#backlogs_container[busy]") + expect(page).to have_no_css("#global-loading-indicator", visible: :visible) + + # Not using `apply_subject_filter`, as it waits for the network to go + # idle, which would hide the very state under test here. + fill_in "Search work packages by subject", with: "Needle" + + expect(page).to have_css("turbo-frame#backlogs_container[busy]") + expect(page).to have_css("#global-loading-indicator", visible: :visible) + + expect(page).to have_no_css("turbo-frame#backlogs_container[busy]") + expect(page).to have_no_css("#global-loading-indicator", visible: :visible) + end + it "narrows the listings when an advanced filter is applied" do backlogs_page.expect_bucket_items(bucket, items: status_b_bucket_wp) backlogs_page.expect_sprint_items(sprint, items: status_b_sprint_wp)