Skip to content

Conversation

@Crazyidiott
Copy link

@Crazyidiott Crazyidiott commented Apr 29, 2025

Fix for Thumbnail Mode Navigation Bug

This PR fixes the issue where page flipping in thumbnail mode becomes stuck, with visual display showing the new page but page index incorrectly reverting to the previous page. A typo in ModeThumb.js is also fixed.

Problem Analysis:
✅ The root cause of this issue has been identified:

  • When jumpToIndex method calls this.br.drawLeafs()(invoked when animating) instead of this.drawLeafs(index), no parameter is passed. Thus scrollTop is not reset by seekTop
  • During animation, scrollTop is underestimated, thus leastVisible and mostVisible are not calculated as desired
  • Therefore the current index is not within the visible range (leastVisible to mostVisible)
  • The method incorrectly resets the firstIndex to the last visible page

Fix Implementation:
✅ Two key changes were made to resolve this issue:

  1. Prevent index modification during animation:

    • Added a check to avoid modifying the firstIndex when animation is in progress
    if (!this.br.animating) {
      if (currentIndex < leastVisible) {
        this.br.updateFirstIndex(leastVisible);
      } else if (currentIndex > mostVisible) {
        this.br.updateFirstIndex(mostVisible);
      }
    }
  2. Ensure proper redraw after animation completes:

    • Changed this.br.drawLeafs() to this.drawLeafs(index)
    • Added explicit call to this.drawLeafs(index) after animation completes
     if (this.br.refs.$brContainer.prop('scrollTop') == leafTop) {
       this.drawLeafs(index);
     } else {
       this.br.animating = true;
       this.br.refs.$brContainer.stop(true)
         .animate({ scrollTop: leafTop }, 'fast', () => {
           this.br.animating = false;
           this.drawLeafs(index);
       });
     }

🛠 Testing Done:

  • ✅ Verified page flipping in thumbnail mode now correctly maintains page index
  • ✅ Confirmed no regression in other viewing modes

✅ Expected Outcome:

  • Page flipping in thumbnail mode now correctly updates both visual display and page index
  • Smooth navigation experience with no unexpected index resets

Observation

  • When I was testing, I found that this.br.refs.$brContainer.prop('scrollTop') == leafTop never returns true. There is always a difference between them. I didn't figure out the reason.

I hope this helps!

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.

1 participant