Address Gadfly review on feet-and-inches entry

formatDimensionInput signed values that had rounded to nothing, so a hair below
zero rendered "-0′ 0″" — wrong, and not something parseDimension round-trips.
The sign is now applied only once the rounded feet/inches are actually nonzero.

That fix wanted the decomposition in one place, which formatCm was duplicating
anyway, so both now share feetAndInches(cm, decimals) — magnitude only, sign
applied by the caller once it knows the result isn't zero.

Doing so incidentally fixed a latent bug in formatCm: it took Math.floor of a
negative total, so -76.2cm rendered "-3′ 6″", which reads as -(3ft+6in) = -42in
rather than the -30in it actually is. Nothing renders a negative dimension today
(formatCm is used for garden sizes), but it was wrong and now isn't.

The dimensionInputMode ternary and its comment were duplicated verbatim between
Inspector and GardenFormModal; it is now a helper in units.ts, where the reason
for it belongs anyway.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01H3zbym8Doka2d7D48maSgZ

web/src/components/gardens/GardenFormModal.tsx

@@ -10,6 +10,7 @@ import { conflictGarden, useCreateGarden, useUpdateGarden, type Garden } from '@
 import {
   dimensionUnitLabel,
   formatCm,
+  dimensionInputMode,
   formatDimensionInput,
   isValidDimensionCm,
   MIN_GARDEN_GRID_CM,
@@ -155,9 +156,7 @@ export function GardenFormModal({ garden, onClose }: { garden?: Garden; onClose:
   }
 
   const unitLabel = dimensionUnitLabel(unit)
-  // Imperial entry needs ' and ", which no numeric keypad offers; metric is pure
-  // decimals and keeps the keypad. A bare number still means feet either way.
-  const dimensionInputMode = unit === 'imperial' ? 'text' : 'decimal'
+  const inputMode = dimensionInputMode(unit)
 
   return (
     <Modal title={isEdit ? 'Edit garden' : 'New garden'} onClose={onClose} busy={pending}>
@@ -179,7 +178,7 @@ export function GardenFormModal({ garden, onClose }: { garden?: Garden; onClose:
             label={`Width (${unitLabel})`}
             name="width"
             type="text"
-            inputMode={dimensionInputMode}
+            inputMode={inputMode}
             required
             value={width}
             onChange={(e) => setWidth(e.target.value)}
@@ -188,7 +187,7 @@ export function GardenFormModal({ garden, onClose }: { garden?: Garden; onClose:
             label={`Height (${unitLabel})`}
             name="height"
             type="text"
-            inputMode={dimensionInputMode}
+            inputMode={inputMode}
             required
             value={height}
             onChange={(e) => setHeight(e.target.value)}
@@ -202,7 +201,7 @@ export function GardenFormModal({ garden, onClose }: { garden?: Garden; onClose:
                 label={`Garden grid (${unitLabel})`}
                 name="gridSize"
                 type="text"
-                inputMode={dimensionInputMode}
+                inputMode={inputMode}
                 value={gridSize}
                 onChange={(e) => setGridSize(e.target.value)}
               />

web/src/editor/Inspector.tsx

@@ -6,6 +6,7 @@ import { useDeleteObject, useUpdateObject } from '@/lib/objects'
 import {
   cmFromSpacing,
   dimensionUnitLabel,
+  dimensionInputMode,
   formatDimensionInput,
   MIN_DIMENSION_CM,
   parseDimension,
@@ -107,10 +108,7 @@ export function Inspector({
   }
 
   const u = dimensionUnitLabel(unit)
-  // Imperial entry needs ' and ", which no numeric keypad offers, so those fields
-  // ask for the full keyboard. Metric is pure decimals and keeps the keypad.
-  // (A bare number still means feet, so the keypad path stays usable either way.)
-  const dimensionInputMode = unit === 'imperial' ? 'text' : 'decimal'
+  const inputMode = dimensionInputMode(unit)
 
   return (
     <div ref={rootRef} className="flex flex-col gap-3">
@@ -152,7 +150,7 @@ export function Inspector({
             label={`Width (${u})`}
             name="width"
             type="text"
-            inputMode={dimensionInputMode}
+            inputMode={inputMode}
             value={width}
             onChange={(e) => setWidth(e.target.value)}
             onBlur={() => commitDim(width, object.widthCm, (cm) => patch({ widthCm: cm }), true)}
@@ -161,7 +159,7 @@ export function Inspector({
             label={`Height (${u})`}
             name="height"
             type="text"
-            inputMode={dimensionInputMode}
+            inputMode={inputMode}
             value={height}
             onChange={(e) => setHeight(e.target.value)}
             onBlur={() => commitDim(height, object.heightCm, (cm) => patch({ heightCm: cm }), true)}
@@ -170,7 +168,7 @@ export function Inspector({
             label={`X (${u})`}
             name="x"
             type="text"
-            inputMode={dimensionInputMode}
+            inputMode={inputMode}
             value={x}
             onChange={(e) => setX(e.target.value)}
             onBlur={() => commitDim(x, object.xCm, (cm) => patch({ xCm: cm }))}
@@ -179,7 +177,7 @@ export function Inspector({
             label={`Y (${u})`}
             name="y"
             type="text"
-            inputMode={dimensionInputMode}
+            inputMode={inputMode}
             value={y}
             onChange={(e) => setY(e.target.value)}
             onBlur={() => commitDim(y, object.yCm, (cm) => patch({ yCm: cm }))}

web/src/lib/units.test.ts

@@ -3,6 +3,7 @@ import {
   cmFromFtIn,
   cmFromMeters,
   cmFromSpacing,
+  dimensionInputMode,
   displayFromCm,
   formatCm,
   formatDimensionInput,
@@ -185,3 +186,26 @@ describe('formatDimensionInput', () => {
     expect(formatDimensionInput(1000, 'metric')).toBe('10')
   })
 })
+
+describe('negative values that round to zero', () => {
+  // A hair below zero is 0′ 0″. "-0′ 0″" would be both wrong and, since
+  // parseDimension gives -0, not something that round-trips.
+  it('does not sign a value that rounded to nothing', () => {
+    expect(formatDimensionInput(-0.1, 'imperial')).toBe('0′ 0″')
+    expect(formatDimensionInput(-0, 'imperial')).toBe('0′ 0″')
+    expect(formatCm(-0.1, 'imperial')).toBe('0′ 0″')
+  })
+
+  it('still signs a value that rounded to something', () => {
+    expect(formatDimensionInput(-1, 'imperial')).toBe('-0′ 0.4″')
+    expect(formatDimensionInput(-76.2, 'imperial')).toBe('-2′ 6″')
+    expect(formatCm(-76.2, 'imperial')).toBe('-2′ 6″')
+  })
+})
+
+describe('dimensionInputMode', () => {
+  it("gives imperial the full keyboard, since ' and \" are on no keypad", () => {
+    expect(dimensionInputMode('imperial')).toBe('text')
+    expect(dimensionInputMode('metric')).toBe('decimal')
+  })
+})

web/src/lib/units.ts

@@ -122,28 +122,45 @@ export function parseDimension(input: string, unit: UnitPref): number | null {
  */
 export function formatDimensionInput(cm: number, unit: UnitPref): string {
   if (unit !== 'imperial') return String(displayFromCm(cm, unit))
-  const sign = cm < 0 ? '-' : ''
+  const { feet, inches } = feetAndInches(cm, 1)
+  // Only sign a value that actually rounded to something: a hair below zero is
+  // 0′ 0″, and "-0′ 0″" would be both wrong and un-round-trippable.
+  const sign = cm < 0 && (feet || inches) ? '-' : ''
+  return `${sign}${feet}′ ${inches}″`
+}
+
+/**
+ * The inputMode a dimension field should ask for. Imperial entry needs ' and ",
+ * which no numeric keypad offers, so those fields get the full keyboard; metric
+ * is pure decimals and keeps the keypad. A bare number still means feet, so the
+ * keypad path stays usable either way.
+ */
+export function dimensionInputMode(unit: UnitPref): 'text' | 'decimal' {
+  return unit === 'imperial' ? 'text' : 'decimal'
+}
+
+/**
+ * Decompose centimeters into feet and inches, rounding inches to `decimals` and
+ * carrying 12″ up to the next foot. Magnitude only — the sign is the caller's to
+ * apply, and only once it knows the rounded result isn't zero.
+ */
+function feetAndInches(cm: number, decimals: number): { feet: number; inches: number } {
+  const scale = 10 ** decimals
   const totalInches = Math.abs(cm) / CM_PER_INCH
   let feet = Math.floor(totalInches / INCHES_PER_FOOT)
-  let inches = Math.round((totalInches - feet * INCHES_PER_FOOT) * 10) / 10
+  let inches = Math.round((totalInches - feet * INCHES_PER_FOOT) * scale) / scale
   if (inches >= INCHES_PER_FOOT) {
     feet += 1
     inches = 0
   }
-  return `${sign}${feet}′ ${inches}″`
+  return { feet, inches }
 }
 
 /** Centimeters → a human string in the given unit (e.g. "1.22 m" or "4′ 0″"). */
 export function formatCm(cm: number, unit: UnitPref): string {
   if (unit === 'imperial') {
-    const totalInches = cm / CM_PER_INCH
-    let feet = Math.floor(totalInches / INCHES_PER_FOOT)
-    let inches = Math.round(totalInches - feet * INCHES_PER_FOOT)
-    if (inches === INCHES_PER_FOOT) {
-      feet += 1
-      inches = 0
-    }
-    return `${feet}′ ${inches}″`
+    const { feet, inches } = feetAndInches(cm, 0)
+    return `${cm < 0 && (feet || inches) ? '-' : ''}${feet}′ ${inches}″`
   }
   const meters = Math.round((cm / CM_PER_METER) * 100) / 100
   return `${meters} m`