Show spinner while loading ZIM content in viewer iframe #1183

Optimus-NP · 2025-02-28T08:57:57Z

Fixes #731

Implemented a spinner to improve user experience while ZIM content is loading in the viewer iframe.
Added .loader and .spinner styles in kiwix.css.
The iframe content is initially hidden (visibility: hidden) and will be displayed once loading completes.
Used CSS animations (@Keyframes spin) for a smooth rotating effect.

Demo

kiwix_issue_731.mp4

Command

/usr/local/bin/kiwix-serve --port=8080 --library /tmp/zimlibrary/linux_library.xml --monitorLibrary

Optimus-NP · 2025-03-09T19:04:56Z

Hi @kelson42,

I've added a spinner to improve the user experience while ZIM content is loading in the viewer iframe. The iframe is initially hidden and becomes visible once loading completes. I also added styles in kiwix.css and used CSS animations for a smooth effect.

Could you review the changes and let me know if any improvements are needed? Thanks!

benoit74 · 2025-03-09T20:25:03Z

@Optimus-NP thank you, I'm not a maintainer on this repo, but I see one potential flaw, can you check how your spinner render on smaller screens (e.g. phones in portrait mode). I'm concerned about the fact that text might be split on two rows which is probably a bit ugly.

Optimus-NP · 2025-03-11T15:18:25Z

Thanks @benoit74 for the feedback. I have addressed the comment.
Requesting for the review.

kiwix_issue_mob_731.mp4

benoit74 · 2025-03-11T15:43:19Z

@kelson42 who should review this?

Optimus-NP · 2025-03-15T19:21:58Z

Hi @kelson42

Requesting you to review this PR.

Optimus-NP · 2025-03-17T15:55:11Z

@veloman-yunkan

Please review this PR.

veloman-yunkan

Failures of the unit-tests must be fixed by updating the cacheids of the modified static resources (the new values can be seen in the test failure reports).

veloman-yunkan · 2025-03-20T08:39:13Z

static/skin/viewer.js

@@ -425,6 +425,10 @@ let viewerSetupComplete = false;

 function on_content_load() {
  if ( viewerSetupComplete ) {
+    const loader = document.getElementById("kiwix__loader");
+    const iframe = document.getElementById("content_iframe");


Use the readily available global variable contentIframe

Thanks @veloman-yunkan

I appreciate your well-thought-out recommendations. They will certainly help improve the outcome.

veloman-yunkan · 2025-03-20T08:42:29Z

static/skin/viewer.js

+    const loader = document.getElementById("kiwix__loader");
+    const iframe = document.getElementById("content_iframe");
+    iframe.style.visibility = "visible";
+    loader.style.display = "none";


I believe this should be moved out of the if statement (it doesn't depend on the viewer setup, which is mostly about loading translations).

I have now integrated for the i18n for the loading content message so I need to wait for the translations to happen.

veloman-yunkan · 2025-03-20T08:43:11Z

static/viewer.html

@@ -30,9 +30,12 @@
        location.href = root + '/';
      }
    </script>
-  </head>
-
+</head>


Fix indentation

Thanks for catching that.

veloman-yunkan · 2025-03-20T08:57:05Z

static/viewer.html

  <body style="margin:0" onload="setupViewer()">
+    <div class="loader" id="kiwix__loader">
+      <div class="spinner"></div>
+      Loading content...


This text is not internationalized/translated. You will have to update the function updateUIText() in viewer.js. Please do that in a separate commit.

I sincerely appreciate your guidance on i18n. I wasn’t familiar with how to use and implement it, but thanks to your help, I now understand its importance and how to integrate it effectively. Your insights were incredibly valuable—thank you!

veloman-yunkan · 2025-03-20T09:10:17Z

static/skin/kiwix.css

+.content-iframe {
+  visibility: hidden; /* Hide initially */
+}


Better rename this class to hidden or invisible and then dynamically remove it from from the classList of the content iframe element in order to make it visible.

That’s a solid idea, appreciate you sharing it.

Optimus-NP · 2025-03-22T22:03:41Z

Failures of the unit-tests must be fixed by updating the cacheids of the modified static resources (the new values can be seen in the test failure reports).

Thanks this fixes the test failure issues.

Optimus-NP · 2025-03-22T22:11:52Z

@veloman-yunkan

Thanks for the review. I have addressed the comment.

Optimus-NP · 2025-03-22T22:12:24Z

@veloman-yunkan @kelson42

Requesting you to review this PR.

veloman-yunkan · 2025-03-25T06:59:09Z

static/skin/i18n/ar.json

@@ -34,5 +34,6 @@
 	"magnet-link-text": "رابط جذب {رابط جاذب}",
 	"magnet-alt-text": "التحميل {التنزيل} عن طريق/بواسطة رابط الجذب\n{الرابط الجاذب}",
 	"torrent-download-link-text": "بت تورنت",
-	"torrent-download-alt-text": "التحميل {التنزيل} عن طريق/بواسطة بت تورنت"
+	"torrent-download-alt-text": "التحميل {التنزيل} عن طريق/بواسطة بت تورنت",
+	"text-loading-content": "جاري تحميل المحتوى"


I wonder where the translations of the new i18n string came from. Did you add them yourself? If so, then please undo those changes. Be advised that translation is outsourced to https://translatewiki.net and its results are integrated into our code via automated pull requests like, for example, #1179. The only files that should be manually modified under static/skin/i18n directory are en.json (the source text for translation), qqq.json (provides context for translation), and test.json (used in unit tests).

Thanks for pointing that out. I’ve reverted the changes to the translation files and ensured that only en.json, qqq.json, and test.json are manually modified as per the guidelines. I'll make sure to follow the correct workflow via TranslateWiki for future updates.

veloman-yunkan · 2025-03-25T07:05:31Z

static/viewer.html

@@ -25,14 +25,16 @@

      const root = getRootLocation();
      const blankPageUrl = root + "/skin/blank.html?KIWIXCACHEID";
-
-      if ( location.hash == '' ) {
+      if (location.hash == '') {


Undo this change

I have reverted this change.

veloman-yunkan · 2025-03-25T07:06:02Z

static/viewer.html

  <body style="margin:0" onload="setupViewer()">
+    <div class="loader" id="kiwix__loader">
+      <div class="spinner"></div>
+      <span id="kiwix__loading_text">Loading content...</span> <!-- Added ID for translation -->


The comment has no value. Please remove it.

I have removed the comment as requested.

veloman-yunkan · 2025-03-25T07:07:14Z

static/viewer.html

@@ -78,7 +80,7 @@
      </div>
    </div>

-    <iframe id="content_iframe"
+    <iframe id="content_iframe" class="content-iframe"


There is no CSS class content-iframe anymore - it has been renamed to hidden

Thanks for the update. I’ve updated the references accordingly to use hidden instead of content-iframe.

veloman-yunkan · 2025-03-25T07:10:38Z

static/skin/viewer.js

+    //viewerSetupComplete is marked as true once the translations are loaded
+    //Now we can load the spinner.
+    const loader = document.getElementById("kiwix__loader");
+
+    contentIframe.classList.remove("hidden");
+    loader.style.display = "none";


Hiding the spinner and showing the content doesn't in any way depend on the translations being loaded and therefore must be performed outside the if.

Thanks for the clarification. I’ve updated the logic to ensure that hiding the spinner and showing the content happens independently of the translations being loaded.

Optimus-NP · 2025-03-25T17:52:16Z

@veloman-yunkan

Thanks for the review. I have addressed the comment.

veloman-yunkan

LGTM, but before we merge please remove the reference to the issue number from the commit title (since it points to the wrong issue as the issue addressed is in a different repository).

Optimus-NP · 2025-03-26T15:44:20Z

LGTM, but before we merge please remove the reference to the issue number from the commit title (since it points to the wrong issue as the issue addressed is in a different repository).

Thank you for catching that. Addressed.

Optimus-NP · 2025-03-26T15:44:50Z

@kelson42 @veloman-yunkan

Please help in merging this.

veloman-yunkan · 2025-03-26T16:02:02Z

LGTM, but before we merge please remove the reference to the issue number from the commit title (since it points to the wrong issue as the issue addressed is in a different repository).

Thank you for catching that. Addressed.

Not really. Please remove the reference to the issue number from the commit title. The PR title could be fixed after we merge. The commit title - once merged - will stay there forever.

- Implemented a spinner to improve user experience while ZIM content is loading in the viewer iframe. - Added .loader and .spinner styles in kiwix.css. - The iframe content is initially hidden (visibility: hidden) and will be displayed once loading completes. - Used CSS animations (@Keyframes spin) for a smooth rotating effect.

Optimus-NP · 2025-03-26T18:07:15Z

LGTM, but before we merge please remove the reference to the issue number from the commit title (since it points to the wrong issue as the issue addressed is in a different repository).

Thank you for catching that. Addressed.

Not really. Please remove the reference to the issue number from the commit title. The PR title could be fixed after we merge. The commit title - once merged - will stay there forever.

done @veloman-yunkan

Optimus-NP · 2025-03-26T18:07:31Z

@kelson42 @veloman-yunkan

Please help in merging this.

Optimus-NP · 2025-03-27T08:54:51Z

@kelson42

Please help in merging this.

Optimus-NP force-pushed the kiwix-tools-issue_731 branch from f57da12 to fc2a6f6 Compare March 11, 2025 15:12

kelson42 force-pushed the kiwix-tools-issue_731 branch from fc2a6f6 to fc89461 Compare March 17, 2025 18:47

kelson42 requested review from veloman-yunkan and kelson42 March 17, 2025 19:33

veloman-yunkan requested changes Mar 20, 2025

View reviewed changes

Optimus-NP force-pushed the kiwix-tools-issue_731 branch 2 times, most recently from f598cb2 to 4a0428a Compare March 22, 2025 21:43

Optimus-NP requested a review from veloman-yunkan March 22, 2025 22:08

veloman-yunkan requested changes Mar 25, 2025

View reviewed changes

Optimus-NP force-pushed the kiwix-tools-issue_731 branch 4 times, most recently from 08ec098 to da87a5f Compare March 25, 2025 16:05

Optimus-NP requested a review from veloman-yunkan March 25, 2025 17:52

veloman-yunkan reviewed Mar 26, 2025

View reviewed changes

Optimus-NP changed the title ~~Show spinner while loading ZIM content in viewer iframe (#731)~~ Show spinner while loading ZIM content in viewer iframe Mar 26, 2025

Optimus-NP requested a review from veloman-yunkan March 26, 2025 15:45

Optimus-NP force-pushed the kiwix-tools-issue_731 branch from da87a5f to c7e86c9 Compare March 26, 2025 16:22

veloman-yunkan approved these changes Mar 27, 2025

View reviewed changes

kelson42 merged commit a0c99f8 into kiwix:main Mar 30, 2025
13 checks passed

Show spinner while loading ZIM content in viewer iframe #1183

Show spinner while loading ZIM content in viewer iframe #1183

Conversation

Optimus-NP commented Feb 28, 2025 • edited Loading

Demo

Command

Optimus-NP commented Mar 9, 2025

benoit74 commented Mar 9, 2025

Optimus-NP commented Mar 11, 2025

benoit74 commented Mar 11, 2025

Optimus-NP commented Mar 15, 2025

Optimus-NP commented Mar 17, 2025

veloman-yunkan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Optimus-NP commented Mar 22, 2025

Optimus-NP commented Mar 22, 2025

Optimus-NP commented Mar 22, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Optimus-NP commented Mar 25, 2025

veloman-yunkan left a comment

Choose a reason for hiding this comment

Optimus-NP commented Mar 26, 2025

Optimus-NP commented Mar 26, 2025

veloman-yunkan commented Mar 26, 2025

Optimus-NP commented Mar 26, 2025

Optimus-NP commented Mar 26, 2025

Optimus-NP commented Mar 27, 2025

Optimus-NP commented Feb 28, 2025 •

edited

Loading