Просмотр исходного кода

Stop the part typeahead opening by itself on every row (#260)

Opening a work order with saved parts dropped a suggestion panel over every
row at once, before anyone had touched the keyboard.

The panel's only condition for being open was that the query is long enough
to search on. That is true of somebody typing, and equally true of a name
loaded from a saved record, and the component could not tell the two apart.
Rows already linked to stock were spared, because those are disabled, so it
was the free-typed lines that covered the fields below them.

It now starts closed when it is mounted with something already in the field,
and opens when the value actually changes. A parent re-rendering for its own
reasons is not someone typing, so the comparison is against the previous
value rather than a run on every render.

The existing tests all rendered straight to the final query, which is the
state this bug lived in and one the component is never actually in. They
type into an empty field now, which is what a person does.
Bernt Christian Egeland 1 месяц назад
Родитель
Сommit
8e66a8bce7

+ 42 - 13
src/__tests__/features/inventory/part-name-suggestions.test.tsx

@@ -40,12 +40,22 @@ const PARTS: PartSuggestion[] = [
   { id: 'p5', name: 'Brake cable', partNumber: 'BC-7', unitCost: 8, sellPrice: 20, quantity: 1 },
 ]
 
+/**
+ * Arrives at `query` the way a person does: by typing into an empty field.
+ *
+ * The panel deliberately stays shut when it is mounted with a name already in
+ * it, because that is a saved record being opened rather than someone typing.
+ * Rendering straight to the final value would test a state the component is
+ * never in.
+ */
 function setup(
   query: string,
   over: Partial<React.ComponentProps<typeof PartNameSuggestions>> = {}
 ) {
   const onSelect = vi.fn()
-  render(<PartNameSuggestions query={query} parts={PARTS} onSelect={onSelect} {...over} />)
+  const props = { parts: PARTS, onSelect, ...over }
+  const { rerender } = render(<PartNameSuggestions query="" {...props} />)
+  rerender(<PartNameSuggestions query={query} {...props} />)
   return { onSelect }
 }
 
@@ -54,6 +64,33 @@ function shown() {
   return screen.queryAllByRole('button').map((b) => b.textContent ?? '')
 }
 
+describe('opening a saved record', () => {
+  it('stays shut when the field already has a name in it', () => {
+    // The reported bug. A work order with five free-typed parts opened five
+    // suggestion panels the moment it loaded, one over each row, before
+    // anybody had touched the keyboard.
+    render(<PartNameSuggestions query="brake" parts={PARTS} onSelect={vi.fn()} />)
+    expect(shown()).toHaveLength(0)
+  })
+
+  it('opens as soon as that name is edited', () => {
+    const props = { parts: PARTS, onSelect: vi.fn() }
+    const { rerender } = render(<PartNameSuggestions query="brake" {...props} />)
+    expect(shown()).toHaveLength(0)
+
+    rerender(<PartNameSuggestions query="brake p" {...props} />)
+    expect(shown().length).toBeGreaterThan(0)
+  })
+
+  it('stays shut on a re-render that does not change the name', () => {
+    // A parent re-rendering for its own reasons is not someone typing.
+    const props = { parts: PARTS, onSelect: vi.fn() }
+    const { rerender } = render(<PartNameSuggestions query="brake" {...props} />)
+    rerender(<PartNameSuggestions query="brake" {...props} />)
+    expect(shown()).toHaveLength(0)
+  })
+})
+
 describe('PartNameSuggestions', () => {
   it('shows nothing until enough has been typed', () => {
     setup('b')
@@ -125,7 +162,7 @@ describe('PartNameSuggestions', () => {
     const noSell: PartSuggestion[] = [
       { id: 'z', name: 'Zero priced', partNumber: null, unitCost: 18, sellPrice: 0, quantity: 3 },
     ]
-    render(<PartNameSuggestions query="zero" parts={noSell} onSelect={vi.fn()} />)
+    setup('zero', { parts: noSell })
     expect(shown()[0]).toContain('$18.00')
     expect(shown()[0]).not.toContain('$0.00')
   })
@@ -134,15 +171,7 @@ describe('PartNameSuggestions', () => {
     const p: PartSuggestion[] = [
       { id: 'm', name: 'Marked up', partNumber: null, unitCost: 10, sellPrice: 25, quantity: 2 },
     ]
-    render(
-      <PartNameSuggestions
-        query="marked"
-        parts={p}
-        onSelect={vi.fn()}
-        markupAppliesToInventory
-        defaultMarkupPercent={40}
-      />
-    )
+    setup('marked', { parts: p, markupAppliesToInventory: true, defaultMarkupPercent: 40 })
     expect(shown()[0]).toContain('$14.00')
   })
 
@@ -158,7 +187,7 @@ describe('PartNameSuggestions', () => {
         quantity: -3,
       },
     ]
-    render(<PartNameSuggestions query="out" parts={p} onSelect={vi.fn()} />)
+    setup('out', { parts: p })
     const text = shown().join(' ')
     expect(text).toContain('suggestions.outOfStock')
     expect(text).toContain('suggestions.onBackorder')
@@ -175,7 +204,7 @@ describe('PartNameSuggestions', () => {
         quantity: 3,
       },
     ]
-    render(<PartNameSuggestions query="brake" parts={long} onSelect={vi.fn()} />)
+    setup('brake', { parts: long })
     expect(screen.getByTitle(long[0].name)).toBeTruthy()
   })
 

+ 13 - 2
src/features/inventory/Components/PartNameSuggestions.tsx

@@ -78,7 +78,14 @@ export function PartNameSuggestions({
 }) {
   const t = useTranslations('inventory')
   const formatCurrency = useFormatCurrency()
-  const [dismissed, setDismissed] = useState(false)
+  // Starts closed when the field arrives with something already in it.
+  //
+  // The panel is a typeahead, and the only signal it had was "the query is
+  // long enough", which is equally true of a name loaded from a saved record.
+  // Opening a work order with five free-typed parts therefore opened five
+  // panels at once, before anyone touched the keyboard.
+  const [dismissed, setDismissed] = useState(() => query.trim().length > 0)
+  const previousQuery = useRef(query)
   const containerRef = useRef<HTMLDivElement>(null)
 
   const trimmed = query.trim().toLowerCase()
@@ -91,8 +98,12 @@ export function PartNameSuggestions({
       .slice(0, MAX_SUGGESTIONS)
   }, [parts, trimmed, disabled])
 
-  // Typing again after dismissing should bring the list back.
+  // Typing again after dismissing should bring the list back. Compared against
+  // the last value rather than run on every render, so the mount pass does not
+  // immediately undo the closed state above.
   useEffect(() => {
+    if (query === previousQuery.current) return
+    previousQuery.current = query
     setDismissed(false)
   }, [query])