Fix PDF completion issues

kolibri/plugins/learn/assets/src/composables/__tests__/useProgressTracking.spec.js

@@ -1,4 +1,4 @@
-import { get } from '@vueuse/core';
+import { get, set } from '@vueuse/core';
 import client from 'kolibri.client';
 import { coreStoreFactory as makeStore } from 'kolibri.coreVue.vuex.store';
 import useProgressTracking from '../useProgressTracking';
@@ -373,6 +373,15 @@ describe('useProgressTracking composable', () => {
       expect(get(progress)).toEqual(1);
       expect(client.mock.calls[0][0].data.progress_delta).toEqual(0.5);
     });
+    it('should max progress and store progress_delta if progress is updated over threshold and over max value and progress_delta is greater than 0', async () => {
+      const { updateContentSession, progress, progress_delta } = await initStore({
+        progress: 0.167,
+      });
+      set(progress_delta, 0.5);
+      await updateContentSession({ progress: 1 });
+      expect(get(progress)).toEqual(1);
+      expect(client.mock.calls[0][0].data.progress_delta).toEqual(1);
+    });
     it('should not update progress and store progress_delta if progress is updated under current value', async () => {
       const { updateContentSession, progress, progress_delta } = await initStore();
       await updateContentSession({ progress: 0.4 });

kolibri/plugins/learn/assets/src/composables/useProgressTracking.js

@@ -350,8 +350,9 @@ export default function useProgressTracking(store) {
       progress = _zeroToOne(progress);
       progress = threeDecimalPlaceRoundup(progress);
       if (get(progress_state) < progress) {
-        const newProgressDelta =
-          get(progress_delta) + threeDecimalPlaceRoundup(progress - get(progress_state));
+        const newProgressDelta = _zeroToOne(
+          get(progress_delta) + threeDecimalPlaceRoundup(progress - get(progress_state))
+        );
         set(progress_delta, newProgressDelta);
         set(progress_state, progress);
       }

kolibri/plugins/pdf_viewer/assets/src/views/PdfRendererIndex.vue

@@ -17,7 +17,6 @@
     <template v-else>
       <transition name="slide">
         <div
-          v-if="showControls"
           class="fullscreen-header"
           :style="{ backgroundColor: this.$themePalette.grey.v_100 }"
         >
@@ -115,7 +114,6 @@
       isInFullscreen: false,
       currentLocation: 0,
       updateContentStateInterval: null,
-      showControls: true,
       visitedPages: {},
     }),
     computed: {
@@ -166,6 +164,13 @@
         return this.totalPages * 30;
       },
       /* eslint-enable kolibri/vue-no-unused-properties */
+      debouncedShowVisiblePages() {
+        // So as not to share debounced functions between instances of the same component
+        // and also to allow access to the cancel method of the debounced function
+        // best practice seems to be to do it as a computed property and not a method:
+        // https://github.com/vuejs/vue/issues/2870#issuecomment-219096773
+        return debounce(this.showVisiblePages, renderDebounceTime);
+      },
     },
     watch: {
       recycleListIsMounted(newVal) {
@@ -196,14 +201,6 @@
           this.debounceForceUpdateRecycleList();
         }
       },
-      // Listen to change in scroll position to determine whether we show top control bar or not
-      currentLocation(newPos, oldPos) {
-        if (newPos > oldPos) {
-          this.showControls = false;
-        } else {
-          this.showControls = true;
-        }
-      },
     },
     destroyed() {
       // Reset the overflow on the HTML tag that we set to hidden in created()
@@ -215,7 +212,6 @@
       window.document.getElementsByTagName('html')[0].style.overflow = 'hidden';
 
       this.currentLocation = this.savedLocation;
-      this.showControls = true; // Ensures it shows on load even if we're scrolled
       const loadPdfPromise = PDFJSLib.getDocument(this.defaultFile.storage_url);
       // pass callback to update loading bar
       loadPdfPromise.onProgress = loadingProgress => {
@@ -301,7 +297,7 @@
         this.savedVisitedPages = visited;
       },
       // handle the recycle list update event
-      handleUpdate: debounce(function(start, end) {
+      handleUpdate(start, end) {
         // check that it is mounted
         if (!this.$refs.recycleList || !this.$refs.recycleList.$el) {
           return;
@@ -325,19 +321,26 @@
           // TODO: there is a miscalculation that causes a wrong position change on scale
           this.savePosition(this.calculatePosition());
 
-          // determine how many pages user has viewed/visited; fix edge case of 2 pages
-          let currentPage =
-            this.totalPages === 2 ? 2 : parseInt(this.currentLocation * this.totalPages) + 1;
+          // determine how many pages user has viewed/visited
+          const currentPage = parseInt(this.currentLocation * this.totalPages) + 1;
+          // If the user has already scrolled all the way to the end and is still not scrolled
+          // to the final page, set the final page as viewed as well.
+          if (currentPage === this.totalPages - 1 && this.scrolledToEnd()) {
+            this.storeVisitedPage(currentPage + 1);
+          }
           this.storeVisitedPage(currentPage);
           this.updateProgress();
           this.updateContentState();
         }
+        this.debouncedShowVisiblePages(start, end);
+      },
+      showVisiblePages(start, end) {
         const startIndex = Math.floor(start) + 1;
         const endIndex = Math.ceil(end) + 1;
         for (let i = startIndex; i <= endIndex; i++) {
           this.showPage(i);
         }
-      }, renderDebounceTime),
+      },
       zoomIn() {
         this.setScale(Math.min(scaleIncrement * 20, this.scale + scaleIncrement));
       },
@@ -350,6 +353,12 @@
       calculatePosition() {
         return this.$refs.recycleList.$el.scrollTop / this.$refs.recycleList.$el.scrollHeight;
       },
+      scrolledToEnd() {
+        return (
+          this.$refs.recycleList.$el.scrollTop + this.$refs.recycleList.$el.clientHeight ===
+          this.$refs.recycleList.$el.scrollHeight
+        );
+      },
       savePosition(val) {
         this.currentLocation = val;
       },