Remove readOnly from Radio

Author: Luke Bowerman

CHANGELOG.md

@@ -36,12 +36,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- `Radio` only sends `onChange` when state actually changes
 - `Field*` components no longer have a bottom margin by default (see `Form` update)
 - `Form` now acts as `SpaceVertical` component automatically putting a gap between each child item
 - `InputText` will now show red error icon when validation fails
 
 ### Removed
 
+- `Radio` no longer supports `readOnly` (was nice to know you, albeit briefly)
 - `FieldInline` no longer supports `labelFontWeight`
 
 ## [0.7.27] - 2020-04-15

packages/components/src/Form/Fields/FieldRadio/__snapshots__/FieldRadio.test.tsx.snap

@@ -120,15 +120,11 @@ exports[`A FieldRadio 1`] = `
     >
       <input
         aria-describedby="FieldRadioID-describedby"
-        checked={false}
         id="FieldRadioID"
         name="thumbsUp"
-        onChange={[Function]}
-        onClick={[Function]}
         type="radio"
       />
       <div
-        checked={false}
         className="c5 c6"
       />
     </div>
@@ -260,15 +256,12 @@ exports[`A FieldRadio checked 1`] = `
     >
       <input
         aria-describedby="FieldRadioID-describedby"
-        checked={false}
+        checked={true}
         id="FieldRadioID"
         name="thumbsUp"
-        onChange={[Function]}
-        onClick={[Function]}
         type="radio"
       />
       <div
-        checked={false}
         className="c5 c6"
       />
     </div>

packages/components/src/Form/Inputs/Radio/FauxRadio.tsx

@@ -29,11 +29,7 @@ import { reset } from '@looker/design-tokens'
 
 const dotSize = 6
 
-interface FauxRadioProps {
-  checked: boolean
-}
-
-export const FauxRadio = styled.div<FauxRadioProps>`
+export const FauxRadio = styled.div`
   ${reset}
   position: relative;
   width: 100%;

packages/components/src/Form/Inputs/Radio/Radio.test.tsx

@@ -28,7 +28,7 @@ import 'jest-styled-components'
 import React from 'react'
 import { assertSnapshot, renderWithTheme } from '@looker/components-test-utils'
 import { fireEvent } from '@testing-library/react'
-import { Radio, RadioProps } from './Radio'
+import { Radio } from './Radio'
 
 test('Radio default', () => {
   assertSnapshot(<Radio id="radioID" />)
@@ -56,7 +56,6 @@ test('Radio with aria-describedby', () => {
 
 test('Radio should trigger onChange handler', () => {
   const onChange = jest.fn()
-
   const { getByRole } = renderWithTheme(
     <Radio id="radioID" onChange={onChange} />
   )
@@ -65,17 +64,3 @@ test('Radio should trigger onChange handler', () => {
   fireEvent.click(radioInput)
   expect(onChange).toHaveBeenCalledTimes(1)
 })
-
-test("Radio readOnly doesn't register onChange events", () => {
-  const mockProps: RadioProps = {
-    onChange: jest.fn(),
-  }
-
-  const { getByRole } = renderWithTheme(
-    <Radio readOnly id="checkboxID" {...mockProps} />
-  )
-
-  const checkboxInput = getByRole('radio')
-  fireEvent.click(checkboxInput)
-  expect(mockProps.onChange).toHaveBeenCalledTimes(0)
-})

packages/components/src/Form/Inputs/Radio/Radio.tsx

@@ -24,62 +24,28 @@
 
  */
 
-import noop from 'lodash/noop'
 import pick from 'lodash/pick'
-import isUndefined from 'lodash/isUndefined'
-import React, { FormEvent, forwardRef, Ref, useEffect, useState } from 'react'
+import React, { forwardRef, Ref } from 'react'
 import styled from 'styled-components'
 import { reset, space, SpaceProps } from '@looker/design-tokens'
 import { InputProps, inputPropKeys } from '../InputProps'
 import { FauxRadio } from './FauxRadio'
 
 export interface RadioProps
   extends SpaceProps,
-    Omit<InputProps, 'type' | 'checked' | 'onClick'> {
+    Omit<InputProps, 'readonly' | 'type' | 'checked' | 'onClick'> {
   checked?: boolean
   className?: string
 }
 
 const RadioLayout = forwardRef(
   (props: RadioProps, ref: Ref<HTMLInputElement>) => {
-    const {
-      className,
-      checked,
-      defaultChecked,
-      onChange,
-      readOnly,
-      ...restProps
-    } = props
-    const [isChecked, setIsChecked] = useState(!!defaultChecked)
-
-    const handleClick = readOnly
-      ? undefined
-      : (event: FormEvent<HTMLInputElement>) => {
-          setIsChecked(true)
-          if (onChange) {
-            onChange(event)
-          }
-        }
-
-    // controlled component: update internal state when props.checked changes
-    useEffect(() => {
-      if (!isUndefined(checked)) {
-        setIsChecked(checked)
-      }
-    }, [checked])
+    const { className, ...restProps } = props
 
     return (
       <div className={className}>
-        <input
-          type="radio"
-          {...pick(restProps, inputPropKeys)}
-          checked={isChecked}
-          onChange={noop}
-          // suppress read-only error as we rely on click rather than change event here
-          onClick={handleClick}
-          ref={ref}
-        />
-        <FauxRadio checked={isChecked} />
+        <input type="radio" {...pick(restProps, inputPropKeys)} ref={ref} />
+        <FauxRadio />
       </div>
     )
   }

packages/components/src/Form/Inputs/Radio/__snapshots__/Radio.test.tsx.snap

@@ -75,12 +75,9 @@ exports[`Radio checked set to false 1`] = `
   <input
     checked={false}
     id="radioID"
-    onChange={[Function]}
-    onClick={[Function]}
     type="radio"
   />
   <div
-    checked={false}
     className="c1 c2"
   />
 </div>
@@ -159,14 +156,11 @@ exports[`Radio checked set to true 1`] = `
   className="c0"
 >
   <input
-    checked={false}
+    checked={true}
     id="radioID"
-    onChange={[Function]}
-    onClick={[Function]}
     type="radio"
   />
   <div
-    checked={false}
     className="c1 c2"
   />
 </div>
@@ -245,14 +239,10 @@ exports[`Radio default 1`] = `
   className="c0"
 >
   <input
-    checked={false}
     id="radioID"
-    onChange={[Function]}
-    onClick={[Function]}
     type="radio"
   />
   <div
-    checked={false}
     className="c1 c2"
   />
 </div>
@@ -359,15 +349,11 @@ exports[`Radio should accept disabled 1`] = `
   className="c0"
 >
   <input
-    checked={false}
     disabled={true}
     id="radioID"
-    onChange={[Function]}
-    onClick={[Function]}
     type="radio"
   />
   <div
-    checked={false}
     className="c1 c2"
   />
 </div>
@@ -474,14 +460,10 @@ exports[`Radio with aria-describedby 1`] = `
 >
   <input
     aria-describedby="some-id"
-    checked={false}
     id="radioID"
-    onChange={[Function]}
-    onClick={[Function]}
     type="radio"
   />
   <div
-    checked={false}
     className="c1 c2"
   />
 </div>
@@ -560,15 +542,11 @@ exports[`Radio with name and id 1`] = `
   className="c0"
 >
   <input
-    checked={false}
     id="Chuck"
     name="Chuck"
-    onChange={[Function]}
-    onClick={[Function]}
     type="radio"
   />
   <div
-    checked={false}
     className="c1 c2"
   />
 </div>

packages/www/src/documentation/components/forms/radio.mdx

@@ -40,18 +40,3 @@ A name and ID can be specified in the `<Radio />` component. Radio buttons of th
   <Radio name="someName" id="three" value="three" />
 </Space>
 ```
-
-## ReadOnly property
-
-`Radio` will ignore user-actions when the `readOnly` property is enabled, similar to `disabled` but without greying out the option.
-
-NOTE: In HTML specifying `readonly` on _only_ prevents changes to `value` attribute of the checkbox and therefore allowing the user to still check and uncheck the checkbox.
-
-In most cases we emulate the HTML specification where it has an existing decision or feature. However, the specification's behavior around checkboxes with a `readonly` attribute is rather unintuitive.
-
-```jsx
-<Space>
-  <Radio readOnly />
-  <Radio readOnly checked />
-</Space>
-```