Prompt clean-up & improvements (#891)

* Prompt should clear value when it's closed
* Prompt should clear value when it's closed
* Remove <form> within Prompt to avoid conflict with other onSubmit handlers. Instead monitor for 'Enter' and 'Escape' and act accordingly
* Changed PromptDemo's onCancel callback

Author: Luke Bowerman

CHANGELOG.md

@@ -18,7 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
-- updated documentation for Filedset to include label and legend
+- `Fieldset` - updated documentation to include label and legend
 - `menuItemStyleContext` in `MenuContext` uses a new interface which contains "preserved icon space"-related properties
 - `MenuItem` renders an empty Box with the same size as the icon(s) of sibling `MenuItem's (if any)
 - `MenuList`, `MenuGroup` contain piece of state the tracks the size of the preserved icon space
@@ -27,7 +27,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 
 - `ToggleSwitch` React warning
-- add placeholder attribute to InlineInputText
+- `Prompt`
+  - Clears out old `defaultValue` when new `defaultValue` is passed in
+  - Clears out any user input after pressing cancel button
+  - Updated test suite to prevent future regressions for the above fixes
+- `InlineInputText` add placeholder attribute 
 
 ### Removed
 

packages/components/src/Modal/Prompt/Prompt.test.tsx

@@ -25,7 +25,7 @@
  */
 
 import { fireEvent } from '@testing-library/react'
-import React from 'react'
+import React, { useState } from 'react'
 
 import { renderWithTheme } from '@looker/components-test-utils'
 import { semanticColors, SemanticColors } from '@looker/design-tokens'
@@ -52,22 +52,27 @@ afterEach(() => {
 })
 
 test('<Prompt/> with defaults', () => {
-  const { getByText, queryByText } = renderWithTheme(
+  const { getByText, getByPlaceholderText, queryByText } = renderWithTheme(
     <Prompt {...requiredProps}>
-      {(open) => <Button onClick={open}>Sesame</Button>}
+      {(open) => <Button onClick={open}>Open Prompt</Button>}
     </Prompt>
   )
 
-  const opener = getByText('Sesame')
+  const opener = getByText('Open Prompt')
   fireEvent.click(opener)
 
-  const button = getByText('Save')
+  const saveButton = getByText('Save')
+  const input = getByPlaceholderText(requiredProps.inputLabel)
 
-  expect(getByText(requiredProps.inputLabel)).toBeVisible()
+  expect(input).toBeVisible()
   expect(getByText(requiredProps.title)).toBeVisible()
-  expect(button).toHaveStyle(`background: ${semanticColors.primary.main}`)
+  expect(saveButton).toHaveStyle(`background: ${semanticColors.primary.main}`)
+
+  fireEvent.click(saveButton)
+  expect(requiredProps.onSave).toHaveBeenCalledTimes(0)
 
-  fireEvent.submit(button)
+  fireEvent.change(input, { target: { value: 'Has Text In It' } })
+  fireEvent.click(saveButton)
   expect(requiredProps.onSave).toHaveBeenCalledTimes(1)
 
   expect(queryByText(requiredProps.inputLabel)).toBeNull()
@@ -77,11 +82,11 @@ test('<Prompt/> with defaults', () => {
 test('<Prompt/> with custom props', () => {
   const { getByDisplayValue, getByText } = renderWithTheme(
     <Prompt {...optionalProps} {...requiredProps}>
-      {(open) => <Button onClick={open}>Sesame</Button>}
+      {(open) => <Button onClick={open}>Open Prompt</Button>}
     </Prompt>
   )
 
-  const opener = getByText('Sesame')
+  const opener = getByText('Open Prompt')
   fireEvent.click(opener)
 
   const saveButton = getByText(optionalProps.saveLabel)
@@ -98,3 +103,53 @@ test('<Prompt/> with custom props', () => {
   expect(optionalProps.onCancel).toHaveBeenCalledTimes(1)
   expect(requiredProps.onSave).toHaveBeenCalledTimes(0)
 })
+
+test('<Prompt /> clears value after closing', () => {
+  const { getByText, getByPlaceholderText } = renderWithTheme(
+    <Prompt {...requiredProps}>
+      {(open) => <Button onClick={open}>Open Prompt</Button>}
+    </Prompt>
+  )
+
+  const opener = getByText('Open Prompt')
+  fireEvent.click(opener)
+
+  const cancelButton = getByText('Cancel')
+  let input = getByPlaceholderText(requiredProps.inputLabel)
+
+  fireEvent.change(input, { target: { value: 'Hello World' } })
+  expect(input).toHaveValue('Hello World')
+  fireEvent.click(cancelButton)
+
+  fireEvent.click(opener)
+  // Note: Need to requery for the input; not doing so results in stale value on the input element
+  input = getByPlaceholderText(requiredProps.inputLabel)
+  expect(input).toHaveValue('')
+})
+
+test('<Prompt /> updates when defaultValue changes', () => {
+  const PromptTest = () => {
+    const [defaultValue, setDefaultValue] = useState('Gouda')
+
+    return (
+      <>
+        <Prompt {...requiredProps} defaultValue={defaultValue}>
+          {(open) => <Button onClick={open}>Open Prompt</Button>}
+        </Prompt>
+        <Button onClick={() => setDefaultValue('Swiss')}>
+          Set Default Value to Swiss
+        </Button>
+      </>
+    )
+  }
+
+  const { getByText, getByDisplayValue } = renderWithTheme(<PromptTest />)
+
+  fireEvent.click(getByText('Open Prompt'))
+  getByDisplayValue('Gouda')
+  fireEvent.click(getByText('Cancel'))
+
+  fireEvent.click(getByText('Set Default Value to Swiss'))
+  fireEvent.click(getByText('Open Prompt'))
+  getByDisplayValue('Swiss')
+})

packages/components/src/Modal/Prompt/PromptDialog.tsx

@@ -24,7 +24,15 @@
 
  */
 
-import React, { FC, FormEvent, ReactNode, useState, useCallback } from 'react'
+import React, {
+  FC,
+  FormEvent,
+  KeyboardEvent,
+  ReactNode,
+  useState,
+  useCallback,
+  useEffect,
+} from 'react'
 import { SemanticColors } from '@looker/design-tokens'
 import { Button, ButtonTransparent } from '../../Button'
 import { Label, InputText } from '../../Form'
@@ -45,7 +53,7 @@ export interface PromptBaseProps {
   /**
    * Callback if user clicks Cancel button or closes the dialog
    */
-  onCancel?: (close: () => void) => void
+  onCancel?: () => void
   /**
    * Callback that is triggered when submit button is pressed
    */
@@ -101,49 +109,63 @@ export const PromptDialog: FC<PromptDialogProps> = ({
   const [value, setValue] = useState(defaultValue)
   const hasValue = !!value.trim()
 
+  useEffect(() => {
+    setValue(defaultValue)
+  }, [defaultValue])
+
   const onChange = (event: FormEvent<HTMLInputElement>) => {
     setValue(event.currentTarget.value)
   }
 
-  const onSubmit = (event: FormEvent<HTMLElement>) => {
-    event.preventDefault()
+  const onSubmit = () => {
     onSave(value)
     close()
+    setValue('')
   }
 
   const cancel = useCallback(() => {
-    if (onCancel) {
-      onCancel(close)
-    } else {
-      close()
-    }
+    close()
+    setValue('')
+    onCancel && onCancel()
   }, [close, onCancel])
 
+  const onKeyDown = (event: KeyboardEvent<HTMLInputElement>) => {
+    if (event.key === 'Enter' && hasValue) {
+      onSubmit()
+    } else if (event.key === 'Escape') {
+      cancel()
+    }
+  }
+
   return (
     <Dialog width="30rem" isOpen={isOpen} onClose={cancel}>
-      <form onSubmit={onSubmit}>
-        <ModalHeader hideClose>{title}</ModalHeader>
-        <ModalContent>
-          <VisuallyHidden>
-            <Label htmlFor="promptInput">{inputLabel}</Label>
-          </VisuallyHidden>
-          <InputText
-            id="promptInput"
-            placeholder={inputLabel}
-            onChange={onChange}
-            width="100%"
-            value={value}
-          />
-        </ModalContent>
-        <ModalFooter secondary={secondary}>
-          <Button disabled={!hasValue} type="submit" color="primary">
-            {saveLabel}
-          </Button>
-          <ButtonTransparent type="reset" color={cancelColor} onClick={cancel}>
-            {cancelLabel}
-          </ButtonTransparent>
-        </ModalFooter>
-      </form>
+      <ModalHeader hideClose>{title}</ModalHeader>
+      <ModalContent>
+        <VisuallyHidden>
+          <Label htmlFor="promptInput">{inputLabel}</Label>
+        </VisuallyHidden>
+        <InputText
+          onKeyDown={onKeyDown}
+          id="promptInput"
+          placeholder={inputLabel}
+          onChange={onChange}
+          width="100%"
+          value={value}
+        />
+      </ModalContent>
+      <ModalFooter secondary={secondary}>
+        <Button
+          disabled={!hasValue}
+          type="submit"
+          onClick={onSubmit}
+          color="primary"
+        >
+          {saveLabel}
+        </Button>
+        <ButtonTransparent type="reset" color={cancelColor} onClick={cancel}>
+          {cancelLabel}
+        </ButtonTransparent>
+      </ModalFooter>
     </Dialog>
   )
 }

packages/playground/src/Dialog/PromptDemo.tsx

@@ -34,9 +34,8 @@ export const PromptDemo: FC = () => {
       title={'Choose a cheese!'}
       inputLabel={'Name of Cheese'}
       saveLabel={'Save'}
-      onCancel={(close: () => void) => {
+      onCancel={() => {
         alert('Prompt closed')
-        close()
       }}
       onSave={(value: string) => alert(`You chose ${value}`)}
       secondary={