Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(QImg): add spinner-delay prop #16932

Closed
wants to merge 3 commits into from
Closed

Conversation

thexeos
Copy link
Contributor

@thexeos thexeos commented Feb 26, 2024

What kind of change does this PR introduce?

  • Feature

Does this PR introduce a breaking change?

  • No

The PR fulfills these requirements:

  • It's submitted to the dev branch (or v[X] branch)
  • It's been tested on a Cordova (iOS, Android) app
  • It's been tested on an Electron app
  • Any necessary documentation has been added or updated in the docs or explained in the PR's description.

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature

Other information:

We use QImg for displaying user avatars and country flags. We keep the spinner enabled, as it helps display the loading state to the user. After the first visit, the user will likely have some or all images cached, causing the spinner to quickly "flash" over the almost-instantly loaded image.

This pull request adds a spinner-delay prop which allows delaying the display of the spinner until after a short (user-defined) period of time has passed since the image begun loading.

The value is customizable, as it is context-dependent and only the developer knows how long it should be. For example, setting the value to 50 (milliseconds) is enough for loading isolated images within a single viewport, however loading dozens or hundreds of images in a tight grid will hog the browser's cache and network modules and would require a much larger delay (e.g., 1000 milliseconds).

The default value is 0 - consistent with current behavior.

We have been using this code for almost 2 years and it is stable.

chrome_QhRsBy6FCx

Copy link

Build Results

JSON API

📜 Changes detected:

diff --git a/./current-build/api/QImg.json b/./pr-build/api/QImg.json
index 8cf5690..db2fc44 100644
--- a/./current-build/api/QImg.json
+++ b/./pr-build/api/QImg.json
@@ -226,6 +226,17 @@
       ],
       "category": "style"
     },
+    "spinner-delay": {
+      "type": "Number",
+      "desc": "Delay showing the spinner when image changes; gives time for the browser to load the image from cache to prevent flashing the spinner unnecesarily",
+      "default": 0,
+      "examples": [
+        500,
+        ":spinner-delay=\"550\""
+      ],
+      "category": "behavior",
+      "required": false
+    },
     "no-spinner": {
       "type": "Boolean",
       "desc": "Do not display the default spinner while waiting for the image to be loaded; It is overriden by the 'loading' slot when one is present",

Types

📜 Changes detected:

diff --git a/./current-build/types/index.d.ts b/./pr-build/types/index.d.ts
index 7781861..4d52dff 100644
--- a/./current-build/types/index.d.ts
+++ b/./pr-build/types/index.d.ts
@@ -5895,6 +5895,10 @@ export interface QImgProps {
    * Size in CSS units, including unit name, for default Spinner (unless using a 'loading' slot)
    */
   spinnerSize?: string | undefined;
+  /**
+   * Delay showing the spinner when image changes; gives time for the browser to load the image from cache to prevent flashing the spinner unnecesarily
+   */
+  spinnerDelay?: number | undefined;
   /**
    * Do not display the default spinner while waiting for the image to be loaded; It is overriden by the 'loading' slot when one is present
    */

Copy link

UI Tests Results

0 tests   0 ✅  0s ⏱️
0 suites  0 💤
2 files    0 ❌
2 errors

For more details on these parsing errors, see this check.

Results for commit fa7f475.

@rstoenescu
Copy link
Member

Hi,

First of all, thank you for taking the time to contribute! Even though you have this working, you still wanted to improve Quasar with your feature, which is something that I can only applaud.

Went with a slightly different approach (delaying isLoading directly) and did some other refactoring.

Will be available in Quasar v2.14.6.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants