Skip to content

Pause/resume for lawnmower - #1491

Open
yannik131 wants to merge 35 commits into
masterfrom
547-pause-resume-for-lawnmower
Open

Pause/resume for lawnmower#1491
yannik131 wants to merge 35 commits into
masterfrom
547-pause-resume-for-lawnmower

Conversation

@yannik131

Copy link
Copy Markdown
Contributor

Closes #547

@yannik131

Copy link
Copy Markdown
Contributor Author

Architectural sketch of what I implemented:

  • introduce a PlayPause component for the two buttons
  • annotatorContainer defines
    • currentLawnmowerState, which is passed as a prop to annotationModesTab -> playPause
    • lawnmowerSaveState (load, save, discard), passed as a prop to annotationCanvas/lawnmower.vue, watcher for changes
  • data flow is then:
    1. user requests change in playPause.vue (emit lawnmowerStateChangeRequested)
    2. annotationModesTab.vue re-emits
    3. annotatorContainer.vue changes the state if possible and updates both props
    4. change is reflected in playPause via the currentLawnmowerState prop, annotationModesTab.vue updates the mode
    5. annotationCanvas/lawnmower.vue loads/saves/discards the state (current image, current section) from local storage tied to volume ID
  • then do the same for label review mode

@yannik131

yannik131 commented Jun 20, 2026

Copy link
Copy Markdown
Contributor Author

@mzur It's working now, but the architecture is not really consistent (mixin/options API for lawnmower, composable for volare). I managed to move the volare-logic from the AnnotatorContainer into the composable and would like to do the same for the lawnmower, but would first like to get some feedback from you regarding the overall structure. I'll mark it ready for review anyways.

Edit: Oh, and also I've run out of AI credits, so I can't use copilot for reviews right now. Since the switch to the new credits-based pricing model a single larger AI code review uses up about 25% of my $2.00 worth of free credits.

Edit 2: I also noticed a bug in the original behaviour that is still present: Focus annotation in volare, filter annotations that keeps the focussed annotation on the screen, remove filter -> wrong annotation is focussed. I will resume development once I get your feedback regarding the structure, though.

@yannik131
yannik131 marked this pull request as ready for review June 20, 2026 12:43
@yannik131
yannik131 requested a review from mzur June 20, 2026 12:44
@mzur
mzur removed their request for review June 25, 2026 12:12
@mzur

mzur commented Jun 25, 2026

Copy link
Copy Markdown
Member

I'll skip the review until we finished the discussion in the issue. Let me know if I should request an AI review for you here.

@yannik131

Copy link
Copy Markdown
Contributor Author

@mzur I don't think volare should be paused when the viewport is changed, that would likely break workflows where users zoom in while in volare. Otherwise this is as we discussed. Let me know what you think about the volare.js structure and I will turn lawnmower.vue into a similar composable.

@yannik131
yannik131 requested a review from mzur July 28, 2026 11:55
@mzur

mzur commented Jul 29, 2026

Copy link
Copy Markdown
Member

Do you need/want an AI review first?

@yannik131

Copy link
Copy Markdown
Contributor Author

@mzur Sure, but I'd also like feedback from you on the composable structure overall. I will use that feedback for the upcoming lawnmower composable.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds pause/resume behavior to the annotation “cycling” modes (Lawnmower and Volume Label Review/Volare), including persisting/restoring state so users can zoom/pan to inspect details without the modes jumping unpredictably.

Changes:

  • Replace simple on/off toggles with a Play/Pause UI for Volare and Lawnmower, including “paused at” timestamps.
  • Persist/restore Lawnmower (viewport/section) and Volare (focused annotation) state via localStorage across pausing and page reloads.
  • Add plumbing/events between annotationCanvas and annotatorContainer to auto-pause Lawnmower on user viewport changes while avoiding pauses during programmatic moves.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
resources/views/annotations/show/tabs/annotationModes.blade.php Switch Volare/Lawnmower controls to the new Play/Pause component and display pause timestamps.
resources/views/annotations/show.blade.php Wire new lawnmower save-state prop and new canvas events for pause/restore coordination.
resources/assets/sass/annotations/sidebar/_main.scss Add styling for the new “paused at” timestamp text.
resources/assets/js/utils.js Add required() helper for composable argument validation and export it.
resources/assets/js/core/components/powerToggle.vue Add separate iconOn/iconOff props and switch icon based on active state.
resources/assets/js/core/components/playPause.vue Introduce a new Play/Pause button pair component and PlayPauseState.
resources/assets/js/annotations/components/annotationModesTab.vue Add paused modes + timestamps, and emit new annotation-mode-changed signals.
resources/assets/js/annotations/components/annotationCanvas/volare.js New composable to manage Volare cycling + pause/resume persistence.
resources/assets/js/annotations/components/annotationCanvas/lawnmower.vue Add save/load/discard state for Lawnmower and emit events around programmatic viewport changes.
resources/assets/js/annotations/components/annotationCanvas.vue Emit movestart from the underlying map to support auto-pausing behavior.
resources/assets/js/annotations/annotatorContainer.vue Integrate new Volare composable, wire pause/resume flows, and add auto-pause-on-viewport-change logic.
Suppressed comments (2)

resources/views/annotations/show/tabs/annotationModes.blade.php:34

  • Same issue as above: in an in-DOM template, @transitionRequested will be lowercased and the handler may never run. Use a kebab-case listener so it compiles to onTransitionRequested.
                @transitionRequested="onLawnmowerPlayPauseTransitionRequested"

resources/assets/js/annotations/components/annotationModesTab.vue:99

  • pauseLawnmower() is called without a timestamp (e.g. from updateLawnmowerState and from viewport-change pausing), which makes lawnmowerPausedAtText blank. Default to Date.now() so pauses always record when they happened.
        pauseLawnmower(timestamp) {

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread resources/views/annotations/show/tabs/annotationModes.blade.php Outdated
Comment thread resources/assets/js/annotations/annotatorContainer.vue Outdated
Comment thread resources/assets/js/annotations/components/annotationModesTab.vue
Comment thread resources/assets/js/annotations/components/annotationModesTab.vue

@mzur mzur left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a full review, just my take on the composable approach with volare. I like a composable approach like this in general. The main benefit could be that logic can be shared across similar parts of BIIGLE (e.g. image and video annotation tools). All the current mixins that are used more than once are possible candidates for refactoring. Also the mixins of the annotationCanvas and videoScreen could maybe be (partially) merged and reused.

However in the case of volare it "just" moves code from one file to another. It makes the annotatorContainer slightly smaller but also introduces inconsistent architecture now (as the other annotation modes don't use the composable approach). And since the annotation modes are exclusive to the image annotation tool, I don't really see a benefit in making them all composable. It will be mostly refactoring for it's own sake which we should only do with our little resources if there is a real benefit in terms of code complexity.

So in summary I like the composable approach but volare and lawnmower mode may be the wrong features to pilot this. But please feel free to argue your point. We can also discuss this in a VC if you like.

Comment thread resources/assets/js/annotations/annotatorContainer.vue Outdated
Comment thread resources/assets/js/annotations/annotatorContainer.vue Outdated
@yannik131

yannik131 commented Aug 23, 2026

Copy link
Copy Markdown
Contributor Author

@mzur I like the composable approach because of the separation of concerns it offers. Decoupling things from the rest of the codebase makes code testable and easier to reason about. This helps other developers to understand the code, including myself in the future. I do agree that the time investment is probably not worth it here. But I will start considering separating code into composables more often from now on and might default to it for new functionality.
I'll leave lawnmower as it is for now. Ready for review.

@yannik131
yannik131 requested a review from mzur August 23, 2026 18:29
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.

Pause/Resume for Lawnmower and Volume Label Review Modes

3 participants