diff --git a/web/src/components/gardens/GardenFormModal.tsx b/web/src/components/gardens/GardenFormModal.tsx index 70375be..496d292 100644 --- a/web/src/components/gardens/GardenFormModal.tsx +++ b/web/src/components/gardens/GardenFormModal.tsx @@ -8,25 +8,34 @@ import { TextField } from '@/components/ui/TextField' import { errorMessage } from '@/lib/api' import { conflictGarden, useCreateGarden, useUpdateGarden, type Garden } from '@/lib/gardens' import { - cmFromDisplay, dimensionUnitLabel, - displayFromCm, formatCm, + dimensionInputMode, + formatDimensionInput, isValidDimensionCm, MIN_GARDEN_GRID_CM, + parseDimension, type UnitPref, } from '@/lib/units' const DEFAULT_METERS = 10 // matches the server's 10 m default const DEFAULT_GRID_CM = 100 // matches the server's 1 m grid default +// What to say when a field can't be read. Naming the accepted forms beats +// "invalid input", which leaves the person guessing which field and which part. +function entryHint(unit: UnitPref): string { + return unit === 'imperial' + ? `Enter sizes as feet and inches — 8' 6", 8', or 8.5 for feet.` + : 'Enter sizes in meters, e.g. 2.5.' +} + const unitOptions = [ { value: 'metric', label: 'Metric (m)' }, { value: 'imperial', label: 'Imperial (ft)' }, ] function dimString(cm: number | undefined, unit: UnitPref): string { - return cm === undefined ? String(DEFAULT_METERS) : String(displayFromCm(cm, unit)) + return cm === undefined ? String(DEFAULT_METERS) : formatDimensionInput(cm, unit) } /** @@ -46,7 +55,7 @@ export function GardenFormModal({ garden, onClose }: { garden?: Garden; onClose: const [width, setWidth] = useState(() => dimString(garden?.widthCm, garden?.unitPref ?? 'metric')) const [height, setHeight] = useState(() => dimString(garden?.heightCm, garden?.unitPref ?? 'metric')) const [gridSize, setGridSize] = useState(() => - String(displayFromCm(garden?.gridSizeCm ?? DEFAULT_GRID_CM, garden?.unitPref ?? 'metric')), + formatDimensionInput(garden?.gridSizeCm ?? DEFAULT_GRID_CM, garden?.unitPref ?? 'metric'), ) const [snapToGrid, setSnapToGrid] = useState(garden?.snapToGrid ?? false) const [notes, setNotes] = useState(garden?.notes ?? '') @@ -55,9 +64,11 @@ export function GardenFormModal({ garden, onClose }: { garden?: Garden; onClose: const [formError, setFormError] = useState(null) function changeUnit(next: UnitPref) { + // Re-render each field in the new unit, preserving the physical size. An + // unparseable field is left as typed rather than blanked. const convert = (s: string) => { - const v = parseFloat(s) - return Number.isFinite(v) ? String(displayFromCm(cmFromDisplay(v, unit), next)) : s + const cm = parseDimension(s, unit) + return cm === null ? s : formatDimensionInput(cm, next) } setWidth(convert(width)) setHeight(convert(height)) @@ -70,11 +81,11 @@ export function GardenFormModal({ garden, onClose }: { garden?: Garden; onClose: // Converted once per render and used by both the submit handler and the // too-fine hint, so the two can never disagree about what was entered. - const gridSizeCm = cmFromDisplay(parseFloat(gridSize), unit) + const gridSizeCm = parseDimension(gridSize, unit) // Soft floor: hint, don't refuse. A garden-scale grid this fine is usually // someone reaching for plant spacing, which lives on the bed instead — but it // is a legitimate choice for a very small garden, so the save still goes through. - const gridTooFine = Number.isFinite(gridSizeCm) && gridSizeCm > 0 && gridSizeCm < MIN_GARDEN_GRID_CM + const gridTooFine = gridSizeCm !== null && gridSizeCm > 0 && gridSizeCm < MIN_GARDEN_GRID_CM async function onSubmit(e: FormEvent) { e.preventDefault() @@ -88,12 +99,20 @@ export function GardenFormModal({ garden, onClose }: { garden?: Garden; onClose: // Validate the converted centimeter values against the same bounds the // server enforces, so sub-cm or over-100m sizes fail here with a clear // message instead of a generic server error. - const widthCm = cmFromDisplay(parseFloat(width), unit) - const heightCm = cmFromDisplay(parseFloat(height), unit) + const widthCm = parseDimension(width, unit) + const heightCm = parseDimension(height, unit) + if (widthCm === null || heightCm === null) { + setFormError(entryHint(unit)) + return + } if (!isValidDimensionCm(widthCm) || !isValidDimensionCm(heightCm)) { setFormError('Width and height must be between 1 cm and 100 m.') return } + if (gridSizeCm === null) { + setFormError(entryHint(unit)) + return + } if (!isValidDimensionCm(gridSizeCm)) { setFormError('Grid size must be between 1 cm and 100 m.') return @@ -126,7 +145,7 @@ export function GardenFormModal({ garden, onClose }: { garden?: Garden; onClose: setUnit(current.unitPref) setWidth(dimString(current.widthCm, current.unitPref)) setHeight(dimString(current.heightCm, current.unitPref)) - setGridSize(String(displayFromCm(current.gridSizeCm, current.unitPref))) + setGridSize(formatDimensionInput(current.gridSizeCm, current.unitPref)) setSnapToGrid(current.snapToGrid) setNotes(current.notes) setConflict('This garden changed elsewhere. The latest values are shown — review and save again.') @@ -137,6 +156,7 @@ export function GardenFormModal({ garden, onClose }: { garden?: Garden; onClose: } const unitLabel = dimensionUnitLabel(unit) + const inputMode = dimensionInputMode(unit) return ( @@ -157,10 +177,8 @@ export function GardenFormModal({ garden, onClose }: { garden?: Garden; onClose: setWidth(e.target.value)} @@ -168,10 +186,8 @@ export function GardenFormModal({ garden, onClose }: { garden?: Garden; onClose: setHeight(e.target.value)} @@ -184,10 +200,8 @@ export function GardenFormModal({ garden, onClose }: { garden?: Garden; onClose: setGridSize(e.target.value)} /> diff --git a/web/src/editor/Inspector.tsx b/web/src/editor/Inspector.tsx index ed3f293..cd2a65c 100644 --- a/web/src/editor/Inspector.tsx +++ b/web/src/editor/Inspector.tsx @@ -4,11 +4,12 @@ import { TextField } from '@/components/ui/TextField' import { TextArea } from '@/components/ui/TextArea' import { useDeleteObject, useUpdateObject } from '@/lib/objects' import { - cmFromDisplay, cmFromSpacing, dimensionUnitLabel, - displayFromCm, + dimensionInputMode, + formatDimensionInput, MIN_DIMENSION_CM, + parseDimension, spacingFromCm, spacingUnitLabel, type UnitPref, @@ -45,10 +46,10 @@ export function Inspector({ // Local field state (initialized once; committed on blur/change). const [name, setName] = useState(object.name) const [notes, setNotes] = useState(object.notes) - const [width, setWidth] = useState(String(displayFromCm(object.widthCm, unit))) - const [height, setHeight] = useState(String(displayFromCm(object.heightCm, unit))) - const [x, setX] = useState(String(displayFromCm(object.xCm, unit))) - const [y, setY] = useState(String(displayFromCm(object.yCm, unit))) + const [width, setWidth] = useState(formatDimensionInput(object.widthCm, unit)) + const [height, setHeight] = useState(formatDimensionInput(object.heightCm, unit)) + const [x, setX] = useState(formatDimensionInput(object.xCm, unit)) + const [y, setY] = useState(formatDimensionInput(object.yCm, unit)) const [rotation, setRotation] = useState(String(Math.round(object.rotationDeg))) const [color, setColor] = useState(object.color ?? DEFAULT_COLOR) const [gridSize, setGridSize] = useState(String(spacingFromCm(object.gridSizeCm, unit))) @@ -62,10 +63,10 @@ export function Inspector({ if (rootRef.current?.contains(document.activeElement)) return setName(object.name) setNotes(object.notes) - setWidth(String(displayFromCm(object.widthCm, unit))) - setHeight(String(displayFromCm(object.heightCm, unit))) - setX(String(displayFromCm(object.xCm, unit))) - setY(String(displayFromCm(object.yCm, unit))) + setWidth(formatDimensionInput(object.widthCm, unit)) + setHeight(formatDimensionInput(object.heightCm, unit)) + setX(formatDimensionInput(object.xCm, unit)) + setY(formatDimensionInput(object.yCm, unit)) setRotation(String(Math.round(object.rotationDeg))) setColor(object.color ?? DEFAULT_COLOR) setGridSize(String(spacingFromCm(object.gridSizeCm, unit))) @@ -77,14 +78,20 @@ export function Inspector({ } // Commit a dimension/position field. `positive` gates width/height (must be - // ≥ the server minimum) but not x/y, which may be zero or negative. Compare at - // display precision first so a blur without an edit doesn't fire a spurious - // PATCH from cm↔ft rounding drift (e.g. 240cm shows 7.9ft → 241cm). + // ≥ the server minimum) but not x/y, which may be zero or negative. + // + // The no-op guard compares the field's TEXT against the string this field + // renders for the current value. With compound entry there is no single + // display number left to compare, and a numeric tolerance would be worse: + // a bed dragged to some arbitrary cm renders as, say, 2′ 7.9″, and merely + // tabbing through the field would snap it to a whole inch. Text equality means + // "you didn't edit this", which is what the guard is actually for. Typing the + // same value a different way (2' 7" vs 2′ 7″) falls through to the cm compare + // below and is still a no-op. const commitDim = (raw: string, current: number, apply: (cm: number) => void, positive = false) => { - const v = parseFloat(raw) - if (!Number.isFinite(v)) return - if (v === displayFromCm(current, unit)) return - const cm = cmFromDisplay(v, unit) + if (raw.trim() === formatDimensionInput(current, unit)) return + const cm = parseDimension(raw, unit) + if (cm === null) return // unparseable: leave the row alone rather than commit a zero if (positive && cm < MIN_DIMENSION_CM) return if (cm !== current) apply(cm) } @@ -101,6 +108,7 @@ export function Inspector({ } const u = dimensionUnitLabel(unit) + const inputMode = dimensionInputMode(unit) return (
@@ -141,9 +149,8 @@ export function Inspector({ setWidth(e.target.value)} onBlur={() => commitDim(width, object.widthCm, (cm) => patch({ widthCm: cm }), true)} @@ -151,9 +158,8 @@ export function Inspector({ setHeight(e.target.value)} onBlur={() => commitDim(height, object.heightCm, (cm) => patch({ heightCm: cm }), true)} @@ -161,9 +167,8 @@ export function Inspector({ setX(e.target.value)} onBlur={() => commitDim(x, object.xCm, (cm) => patch({ xCm: cm }))} @@ -171,9 +176,8 @@ export function Inspector({ setY(e.target.value)} onBlur={() => commitDim(y, object.yCm, (cm) => patch({ yCm: cm }))} diff --git a/web/src/lib/units.test.ts b/web/src/lib/units.test.ts index 46e2280..a430a27 100644 --- a/web/src/lib/units.test.ts +++ b/web/src/lib/units.test.ts @@ -1,13 +1,15 @@ import { describe, expect, it } from 'vitest' import { - cmFromDisplay, cmFromFtIn, cmFromMeters, cmFromSpacing, + dimensionInputMode, displayFromCm, formatCm, + formatDimensionInput, formatDimensions, formatSpacing, + parseDimension, spacingFromCm, } from './units' @@ -34,14 +36,12 @@ describe('cm conversions', () => { expect(cmFromSpacing(1, 'imperial')).toBe(2.54) }) - it('round-trips display → cm → display at display precision', () => { - const imperial = [0.5, 1, 2.5, 4, 8, 12.5, 24] - for (const ft of imperial) { - expect(displayFromCm(cmFromDisplay(ft, 'imperial'), 'imperial')).toBe(ft) + it('round-trips typed → cm → displayed at display precision', () => { + for (const ft of [0.5, 1, 2.5, 4, 8, 12.5, 24]) { + expect(displayFromCm(parseDimension(String(ft), 'imperial')!, 'imperial')).toBe(ft) } - const metric = [0.15, 1, 1.22, 3.05, 10, 24.5] - for (const m of metric) { - expect(displayFromCm(cmFromDisplay(m, 'metric'), 'metric')).toBe(m) + for (const m of [0.15, 1, 1.22, 3.05, 10, 24.5]) { + expect(displayFromCm(parseDimension(String(m), 'metric')!, 'metric')).toBe(m) } }) }) @@ -93,3 +93,119 @@ describe('plant spacing (small scale)', () => { expect(spacingFromCm(30.48, 'imperial')).toBe(12) // an exactly-12in grid reads back as 12 }) }) + +describe('parseDimension (imperial compound entry)', () => { + const cm = (inches: number) => inches * 2.54 + + it('round-trips the whole-inch marks decimal feet could not represent', () => { + // The table from #59: at 0.1 ft granularity only 0", 6" and 12" were exact. + for (let inch = 0; inch <= 12; inch++) { + const input = `2' ${inch}"` + const parsed = parseDimension(input, 'imperial') + expect(parsed).toBeCloseTo(cm(24 + inch), 9) + expect(formatDimensionInput(parsed!, 'imperial')).toBe(`${inch === 12 ? 3 : 2}′ ${inch === 12 ? 0 : inch}″`) + } + }) + + it('accepts both quote styles and the spelled-out units', () => { + const want = cm(31) + for (const input of [`2' 7"`, '2′ 7″', '2ft 7in', '2 ft 7 in', "2' 7", '31"', '31 inches', '2FT 7IN']) { + expect(parseDimension(input, 'imperial')).toBeCloseTo(want, 9) + } + }) + + it('still reads a bare number as feet', () => { + expect(parseDimension('2.5', 'imperial')).toBeCloseTo(cm(30), 9) + expect(parseDimension('24', 'imperial')).toBeCloseTo(cm(288), 9) + }) + + it('reads feet alone', () => { + expect(parseDimension("2'", 'imperial')).toBeCloseTo(cm(24), 9) + expect(parseDimension('2ft', 'imperial')).toBeCloseTo(cm(24), 9) + }) + + // The bug this issue would otherwise produce: attaching the sign to the feet + // term alone gives -2ft + 6in = -18", not -30". + it('applies the sign to the whole value, not just the feet', () => { + expect(parseDimension(`-2' 6"`, 'imperial')).toBeCloseTo(cm(-30), 9) + expect(parseDimension('−2′ 6″', 'imperial')).toBeCloseTo(cm(-30), 9) // unicode minus + expect(parseDimension('-31"', 'imperial')).toBeCloseTo(cm(-31), 9) + expect(parseDimension('-2', 'imperial')).toBeCloseTo(cm(-24), 9) + }) + + it('rejects garbage rather than committing a zero', () => { + for (const input of ['', ' ', 'abc', `'`, `"`, `2' 7" 3`, '2 3', 'ft', '--2', '2..5']) { + expect(parseDimension(input, 'imperial')).toBeNull() + } + }) +}) + +describe('parseDimension (metric)', () => { + it('takes decimal meters, with or without the unit', () => { + expect(parseDimension('1.22', 'metric')).toBe(122) + expect(parseDimension('1.22m', 'metric')).toBe(122) + expect(parseDimension('10 m', 'metric')).toBe(1000) + expect(parseDimension('-3', 'metric')).toBe(-300) + }) + + it('does not accept feet-and-inches syntax', () => { + expect(parseDimension(`2' 7"`, 'metric')).toBeNull() + expect(parseDimension('31"', 'metric')).toBeNull() + }) + + it('rejects garbage', () => { + expect(parseDimension('', 'metric')).toBeNull() + expect(parseDimension('abc', 'metric')).toBeNull() + }) +}) + +describe('formatDimensionInput', () => { + it('renders imperial as feet and inches, carrying 12″ up', () => { + expect(formatDimensionInput(0, 'imperial')).toBe('0′ 0″') + expect(formatDimensionInput(30.48, 'imperial')).toBe('1′ 0″') + expect(formatDimensionInput(78.74, 'imperial')).toBe('2′ 7″') + expect(formatDimensionInput(-76.2, 'imperial')).toBe('-2′ 6″') + }) + + it('keeps one decimal inch, so a dragged position reads honestly', () => { + // A bed dragged to an arbitrary cm must not render as a whole inch — that's + // what would let a mere blur snap it. + expect(formatDimensionInput(80.90185676392574, 'imperial')).toBe('2′ 7.9″') + }) + + it('is the exact string parseDimension round-trips', () => { + for (const value of [0, 30.48, 78.74, 80.9, -76.2, 731.52]) { + const text = formatDimensionInput(value, 'imperial') + const back = parseDimension(text, 'imperial') + expect(formatDimensionInput(back!, 'imperial')).toBe(text) + } + }) + + it('leaves metric as decimal meters', () => { + expect(formatDimensionInput(122, 'metric')).toBe('1.22') + 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') + }) +}) diff --git a/web/src/lib/units.ts b/web/src/lib/units.ts index 32a78cf..fcdaada 100644 --- a/web/src/lib/units.ts +++ b/web/src/lib/units.ts @@ -40,11 +40,6 @@ export function cmFromMeters(meters: number): number { return roundCm(meters * CM_PER_METER) } -/** A value typed in the given unit (meters, or feet) → centimeters. */ -export function cmFromDisplay(value: number, unit: UnitPref): number { - return unit === 'imperial' ? cmFromFtIn(value) : cmFromMeters(value) -} - /** Centimeters → a number in the given unit, for prefilling an input field. * Rounded so cm-quantization doesn't show through (e.g. 244cm shows as 8 ft, * not 8.01): meters to the cm (2 dp), feet to 0.1 ft. */ @@ -56,17 +51,116 @@ export function displayFromCm(cm: number, unit: UnitPref): number { return Math.round((cm / CM_PER_METER) * 100) / 100 } +// --- Dimension entry ------------------------------------------------------- +// Imperial dimensions are typed the way people say them — 2' 7", not 2.6 — while +// a bare number still means feet so nothing anyone typed before this existed +// breaks. Metric entry is decimal meters, unchanged. + +// Feet, then optionally inches. The inch mark is optional once feet are explicit, +// because "2' 7" is a natural thing to type and unambiguous. +const FT_IN_RE = /^(\d*\.?\d+)\s*(?:'|ft|feet|foot)(?:\s*(\d*\.?\d+)\s*(?:"|in|inch|inches)?)?$/ +// Inches alone, where the mark is required — a bare number means feet. +const IN_ONLY_RE = /^(\d*\.?\d+)\s*(?:"|in|inch|inches)$/ +// A bare decimal, which means feet (imperial) or meters (metric). +const BARE_RE = /^(\d*\.?\d+)$/ +const METERS_RE = /^(\d*\.?\d+)\s*m?$/ + +/** Normalize the characters people actually paste: typographic quotes and the + * various unicode dashes, so text copied from anywhere parses like text typed + * here. Lowercased so "FT"/"In" work too. */ +function normalizeEntry(s: string): string { + return s + .trim() + .replace(/[′’‵]/g, "'") + .replace(/[″”‶]/g, '"') + .replace(/[−–—]/g, '-') + .toLowerCase() +} + +/** Split a leading sign off a normalized entry. The sign belongs to the WHOLE + * value, not just its first term: -2' 6" is −(2ft + 6in) = −30in, not −2ft + 6in. */ +function splitSign(s: string): { sign: number; rest: string } { + if (s.startsWith('-')) return { sign: -1, rest: s.slice(1).trim() } + if (s.startsWith('+')) return { sign: 1, rest: s.slice(1).trim() } + return { sign: 1, rest: s } +} + +/** + * Parse a typed dimension into centimeters, or null if it isn't a dimension. + * + * Imperial accepts `2' 7"`, `2′ 7″`, `2ft 7in`, `2' 7`, `31"`, `2'`, and a bare + * `2.5` still meaning 2½ feet. Metric accepts decimal meters with an optional + * `m`. Null means "don't commit anything" — never a silent zero, which would + * turn a typo into a destructive edit. + */ +export function parseDimension(input: string, unit: UnitPref): number | null { + const { sign, rest } = splitSign(normalizeEntry(input)) + if (rest === '') return null + + if (unit !== 'imperial') { + const m = rest.match(METERS_RE) + return m ? sign * cmFromMeters(parseFloat(m[1])) : null + } + + const bare = rest.match(BARE_RE) + if (bare) return sign * cmFromFtIn(parseFloat(bare[1])) + + const ftIn = rest.match(FT_IN_RE) + if (ftIn) return sign * cmFromFtIn(parseFloat(ftIn[1]), ftIn[2] ? parseFloat(ftIn[2]) : 0) + + const inOnly = rest.match(IN_ONLY_RE) + if (inOnly) return sign * cmFromFtIn(0, parseFloat(inOnly[1])) + + return null +} + +/** + * Centimeters → the string a dimension input shows, and the exact string + * parseDimension round-trips. Imperial reads `2′ 7″`; inches carry one decimal + * (`7′ 10.5″`) because 0.1″ is ~0.25 cm, so what's displayed stays honest about + * what's stored rather than rounding a dragged position to the nearest inch. + */ +export function formatDimensionInput(cm: number, unit: UnitPref): string { + if (unit !== 'imperial') return String(displayFromCm(cm, unit)) + 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) * scale) / scale + if (inches >= INCHES_PER_FOOT) { + feet += 1 + inches = 0 + } + 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`