Fix long names in the joystick mapping being cropped

[rafaellehmkuhl]

Finally found an elegant solution to that.

Long button function names were being clipped at the SVG viewport edges because the labels lived inside the SVG as elements. Hide the original SVG labels and render an HTML overlay div per button, anchored at the same screen position as the original text. The overlays live outside the SVG so they can extend beyond its bounds and wrap onto multiple lines when the function name is too long.

Before:

After:

Fix #456

[github-actions]

Automated PR Review (Claude)

0. Summary

Verdict: MINOR SUGGESTIONS

Minor items to consider: 1.1, 1.2, 4.1, 6.1

This PR fixes a longstanding issue where joystick button function names were being cropped when they exceeded the SVG viewport boundaries. The approach hides the original SVG <text> elements (keeping them in the layout for position reference) and renders HTML overlay <div>s anchored at the same screen position. These overlays live outside the SVG and can wrap text naturally. A ResizeObserver keeps overlay positions in sync when the container resizes. A small change to ConfigurationJoystickView.vue adds mx-auto to horizontally center the joystick container now that the wrapper div is introduced.

1. Correctness & Implementation Bugs

1.1 (minor) — updateLabelsState mutates labelOverlays.value entries directly (overlay.text = functionName), but those entries are plain objects inside a ref<LabelOverlay[]>. Because the array ref tracks the array reference, in-place property mutation on items may not trigger Vue's reactivity for the v-for template rendering. Since labelOverlays is a ref() (not reactive() or shallowRef()), Vue 3 does deep-unwrap refs, so this will likely work due to Vue 3's deep proxy — but it's a subtle dependency. The same applies to toggleButton mutating overlay.color. Worth confirming this is reliable, or consider reassigning the array (spread) after mutation to guarantee reactivity.

1.2 (minor) — computeOverlayPositions is called inside the setInterval callback on every tick until svg is found. The first time svg becomes non-null, computeOverlayPositions runs, but the interval continues ticking (it only returns early on subsequent ticks via if (svg) return). This is correct and matches existing behavior, but note that computeOverlayPositions is only called once from the interval (since subsequent ticks hit the return). The ResizeObserver handles future recalculations, which is good. However, if the SVG <object> loads but internal text elements are not yet fully laid out when getSVGDocument() first succeeds, getBoundingClientRect() could return zero-rects. This is unlikely but a potential edge case — the existing 100ms polling mitigates it, and the ResizeObserver would correct it on next resize.

2. AGENTS.md Adherence

No findings.

The PR does not add new dependencies, does not introduce new local-storage keys, does not add comments explaining "what" (the comments explain "why" — e.g., explaining the SVG-to-HTML coordinate conversion strategy), and the new JSDoc entries have proper @param types and @returns. No npm/npx usage. Good compliance overall.

3. Security

3.1 (nit) — No obfuscated or intentionally unreadable code.

3.2 — No suspicious base64/hex/encoded blobs.

3.3 — No hidden Unicode, zero-width characters, or homoglyph attacks.

3.4 — No unexpected network calls or exfiltration patterns.

3.5 — No changes to build scripts, CI workflows, Dockerfiles, or Electron main-process code.

3.6 — No eval, Function(), v-html, or secret handling changes. The template uses {{ label.text }} (safe text interpolation, not v-html).

3.7 — No new dependencies added.

3.8 — No other suspicious patterns detected.

No security findings.

4. Performance

4.1 (minor) — computeOverlayPositions uses document.querySelector with a class selector on every call. Since this runs on every ResizeObserver callback (which can fire frequently during window resizing), and getBoundingClientRect() is called for each button label, this could cause layout thrashing if many resizes fire in rapid succession. Consider debouncing the ResizeObserver callback (e.g., via requestAnimationFrame) to coalesce rapid resize events:

resizeObserver = new ResizeObserver(() => {
  requestAnimationFrame(() => {
    computeOverlayPositions()
    updateLabelsState()
  })
})

4.2 (nit) — labelOverlays.value.find((l) => l.id === labelId) is called in a loop (inside updateLabelsState and toggleButton). With ≤18 buttons this is negligible, but a Map keyed by labelId would be O(1). Not a real concern at this scale.

5. UI / UX

No findings.

The approach is sound: rendering HTML overlays on top of hidden SVG text elements preserves visual position while allowing natural text wrapping. The maxWidth constraint (labelFontSize * 15) and responsive font sizing (Math.max(9, ...)) are reasonable. The -55% translateY approximation for baseline alignment looks like it was tuned visually — acceptable for this use case.

6. Code Quality & Style

6.1 (nit) — The LabelOverlay interface is defined inline in the <script setup> block. This is fine for a component-local type, but it could be extracted if it grows. Currently acceptable.

6.2 (nit) — The @ts-ignore comment at line ~125 of the diff (buttonPath[button].replace('path', 'text')) is carried over from existing code. It's pre-existing and outside the scope of this PR to fix, but worth noting.

7. Tests

No findings.

This is a UI/visual fix to an SVG overlay component. There are no existing unit tests for JoystickPS.vue in the base branch, and the nature of the fix (SVG coordinate mapping to HTML overlays) is inherently difficult to unit test without a real browser DOM. The lack of new tests is understandable here.

8. Documentation

No findings.

This is a bug fix that doesn't change any public API or feature-parity between Lite and Standalone. No README or docs update needed.

9. Nitpicks / Optional

9.1 (nit) — The ResizeObserver cleanup is properly handled in onBeforeUnmount with resizeObserver?.disconnect(). Good.

9.2 (nit) — In ConfigurationJoystickView.vue, the addition of mx-auto to center the container is a sensible complement to the wrapper <div> change — the <object> previously self-centered, but the new <div> wrapper needs explicit centering. Clean fix.

9.3 (nit) — The fallback viewBoxWidth of 1250 in const viewBoxWidth = svgRoot?.viewBox?.baseVal?.width || 1250 is a magic number. A named constant would improve readability:

const DEFAULT_SVG_VIEWBOX_WIDTH = 1250

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

[ES-Alexander]

Nice! It makes me slightly uncomfortable to be kind of and kind of not using the SVG (😅), but it does the job and definitely improves things!

A couple of notes/questions, but nothing significant.

---

If I make the window narrow it seems the left side wrapping doesn't work properly:

but given the state we're coming from I don't consider that a blocker for this PR.

[ES-Alexander]

Is this literally talking about the letter "y"? If so it should be in quotes. Otherwise I think this comment is incomplete.

[rafaellehmkuhl]

Was incomplete. Fixed.

[ES-Alexander]

Curious where the 1250 default comes from. Is there a variable somewhere, or is this a guess based on common screen sizes or something?

[rafaellehmkuhl]

It's the default size of our SVGs, but it's a fallback. Explained better in the code now.

[rafaellehmkuhl]

Nice! It makes me slightly uncomfortable to be kind of and kind of not using the SVG (😅), but it does the job and definitely improves things!

A couple of notes/questions, but nothing significant.

If I make the window narrow it seems the left side wrapping doesn't work properly: but given the state we're coming from I don't consider that a blocker for this PR.

Was able to improve it! No cropping even on small screens now!