Skip to content

Fix save modal showing nonsensical 'Saving page X of Y' when X > Y#996

Merged
Tim020 merged 2 commits into
devfrom
fix/save-modal-page-count
May 7, 2026
Merged

Fix save modal showing nonsensical 'Saving page X of Y' when X > Y#996
Tim020 merged 2 commits into
devfrom
fix/save-modal-page-count

Conversation

@Tim020
Copy link
Copy Markdown
Contributor

@Tim020 Tim020 commented May 7, 2026

Summary

  • Bug: The "Saving Script" modal displayed the raw script page number as X and the count of loaded pages as Y, producing impossible values like "Saving page 13 of 1"
  • Root cause: totalSavePages was set to Object.keys(TMP_SCRIPT).length (count of loaded pages) while curSavePage was set to the actual page number — two different units in the same "X of Y" display
  • Fix: Loop from 1 to Math.max(currentMaxPage, ...TMP_SCRIPT keys) so both values share the same scale. Taking the max of both sources handles existing pages (via currentMaxPage from the backend) and newly added pages that haven't been saved yet (which live in TMP_SCRIPT above currentMaxPage). Pages not loaded into TMP_SCRIPT are skipped silently, hiding the implementation detail that only changed pages are actually saved

Test plan

  • Open a show with a script that has content beyond page 1
  • Navigate to a later page (e.g. page 13), make an edit, and click Save
  • Confirm the modal now shows "Saving page 1 of N" → "Saving page N of N" where N = total script pages — never X > Y
  • Add a brand new page beyond the current max, make an edit, and click Save — confirm the new page is included in the count and is saved correctly
  • Confirm the progress bar fills smoothly left to right with no value exceeding the max
  • Verify autosave toast is unaffected (shows actual page number, no "of Y" — unchanged)

🤖 Generated with Claude Code

The modal displayed the raw script page number as X and the count of
pages in TMP_SCRIPT as Y, so editing a single page at a high page
number (e.g. page 13) showed "Saving page 13 of 1".

Fix by iterating 1..currentMaxPage so the progress reflects the full
script length, skipping pages not loaded into TMP_SCRIPT transparently.
This also avoids surfacing the implementation detail that only changed
pages are saved.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions github-actions Bot added small-diff Small pull request client Pull requests changing front end code labels May 7, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 7, 2026

Client Test Results

99 tests  ±0   99 ✅ ±0   0s ⏱️ ±0s
 4 suites ±0    0 💤 ±0 
 1 files   ±0    0 ❌ ±0 

Results for commit 1f5d562. ± Comparison against base commit 266e3d8.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 7, 2026

Python Test Results

  1 files  ±0    1 suites  ±0   1m 24s ⏱️ -1s
585 tests ±0  585 ✅ ±0  0 💤 ±0  0 ❌ ±0 
590 runs  ±0  590 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 1f5d562. ± Comparison against base commit 266e3d8.

♻️ This comment has been updated with latest results.

currentMaxPage alone missed newly added pages (page numbers above the
last backend-saved page). Use Math.max(currentMaxPage, ...TMP_SCRIPT
keys) so new pages are included in both the loop bound and the total
shown to the user.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 7, 2026

@Tim020 Tim020 enabled auto-merge (squash) May 7, 2026 23:55
@Tim020 Tim020 merged commit 31e97a2 into dev May 7, 2026
25 checks passed
@Tim020 Tim020 deleted the fix/save-modal-page-count branch May 7, 2026 23:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

client Pull requests changing front end code small-diff Small pull request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant