Visualizer: Refresh scale bar on home button and resource focus#1064
Merged
BioCam merged 1 commit intoMay 26, 2026
Merged
Conversation
updateScaleBar was defined inside the load handler, so it was closure- scoped and invisible to the top-level fitToViewport and focusOnResource. Both guarded the call with `if (typeof updateScaleBar === "function")`, which silently evaluated to false in global scope. As a result the home button and resource-focus paths left the scale bar showing the previous zoom's value, while the wheel-zoom path - which lives inside the same load handler - worked correctly. The proximate cause is the scope mismatch; the underlying cause is that the five-updater refresh sequence (updateScaleBar, updateBullseyeScale, updateWrtBullseyeScale, updateTooltipScale, updateDeltaLinesScale) was duplicated at four view-mutation sites. When updateScaleBar was added, one of the four copies was missed. Hoist updateScaleBar to top level next to its sibling updaters and add an `if (!stage) return;` guard so pre-load callers stay silent. Extract the refresh sequence into refreshScaleOverlays and route fitToViewport, the wheel handler, zoomByFactor, and focusOnResource through it. The now-dead `typeof updateScaleBar` guards are removed, and const/let become var to match sibling style. Closes PyLabRobot#898. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #898.
Problem
When the home button is pressed, the scale bar keeps showing the previous zoom's value instead of refreshing to match the reset view. Clicking a resource to focus on it has the same latent bug. The other scale-dependent overlays (bullseye, WRT-bullseye, tooltip, delta lines) update correctly - only the scale bar is stuck.
Root cause
updateScaleBarwas defined inside thewindow.addEventListener("load", ...)handler, so it was closure-scoped and invisible to the top-levelfitToViewportandfocusOnResource. Both guarded the call withif (typeof updateScaleBar === "function"), which silently evaluated to false in global scope. The wheel-zoom and zoom-button paths worked because their call sites live inside the same closure. The deeper cause is that the five-updater refresh sequence was duplicated at four view-mutation sites; whenupdateScaleBarwas added later, one copy was missed - andfocusOnResourcecarried the same latent miss.Change
Hoist
updateScaleBarto top level next to its sibling updaters, with anif (!stage) return;guard so pre-load callers stay silent. Drop the now-deadtypeofchecks. Extract the five-updater sequence intorefreshScaleOverlaysand routefitToViewport, the wheel handler,zoomByFactor, andfocusOnResourcethrough it. Any future view-mutation entry point now wires to a single helper instead of remembering five updaters.Net: +34 / -40 in
pylabrobot/visualizer/lib.js. No behaviour change to paths that already worked.