Deal with scenario where Dialog only has isOpen specified (#1557)

Author: Luke Bowerman

CHANGELOG.md

@@ -5,6 +5,10 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [0.9.18] - 2020-10-12
+
+- `useDialog` needs to support scenario it is controlled but `onClose` isn't specified
+
 ## [0.9.17] - 2020-10-12
 
 ## Added

packages/components/src/Dialog/Dialog.test.tsx

@@ -25,14 +25,15 @@
  */
 
 import 'jest-styled-components'
-import React from 'react'
+import React, { useState } from 'react'
 import { renderWithTheme } from '@looker/components-test-utils'
 import {
   screen,
   fireEvent,
   waitForElementToBeRemoved,
 } from '@testing-library/react'
 import { SimpleContent } from '../__mocks__/DialogContentSimple'
+import { DialogMediumContent } from '../__mocks__/DialogMediumContent'
 import { Dialog } from './Dialog'
 import { DialogManager } from './DialogManager'
 import {
@@ -159,9 +160,6 @@ describe('Dialog', () => {
     await waitForElementToBeRemoved(() => screen.getByText('Dialog content'))
   })
 
-  /**
-   * NOTE: All other tests assume a "uncontrolled" version of Dialog.
-   */
   test('Controlled', async () => {
     renderWithTheme(<Controlled />)
 
@@ -176,6 +174,26 @@ describe('Dialog', () => {
     await waitForElementToBeRemoved(() => screen.getByText(/We the People/))
   })
 
+  test('Controlled no callbacks', async () => {
+    const SimpleControlled = () => {
+      const [isOpen, setOpen] = useState(false)
+
+      return (
+        <>
+          <Dialog content={<DialogMediumContent />} isOpen={isOpen} />
+          <button onClick={() => setOpen(true)}>Open Dialog</button>
+        </>
+      )
+    }
+
+    renderWithTheme(<SimpleControlled />)
+
+    // Open Dialog
+    const link = screen.getByText('Open Dialog')
+    fireEvent.click(link)
+    expect(screen.queryByText(/We the People/)).toBeInTheDocument()
+  })
+
   test('Controlled - no children', async () => {
     renderWithTheme(<ControlledNoChildren />)
 

packages/components/src/Dialog/useDialog.tsx

@@ -173,9 +173,9 @@ export const useDialog = ({
   /**
    * LEGACY SUPPORT
    * Eventually we need to deprecate support for `isOpen` without specifying a `setOpen`
-   * being explicitly so we can unwind this semi-controlled state.
+   *  explicitly so we can unwind this semi-controlled state.
    */
-  const isPartiallyControlled = !!onClose && controlledIsOpen !== undefined
+  const isPartiallyControlled = controlledIsOpen !== undefined
 
   const isOpen =
     isPartiallyControlled || isControlled