From d641a632361f9e018325322a5539c8d0a7e76240 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Mon, 20 Nov 2023 20:14:02 +0000 Subject: [PATCH] UX: Ensure loading slider does not 'reset' halfway through a transition (#24446) For transitions to nested routes (e.g. /u/blah/activity), where each layer has an async model hook, the `loading` event will be fired twice within the same transition. This was causing the loading slider to jump backwards halfway through loading. This commit fixes things to handle nested loading events with a single animation. --- .../discourse/app/services/loading-slider.js | 9 +++++++++ .../tests/acceptance/loading-indicator-test.js | 16 ++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/app/assets/javascripts/discourse/app/services/loading-slider.js b/app/assets/javascripts/discourse/app/services/loading-slider.js index e0d4bdfe08a..850a2ee644e 100644 --- a/app/assets/javascripts/discourse/app/services/loading-slider.js +++ b/app/assets/javascripts/discourse/app/services/loading-slider.js @@ -83,6 +83,11 @@ export default class LoadingSlider extends Service.extend(Evented) { } transitionStarted() { + if (this.loading) { + // Nested transition + return; + } + this.timer.start(); this.loading = true; this.trigger("stateChanged", true); @@ -97,6 +102,10 @@ export default class LoadingSlider extends Service.extend(Evented) { @bind transitionEnded() { + if (!this.loading) { + return; + } + let duration = this.timer.stop(); if (duration < MIN_LOADING_TIME) { duration = MIN_LOADING_TIME; diff --git a/app/assets/javascripts/discourse/tests/acceptance/loading-indicator-test.js b/app/assets/javascripts/discourse/tests/acceptance/loading-indicator-test.js index 2a0b4cbc550..af62e697e3d 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/loading-indicator-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/loading-indicator-test.js @@ -1,3 +1,4 @@ +import { getOwner } from "@ember/application"; import { currentRouteName, getSettledState, @@ -96,4 +97,19 @@ acceptance("Page Loading Indicator", function (needs) { assert.strictEqual(currentRouteName(), "about"); assert.dom("#main-outlet section.about").exists(); }); + + test("it only performs one slide during nested loading events", async function (assert) { + this.siteSettings.page_loading_indicator = "slider"; + + await visit("/"); + + const service = getOwner(this).lookup("service:loading-slider"); + service.on("stateChanged", (loading) => { + assert.step(`loading: ${loading}`); + }); + + await visit("/u/eviltrout/activity"); + + assert.verifySteps(["loading: true", "loading: false"]); + }); });