Skip to content

Pulse and persist vehicle disconnection indicators (border and widget)#2621

Open
rafaellehmkuhl wants to merge 3 commits intobluerobotics:masterfrom
rafaellehmkuhl:be-more-clear-about-disconnections
Open

Pulse and persist vehicle disconnection indicators (border and widget)#2621
rafaellehmkuhl wants to merge 3 commits intobluerobotics:masterfrom
rafaellehmkuhl:be-more-clear-about-disconnections

Conversation

@rafaellehmkuhl
Copy link
Copy Markdown
Member

@rafaellehmkuhl rafaellehmkuhl commented Apr 23, 2026

Also fixes the problem of the border pushing elements and squeezing the screen space.

Cap.2026-04-23.at.00.24.20.mp4

Closes #2616
Closes #1817

Previously the red border around the main view faded out a few seconds
after a vehicle disconnection, making it unclear whether the
disconnection was still ongoing. Now the border stays solid red and
pulses for as long as the vehicle remains offline, only being cleared
when the connection is reestablished.
The disconnected state previously rendered the indicator in a dim gray
that was easy to miss. It now turns red with a glowing drop shadow and
a pulsing opacity animation, making the disconnection state clearly
visible at a glance.
Moves the connection-status border from the main view container to a top-level
fixed overlay using inset box-shadow. This keeps the red ring continuous on
every edge, sits above all widgets, and ignores pointer events so it never
interferes with clicks or layout.
@github-actions
Copy link
Copy Markdown

Automated PR Review (Claude)

0. Summary

Verdict: MINOR SUGGESTIONS

Minor items to consider: 1.1, 4.1, 6.1, 9.1, 9.2.

This PR improves the vehicle disconnection visual feedback in two ways: (1) replaces the old static-border inline-style approach in App.vue with a fixed overlay <div> that shows a pulsing red box-shadow while disconnected and a brief green flash on reconnection, and (2) makes the BaseCommIndicator mini-widget pulse its icon red+glow while disconnected instead of dimming to gray. The old connectionStatusFeedback ref / resetConnectionStatusFeedback function are removed, and the new approach is cleaner — CSS handles the persistent animation, JS only manages the transient reconnection flash.


1. Correctness & Implementation Bugs

1.1 (minor)reconnectedFeedbackTimeout is not cleared on component unmount (src/App.vue). If App.vue is ever unmounted while the timeout is pending (unlikely for the root component, but defensive coding), the callback would fire on a stale ref. Consider clearing it in the existing onBeforeUnmount block for consistency with the project's pattern of cleaning up timers/listeners.

No other correctness issues found. The clearTimeout guard before re-setting the timeout is correct and prevents the old race condition that existed in resetConnectionStatusFeedback. The CSS class toggling logic is straightforward and correct.


2. AGENTS.md Adherence

No findings. No new dependencies are added, no new local-storage keys are introduced, no JSDoc-requiring public functions are added (the eslint-disable jsdoc/require-jsdoc block already covers this region), and yarn is the package manager. The new comment on line ~238 (after diff) explains "why" (the role split between CSS and JS), which is appropriate per the comment policy.


3. Security

3.1 (no issue) No obfuscated or intentionally unreadable code.
3.2 (no issue) No base64/hex blobs, binary-like strings, or large encoded constants.
3.3 (no issue) No hidden Unicode, zero-width characters, RTL overrides, or homoglyph attacks.
3.4 (no issue) No new network calls, fetch/XHR/websocket to unknown hosts, or exfiltration patterns.
3.5 (no issue) No changes to build scripts, CI workflows, Dockerfiles, or Electron main-process code.
3.6 (no issue) No new secrets, env vars, tokens, eval, Function(), or v-html usage. No CORS/CSP changes.
3.7 (no issue) No new dependencies added.
3.8 (no issue) No other suspicious patterns detected. The changes are purely CSS animations and a minor JS refactor.


4. Performance

4.1 (nit) — The vehicle-disconnected-pulse CSS animation runs infinite with box-shadow transitions (src/App.vue:~380). Animated box-shadow is not GPU-composited and can cause paint operations each frame. On most modern devices this is negligible, but for low-power embedded panels (common in marine robotics), consider using opacity on a pseudo-element or outline instead if jank is observed. Not blocking, since this only runs during the disconnected state.

No memory leaks, unclosed subscriptions, or redundant network requests.


5. UI / UX

No findings. The overlay correctly uses pointer-events: none so it doesn't block interaction, aria-hidden="true" so screen readers ignore the decorative element, position: fixed with inset: 0 for full-viewport coverage, and z-index: 9999 to sit above all content. The border-radius: 0 0 12px 12px comment explains the OS window rounding rationale. The pulsing red animation and green reconnection flash provide clear, persistent feedback — a significant UX improvement over the old static border that faded after 4 seconds while still disconnected.


6. Code Quality & Style

6.1 (minor) — The /* eslint-disable jsdoc/require-jsdoc */ comment at src/App.vue:233 was originally placed to cover the old connectionStatusFeedback ref and resetConnectionStatusFeedback function. After this PR, it now covers showReconnectedFeedback (a simple ref) and reconnectedFeedbackTimeout (a let variable) — neither of which requires JSDoc. However, the disable comment still blankets everything below it in the <script setup> block (including routerSection, hideMouse, resetHideMouseTimeout, etc.). This was a pre-existing issue, not introduced by this PR, but worth noting since the diff touches this area.

No dead code, no any types, no naming issues. The CSS class names (vehicle-connection-overlay, is-disconnected, is-reconnected, disconnected-pulse) are clear and descriptive.


7. Tests

No findings. This is a visual/CSS-focused change with no new logic branches that would meaningfully benefit from unit tests. The core behavior (watching isVehicleOnline) is unchanged.


8. Documentation

No findings. No feature-parity differences between Lite and Standalone are introduced. No user-facing documentation changes are needed.


9. Nitpicks / Optional

9.1 (nit) — In BaseCommIndicator.vue, the filter: drop-shadow(...) on .disconnected-pulse is applied alongside the animation. Since the drop-shadow is static (not animated), it will appear/disappear abruptly when the vehicle connects/disconnects rather than transitioning. If a smooth entrance is desired, consider adding a transition: filter 0.3s ease or incorporating the drop-shadow into the keyframes.

9.2 (nit) — The 1.6s animation duration is used in both App.vue (vehicle-disconnected-pulse) and BaseCommIndicator.vue (comm-indicator-pulse). If these are meant to stay synchronized, consider extracting the duration to a CSS custom property (e.g., --disconnect-pulse-duration: 1.6s) for easier future maintenance. This is optional given the small scope.


Generated by Claude. This is advisory; a human reviewer must still approve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant