Преглед на файлове

fix(inventory): hex colour field accepts character-by-character typing (#1407)

  Reporter typed into the Add Spool modal's hex colour input and only
  the first character stuck - everything after that defaulted to "0"
  with no way to override except by pasting the full hex.

  Pre-fix, the #1055 fix aggressively normalized the input to a valid
  8-char rgba on every keystroke. After typing the first char the
  controlled input value snapped to e.g. "A00000", the browser placed
  the cursor at the end, and the user's next keystroke landed at
  position 7. The #1055 fix's 7-char branch then truncated that byte
  away, leaving the form state unchanged - so the user appeared to
  type nothing.

  Fix splits the typing-state from the backend-state:

  - The hex input gets its own `hexDraft` useState holding 0-6 chars
    freely. Typing one char at a time works naturally because the
    controlled value matches what the user typed.
  - `updateField('rgba', ...)` fires only when the draft reaches a
    complete 6-char RGB (commits as `<6chars>FF`). Below that, the
    form state stays untouched - no mid-keystroke snap.
  - On blur, a partial 1-5 char draft is right-padded with `0` and
    committed. Keeps the #1055 invariant: anything reaching the
    backend is exactly 8 hex chars matching /^[0-9A-F]{8}$/.
  - A `useEffect` resyncs the draft when an external action (color
    picker, swatch click, edit-mode load) changes the canonical hex.
  - Paste of 7-/8-char strings truncates to the leading RGB. Bambu
    filaments are opaque; the UI never exposed an alpha affordance,
    so dropping the (undocumented) paste-with-alpha case is fine.
maziggy преди 1 седмица
родител
ревизия
8d52c713ef

Файловите разлики са ограничени, защото са твърде много
+ 2 - 0
CHANGELOG.md


+ 107 - 46
frontend/src/__tests__/components/spool-form/ColorSectionHexInput.test.tsx

@@ -1,23 +1,30 @@
 /**
- * Regression tests for the ColorSection hex input normalization (#1055).
+ * Regression tests for the ColorSection hex input.
  *
- * The original bug: typing 5 hex chars on the RRGGBB field produced a 7-char
- * rgba ("FFFFF" + "FF" alpha = 7 chars); typing 7 chars left the 7-char string
- * unpadded. Either way the value passed frontend validation, survived a backend
- * PATCH (SpoolUpdate had no pattern constraint), and then bricked the entire
- * Filaments page because SpoolResponse enforced the 8-char pattern on serialize
- * and one bad row 500'd the whole list endpoint.
+ * Original bug (#1055): typing 5 or 7 hex chars produced a 7-char rgba
+ * ("FFFFF" + "FF" alpha = 7 chars). That passed frontend validation, survived
+ * a backend PATCH (SpoolUpdate had no pattern constraint), and then bricked
+ * the entire Filaments page because SpoolResponse enforced the 8-char pattern
+ * on serialize and one bad row 500'd the whole list endpoint.
  *
- * The input now emits a valid 8-char RRGGBBAA on every keystroke: shorter input
- * is right-padded with '0' and given FF alpha; 7-char input drops the stray 7th
- * char; 8-char paste passes through unchanged.
+ * Second bug (#1407): the original #1055 fix solved the "no malformed rgba"
+ * problem by aggressively normalizing to 8 chars on EVERY keystroke. That
+ * worked for the data contract but broke typing: after the first char the
+ * controlled input value snapped to e.g. "A00000", the cursor jumped to the
+ * end, and the user's next keystroke landed at position 7 — which the 7-char
+ * branch then truncated away. Every keystroke past the first was lost.
  *
- * These tests drive the onChange handler directly (via fireEvent.change) rather
- * than userEvent.type so each assertion exercises a specific input length. The
- * component itself is a controlled input whose displayed value derives from
- * formData.rgba.substring(0, 6), so the real-world UX of typing one char at a
- * time is quirkier than the handler contract — but the handler contract is
- * what this regression guards.
+ * Current contract:
+ *   - The hex input has its own draft state (0–6 chars) decoupled from
+ *     `formData.rgba`. Typing one char at a time works naturally.
+ *   - `updateField('rgba', ...)` is only called once the draft reaches a full
+ *     6-char hex — at which point we append "FF" alpha for an 8-char result.
+ *   - On blur, a partial draft (1–5 chars) is right-padded with '0' and
+ *     committed. Preserves the #1055 invariant: anything the backend ever
+ *     sees is exactly 8 hex chars.
+ *   - Paste of 7/8-char strings (rare alpha-channel case) truncates to the
+ *     leading 6-char RGB on input. Bambu filaments are opaque, so an alpha
+ *     affordance was never exposed in the UI.
  */
 
 import { describe, it, expect, vi } from 'vitest';
@@ -57,45 +64,87 @@ function lastRgba(updateField: ReturnType<typeof vi.fn>): string | undefined {
   return rgbaCalls.at(-1)?.[1] as string | undefined;
 }
 
-describe('ColorSection hex input normalization (#1055)', () => {
-  it('pads a 6-char RRGGBB to 8-char RRGGBBAA with FF alpha', () => {
-    const { hexInput, updateField } = renderColorSection();
-    fireEvent.change(hexInput, { target: { value: 'FF0000' } });
-    expect(lastRgba(updateField)).toBe('FF0000FF');
+function rgbaCallCount(updateField: ReturnType<typeof vi.fn>): number {
+  return updateField.mock.calls.filter(([key]) => key === 'rgba').length;
+}
+
+describe('ColorSection hex input — typing UX (#1407)', () => {
+  it('reflects each keystroke in the draft input value (the #1407 trigger)', () => {
+    // Pre-fix: after typing the first char the controlled value snapped to
+    // e.g. "A00000" with cursor at position 6, so the user's next keystroke
+    // landed at position 7 and got truncated by the 7-char branch. The draft
+    // state must now hold whatever the user has typed, regardless of length.
+    const { hexInput } = renderColorSection();
+
+    fireEvent.change(hexInput, { target: { value: 'A' } });
+    expect(hexInput.value).toBe('A');
+
+    fireEvent.change(hexInput, { target: { value: 'AB' } });
+    expect(hexInput.value).toBe('AB');
+
+    fireEvent.change(hexInput, { target: { value: 'ABC' } });
+    expect(hexInput.value).toBe('ABC');
+
+    fireEvent.change(hexInput, { target: { value: 'ABCDE' } });
+    expect(hexInput.value).toBe('ABCDE');
   });
 
-  it('passes an 8-char RRGGBBAA paste through unchanged', () => {
+  it('does NOT commit to formData.rgba while the draft is partial (1–5 chars)', () => {
+    // Committing a partial value mid-typing was the entire cause of #1407 —
+    // the controlled value snap then re-rendered the input back over what
+    // the user was typing. Defer commit until the draft is a complete RGB.
     const { hexInput, updateField } = renderColorSection();
-    fireEvent.change(hexInput, { target: { value: '00112233' } });
-    expect(lastRgba(updateField)).toBe('00112233');
+
+    for (const partial of ['A', 'AB', 'ABC', 'ABCD', 'ABCDE']) {
+      updateField.mockClear();
+      fireEvent.change(hexInput, { target: { value: partial } });
+      expect(rgbaCallCount(updateField)).toBe(0);
+    }
   });
 
-  it('drops the stray 7th char — the exact #1055 trigger pattern', () => {
+  it('commits to formData.rgba once the draft reaches 6 chars', () => {
     const { hexInput, updateField } = renderColorSection();
-    fireEvent.change(hexInput, { target: { value: 'FFFFFFF' } });
-    // Previously emitted "FFFFFFF" (7 chars) verbatim. Must now be 8 chars.
-    const rgba = lastRgba(updateField);
-    expect(rgba).toBe('FFFFFFFF');
-    expect(rgba).toMatch(/^[0-9A-F]{8}$/);
+    fireEvent.change(hexInput, { target: { value: 'FF0000' } });
+    expect(lastRgba(updateField)).toBe('FF0000FF');
   });
 
-  it('pads a 5-char input to 8 chars instead of emitting a 7-char rgba', () => {
-    // 5-char + 'FF' alpha = 7 chars was the other #1055 trigger pattern.
-    // Right-pad RGB to 6 with '0' so the output is always 8 chars.
+  it('on blur, pads a partial draft to 6 chars and commits', () => {
+    // Backstop: a user who leaves the field with "AB" must end up with a
+    // valid form state, not a malformed rgba (#1055 invariant).
     const { hexInput, updateField } = renderColorSection();
-    fireEvent.change(hexInput, { target: { value: 'FFFFF' } });
+
+    fireEvent.change(hexInput, { target: { value: 'AB' } });
+    fireEvent.blur(hexInput);
+
     const rgba = lastRgba(updateField);
-    expect(rgba).toBe('FFFFF0FF');
+    expect(rgba).toBe('AB0000FF');
     expect(rgba).toMatch(/^[0-9A-F]{8}$/);
   });
 
-  it('pads any partial input to exactly 8 chars — never 7', () => {
-    // The essential invariant: for every legal input length (0..8), the
-    // emitted rgba must be 8 chars. Anything else risks reintroducing #1055.
+  it('on blur, does NOT commit when the draft is empty', () => {
+    // Clearing the field then tabbing away must not auto-fill the form with
+    // a synthetic colour the user never picked.
+    const { hexInput, updateField } = renderColorSection({ rgba: 'FF0000FF' });
+    updateField.mockClear();
+
+    fireEvent.change(hexInput, { target: { value: '' } });
+    fireEvent.blur(hexInput);
+
+    expect(rgbaCallCount(updateField)).toBe(0);
+  });
+});
+
+describe('ColorSection hex input — backend invariant (#1055)', () => {
+  it('committed rgba is always exactly 8 hex chars', () => {
+    // The essential invariant: anything that reaches the backend must match
+    // /^[0-9A-F]{8}$/. The new contract enforces this two ways — commit only
+    // at length 6 (always padded with "FF") and pad on blur.
     const { hexInput, updateField } = renderColorSection();
-    for (const input of ['', 'F', 'FF', 'FFF', 'FFFF', 'FFFFF', 'FFFFFF', 'FFFFFFF', 'FFFFFFFF']) {
+
+    for (const input of ['F', 'FF', 'FFF', 'FFFF', 'FFFFF', 'FFFFFF']) {
       updateField.mockClear();
       fireEvent.change(hexInput, { target: { value: input } });
+      fireEvent.blur(hexInput);
       const rgba = lastRgba(updateField);
       expect(rgba).toBeDefined();
       expect(rgba!.length).toBe(8);
@@ -103,17 +152,29 @@ describe('ColorSection hex input normalization (#1055)', () => {
     }
   });
 
-  it('ignores input past 8 chars (no updateField call)', () => {
-    const { hexInput, updateField } = renderColorSection({ rgba: 'FFFFFFFF' });
-    updateField.mockClear();
+  it('truncates paste of 7–8 chars to the leading RGB triplet', () => {
+    // Pre-fix, an 8-char paste passed through and a 7-char paste dropped the
+    // last char. Both are rare alpha-channel cases; Bambu filaments are
+    // opaque and the UI exposes no alpha affordance, so we truncate to the
+    // leading 6-char RGB and force FF alpha. Loses the (undocumented) 8-char
+    // paste-with-alpha case, gains uniform commit-at-6 semantics.
+    const { hexInput, updateField } = renderColorSection();
+
     fireEvent.change(hexInput, { target: { value: '0011223344' } });
-    expect(updateField.mock.calls.filter(([k]) => k === 'rgba')).toHaveLength(0);
+    expect(hexInput.value).toBe('001122');
+    expect(lastRgba(updateField)).toBe('001122FF');
   });
 
-  it('strips non-hex characters before normalizing', () => {
-    // '#FF00ZZ' → strip '#' and non-hex → 'FF00' (4 chars) → pad to 6 + FF alpha
+  it('strips non-hex characters', () => {
+    // '#FF00ZZ' → strip '#' and 'Z' → 'FF00' (length 4, no commit yet).
+    // Append two more hex chars to reach length 6, then commit.
     const { hexInput, updateField } = renderColorSection();
+
     fireEvent.change(hexInput, { target: { value: '#FF00ZZ' } });
-    expect(lastRgba(updateField)).toBe('FF0000FF');
+    expect(hexInput.value).toBe('FF00');
+    expect(rgbaCallCount(updateField)).toBe(0);
+
+    fireEvent.change(hexInput, { target: { value: 'FF0011' } });
+    expect(lastRgba(updateField)).toBe('FF0011FF');
   });
 });

+ 42 - 13
frontend/src/components/spool-form/ColorSection.tsx

@@ -40,6 +40,19 @@ export function ColorSection({
   // Current hex without # prefix
   const currentHex = formData.rgba.replace('#', '').substring(0, 6);
 
+  // Draft state for the manual hex input. Decoupled from `formData.rgba` so
+  // mid-typing values (1–5 chars) don't trigger the immediate auto-pad-to-6
+  // that used to drop every keystroke past the first (#1407). The draft is
+  // committed to `formData.rgba` once it reaches 6 chars, or on blur (where
+  // shorter drafts are padded with `0` so the backend never sees a malformed
+  // rgba — preserves the #1055 invariant). The useEffect re-syncs the draft
+  // whenever an external action (color picker, swatch click, edit-mode load)
+  // changes `currentHex`.
+  const [hexDraft, setHexDraft] = useState(currentHex);
+  useEffect(() => {
+    setHexDraft(currentHex);
+  }, [currentHex]);
+
   const isSelected = (hex: string) => {
     return currentHex.toUpperCase() === hex.toUpperCase();
   };
@@ -393,20 +406,36 @@ export function ColorSection({
                 type="text"
                 className="w-full pl-7 pr-3 py-2 bg-bambu-dark border border-bambu-dark-tertiary rounded-lg text-white text-sm font-mono uppercase focus:outline-none focus:border-bambu-green"
                 placeholder="RRGGBB"
-                value={currentHex.toUpperCase()}
+                value={hexDraft.toUpperCase()}
                 onChange={(e) => {
-                  const val = e.target.value.replace('#', '').replace(/[^0-9A-Fa-f]/g, '').toUpperCase();
-                  if (val.length > 8) return;
-                  // Normalize to a valid 8-char RRGGBBAA on every keystroke so
-                  // the backend never receives a malformed rgba (#1055). 8-char
-                  // paste passes through; 7-char drops the stray typo; anything
-                  // shorter is right-padded with '0' to a full RGB triplet and
-                  // given FF alpha. Prior logic emitted 3/5/7-char strings mid-
-                  // typing that PATCH would accept (SpoolUpdate was unchecked)
-                  // and later 500 the list endpoint on response serialization.
-                  const rgba =
-                    val.length === 8 ? val : val.length === 7 ? val.substring(0, 6) + 'FF' : val.padEnd(6, '0') + 'FF';
-                  updateField('rgba', rgba);
+                  // Sanitize: drop `#`, non-hex chars, uppercase. 7/8-char
+                  // pastes (with an alpha byte) truncate to the leading RGB —
+                  // Bambu filaments are opaque, so we never expose an alpha
+                  // affordance and discarding pasted alpha is fine. The draft
+                  // can hold 0–6 chars freely while the user types; we only
+                  // commit to `formData.rgba` (and through to the backend)
+                  // once the value is a complete 6-char hex, which keeps the
+                  // #1055 invariant intact without re-introducing the
+                  // mid-keystroke truncation that broke typing in #1407.
+                  const sanitized = e.target.value
+                    .replace('#', '')
+                    .replace(/[^0-9A-Fa-f]/g, '')
+                    .toUpperCase();
+                  const next = sanitized.length > 6 ? sanitized.substring(0, 6) : sanitized;
+                  setHexDraft(next);
+                  if (next.length === 6) {
+                    updateField('rgba', next + 'FF');
+                  }
+                }}
+                onBlur={() => {
+                  // User left the field with a partial value — pad to 6 chars
+                  // and commit so the form state always carries a valid rgba
+                  // when submitted.
+                  if (hexDraft.length > 0 && hexDraft.length < 6) {
+                    const padded = hexDraft.padEnd(6, '0');
+                    setHexDraft(padded);
+                    updateField('rgba', padded + 'FF');
+                  }
                 }}
               />
             </div>

Файловите разлики са ограничени, защото са твърде много
+ 0 - 0
static/assets/index-BvnKyMoU.js


+ 1 - 1
static/index.html

@@ -26,7 +26,7 @@
 
     <!-- Splash screens for iOS -->
     <link rel="apple-touch-startup-image" href="/img/android-chrome-512x512.png" />
-    <script type="module" crossorigin src="/assets/index-DbgmsH_e.js"></script>
+    <script type="module" crossorigin src="/assets/index-BvnKyMoU.js"></script>
     <link rel="stylesheet" crossorigin href="/assets/index-KYwGxnG9.css">
   </head>
   <body>

Някои файлове не бяха показани, защото твърде много файлове са промени