Label snap step units and keep typed Rotate steps

[Devin T. Currie (DTCurrie)]

Labels the snap step fields with their units and keeps a typed Rotate step instead of reverting it. Before this, Move showed a bare number with no unit, and a step typed into Rotate snapped back to its old value.

Hooks

• transformSnaps(settings) is new. It turns the snap settings into the steps a Three.js gizmo takes: the Move step passes through in metres, the Rotate step converts from degrees to radians, and an axis with snapping off or a step of 0 gets null.
• Settings.snapTranslate documents that it is in metres.

Plugins

• TransformDashboard labels the snap fields Move (m) and Rotate (°), and drops the Rotate format callback.
• MoveGizmo reads its translation and rotation steps from transformSnaps.

Components

• SelectedTransformControls reads all three steps from transformSnaps.

Why?

Why is Move in metres when every pose field is in mm?

Millimetres are the unit Viam's APIs speak, and pose fields show them because they round-trip through those APIs. The snap step never reaches a Viam API. Keeping it in metres leaves every saved step unchanged, and the label says which unit it is.

Why move the units into the labels?

Tweakpane parses typed text as a bare number. The Rotate format callback appended ° to the value, so an edit to that text no longer parsed and reverted. A label carries the unit without touching the value.

Why not stack this on #1108?

The two PRs edit separate hunks of SelectedTransformControls.svelte, but both change its three import. #1108 drops Group and this PR drops MathUtils. Whichever merges second resolves that one line to import { Matrix4 } from 'three'.

Testing

Added transformSnaps.spec.ts for the pass-through and degree conversions and the off and zero cases. Scaling the Move step by 0.001 inside transformSnaps fails its pass-through case.

Ran the binaries behind pnpm check and pnpm test directly, since this was a worktree: svelte-check (0 errors) and the full vitest --run (134 files, 1974 tests). Ran eslint and prettier --check on the changed files. No Go changed, so go vet and golangci-lint did not run.

[changeset-bot]

🦋 Changeset detected

Latest commit: 597df51

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@viamrobotics/visualization	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[github-actions]

PR Preview Action v1.8.1
🚀 View preview at https://viamrobotics.github.io/visualization/pr-preview/pr-1109/
Built to branch gh-pages at 2026-10-07 16:03 UTC. Preview will be ready when the GitHub Pages deployment is complete.

[Devin T. Currie (DTCurrie)]

Claude (@claude) review

[claude]

Claude finished Devin T. Currie (@DTCurrie)'s task in 2m 16s —— View job

---

Review

Clean, well-scoped PR. The extraction into transformSnaps eliminates the duplicated inline logic from two components, makes the degree→radian conversion testable in isolation, and the label-carries-unit approach correctly sidesteps Tweakpane's parse-on-edit behavior. Two nits in the inline comments, neither a blocker.

---

transformSnaps.ts — the toSceneUnits callback name implies conversion, but two of the three axes pass identity functions. See inline comment on line 23 for a couple of simpler alternatives. Current code is correct.

MoveGizmo.svelte — transformSnaps() returns all three axes but MoveGizmo only binds two (no scale handle). See inline on line 86. Pure function, so no correctness issue — just a tiny mismatch between what's computed and what's consumed.

Tests — good coverage. The five cases pin: pass-through in metres, radian conversion (literal Math.PI / 2, not the implementation's own expression), scale factor, snapping: false gives all-null, and zero step frees the axis. The 'leaves an axis free when its step is 0' case tests only snapTranslate — one is enough since the step helper is shared.

Comments — both the HTML comment in TransformDashboard.svelte and the TSDoc on snapTranslate explain divergences from convention (unit-in-value format, mm everywhere else) that a future reader would genuinely ask about. Both are justified by the rules.

SelectedTransformControls.svelte import cleanup — dropping MathUtils is correct; the logic moved into transformSnaps. The note about the eventual import { Matrix4 } from 'three' merge conflict with #1108 is accurate and well-flagged in the PR description.

---

Summary: Two nits (inline), both optional. Ready to merge as-is.

[claude]

Nit: The identity closures (metres) => metres and (factor) => factor are readable as documentation of intent, but toSceneUnits implies conversion is happening — which isn't true for these two. You could drop the callback for the pass-through axes and name the helper snap instead:

Suggested change

	translation: step(settings.snapTranslate, (metres) => metres),
	translation: settings.snapping && settings.snapTranslate > 0 ? settings.snapTranslate : null,

Or, keeping the helper but removing the identity wrapping:

const snap = (value: number) => settings.snapping && value > 0 ? value : null

return {
    translation: snap(settings.snapTranslate),
    rotation: snap(settings.snapRotate) !== null ? MathUtils.degToRad(settings.snapRotate) : null,
    scale: snap(settings.snapScale),
}

Current code is correct and readable — just noting the slight tension between the callback name and what it does for translation/scale.

[claude]

Nit: snaps.scale is computed here but MoveGizmo only binds translationSnap and rotationSnap — there's no scale handle on this gizmo. Negligible since transformSnaps is pure and cheap, but if you wanted to keep the abstraction tight you could destructure only what's used:

Suggested change

	const snaps = $derived(transformSnaps(settings.current))
	const { translation, rotation } = $derived(transformSnaps(settings.current))

Then bind translationSnap={translation} and rotationSnap={rotation} below.

[Devin T. Currie (DTCurrie)]

transformSnaps.ts — the toSceneUnits callback name implies conversion, but two of the three axes pass identity functions. See inline comment on line 23 for a couple of simpler alternatives. Current code is correct.

MoveGizmo.svelte — transformSnaps() returns all three axes but MoveGizmo only binds two (no scale handle). See inline on line 86. Pure function, so no correctness issue — just a tiny mismatch between what's computed and what's consumed.

Claude (@claude) resolve these

[claude]

Claude finished Devin T. Currie (@DTCurrie)'s task in 3s —— View job

---

Resolved

• Read transformSnaps.ts and MoveGizmo.svelte
• Fix transformSnaps.ts: rename helper and simplify
• Fix MoveGizmo.svelte: destructure only used axes
• Verify: svelte-check 0 errors/warnings, eslint clean
• Commit and push

transformSnaps.ts — renamed step/toSceneUnits to snap/convert. Added an identity default so only rotation passes a converter. MathUtils.degToRad is passed directly instead of wrapped in a lambda.

const snap = (value: number, convert: (v: number) => number = (v) => v) =>
    settings.snapping && value > 0 ? convert(value) : null

return {
    translation: snap(settings.snapTranslate),
    rotation: snap(settings.snapRotate, MathUtils.degToRad),
    scale: snap(settings.snapScale),
}

MoveGizmo.svelte — destructures { translation, rotation } directly, so scale is never computed at the call site.