Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Checkbox: Add forward ref (BREAKING CHANGE) #1057

Merged
merged 4 commits into from Jul 17, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
54 changes: 54 additions & 0 deletions docs/src/Checkbox.doc.js
Expand Up @@ -74,6 +74,13 @@ a checkbox and its indeterminism are independent.`,
name: 'onClick',
type: `({ event: SyntheticInputEvent<HTMLInputElement>, checked: boolean }) => void`,
},
{
name: 'ref',
type: "React.Ref<'div'>",
AlbertCarreras marked this conversation as resolved.
Show resolved Hide resolved
description:
'Forward the ref to the underlying Box component wrapping the input element',
href: 'refExample',
},
{
name: 'size',
type: `"sm" | "md"`,
Expand Down Expand Up @@ -183,6 +190,53 @@ function CheckboxExample() {
/>
);

card(
<Example
id="refExample"
name="Example: ref"
description={`
A \`Checkbox\` with an anchor ref to a Flyout component
`}
defaultCode={`
function CheckboxFlyoutExample() {
const [open, setOpen] = React.useState(false);
const [checked, setChecked] = React.useState(false);

const anchorRef = React.useRef();

return (
<Box >
<Checkbox
checked={checked}
id="ref"
label="Private Board"
name="privacy"
ref={anchorRef}
onChange={({ checked }) => {
setOpen(true)
setChecked(checked);
}}
/>
{open && (
<Flyout
anchor={anchorRef.current}
idealDirection="up"
onDismiss={() => setOpen(false)}
shouldFocus={false}
>
<Box padding={3}>
<Text weight="bold">Your board is now </Text>
<Text weight="bold">{checked ? 'private': 'public'}</Text>
</Box>
</Flyout>
)}
</Box>
);
}
`}
/>
);

card(
<Combination
id="combinations"
Expand Down
66 changes: 48 additions & 18 deletions packages/gestalt/src/Checkbox.js
Expand Up @@ -16,6 +16,7 @@ type Props = {|
checked?: boolean,
disabled?: boolean,
errorMessage?: string,
forwardedRef?: React.Ref<'input'>,
hasError?: boolean,
id: string,
indeterminate?: boolean,
Expand All @@ -32,26 +33,34 @@ type Props = {|
size?: 'sm' | 'md',
|};

export default function Checkbox({
checked = false,
disabled = false,
errorMessage,
hasError = false,
id,
indeterminate = false,
label,
name,
onChange,
onClick,
size = 'md',
}: Props): React.Node {
const inputElement = React.useRef<?HTMLInputElement>(null);
function Checkbox(props: Props): React.Node {
const {
checked = false,
disabled = false,
errorMessage,
forwardedRef,
hasError = false,
id,
indeterminate = false,
label,
name,
onChange,
onClick,
size = 'md',
} = props;

const innerRef = React.useRef(null);
// When using both forwardedRef and innerRef, React.useimperativehandle() allows a parent component
// that renders <Checkbox ref={inputRef} /> to call inputRef.current.focus()
// $FlowFixMe Flow thinks forwardedRef is a number, which is incorrect
React.useImperativeHandle(forwardedRef, () => innerRef.current);

const [focused, setFocused] = React.useState(false);
const [hovered, setHover] = React.useState(false);

React.useEffect(() => {
if (inputElement && inputElement.current) {
inputElement.current.indeterminate = indeterminate;
if (innerRef && innerRef.current) {
innerRef.current.indeterminate = indeterminate;
}
}, [indeterminate]);

Expand Down Expand Up @@ -104,7 +113,7 @@ export default function Checkbox({
marginRight={-1}
>
<Label htmlFor={id}>
<Box paddingX={1} position="relative">
AlbertCarreras marked this conversation as resolved.
Show resolved Hide resolved

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This position relative is need as the input element is position absolute

<Box paddingX={1}>
<input
AlbertCarreras marked this conversation as resolved.
Show resolved Hide resolved
checked={checked}
className={classnames(controlStyles.input, styleSize, {
Expand All @@ -119,7 +128,7 @@ export default function Checkbox({
onFocus={() => setFocused(true)}
onMouseEnter={handleHover}
onMouseLeave={handleHover}
ref={inputElement}
ref={innerRef}
type="checkbox"
/>
<div
Expand Down Expand Up @@ -174,6 +183,12 @@ Checkbox.propTypes = {
checked: PropTypes.bool,
disabled: PropTypes.bool,
errorMessage: PropTypes.string,
forwardedRef: PropTypes.oneOfType([
PropTypes.func,
PropTypes.shape({
current: PropTypes.any,
}),
]),
hasError: PropTypes.bool,
id: PropTypes.string.isRequired,
indeterminate: PropTypes.bool,
Expand All @@ -183,3 +198,18 @@ Checkbox.propTypes = {
onClick: PropTypes.func,
size: PropTypes.oneOf(['sm', 'md']),
};

function CheckboxWithRef(props, ref) {
return <Checkbox {...props} forwardedRef={ref} />;
}

CheckboxWithRef.displayName = 'ForwardRef(Checkbox)';

const CheckboxWithForwardRef: React$AbstractComponent<
Props,
HTMLInputElement
> = React.forwardRef<Props, HTMLInputElement>(CheckboxWithRef);

CheckboxWithForwardRef.displayName = 'Checkbox';

export default CheckboxWithForwardRef;
60 changes: 47 additions & 13 deletions packages/gestalt/src/Checkbox.jsdom.test.js
Expand Up @@ -3,21 +3,55 @@ import React from 'react';
import { render } from '@testing-library/react';
import Checkbox from './Checkbox.js';

test('Checkbox handles click', () => {
const mockOnClick = jest.fn();
const mockOnChange = jest.fn();
const { getByLabelText } = render(
<form>
<label htmlFor="testcheckbox">Label</label>
const mockOnClick = jest.fn();
const mockOnChange = jest.fn();

describe('Checkbox', () => {
it('Checkbox handles click', () => {
const { getByLabelText } = render(
<form>
<label htmlFor="testcheckbox">Label</label>
<Checkbox
size="sm"
id="testcheckbox"
onChange={mockOnChange}
onClick={mockOnClick}
/>
</form>
);
getByLabelText('Label').click();
expect(mockOnClick).toHaveBeenCalled();
expect(mockOnChange).toHaveBeenCalled();
});

it('forwards a ref to <Box ref={ref}><input/></Box>', () => {
const ref = React.createRef();
render(
<Checkbox checked id="testcheckbox" onChange={mockOnChange} ref={ref} />
);
expect(ref.current instanceof HTMLInputElement).toEqual(true);
expect(ref.current?.checked).toEqual(true);
});

it('sets the innermost input to indeterminate with ref', () => {
const ref = React.createRef();
render(
<Checkbox
size="sm"
indeterminate
id="testcheckbox"
onChange={mockOnChange}
onClick={mockOnClick}
ref={ref}
/>
</form>
);
getByLabelText('Label').click();
expect(mockOnClick).toHaveBeenCalled();
expect(mockOnChange).toHaveBeenCalled();
);
expect(ref.current instanceof HTMLInputElement).toEqual(true);
expect(ref.current?.indeterminate).toEqual(true);
});

it('sets the innermost input to indeterminate without ref', () => {
const { container } = render(
<Checkbox indeterminate id="testcheckbox" onChange={mockOnChange} />
);
const input = container.querySelector('input');
expect(input?.indeterminate).toBe(true);
});
});
16 changes: 8 additions & 8 deletions packages/gestalt/src/__snapshots__/Checkbox.test.js.snap
Expand Up @@ -12,7 +12,7 @@ exports[`Checkbox 1`] = `
htmlFor="id"
>
<div
className="box paddingX1 relative"
className="box paddingX1"
>
<input
checked={false}
Expand Down Expand Up @@ -62,7 +62,7 @@ exports[`Checkbox checked 1`] = `
htmlFor="id"
>
<div
className="box paddingX1 relative"
className="box paddingX1"
>
<input
checked={true}
Expand Down Expand Up @@ -126,7 +126,7 @@ exports[`Checkbox disabled & checked 1`] = `
htmlFor="id"
>
<div
className="box paddingX1 relative"
className="box paddingX1"
>
<input
checked={true}
Expand Down Expand Up @@ -190,7 +190,7 @@ exports[`Checkbox disabled 1`] = `
htmlFor="id"
>
<div
className="box paddingX1 relative"
className="box paddingX1"
>
<input
checked={false}
Expand Down Expand Up @@ -240,7 +240,7 @@ exports[`Checkbox indeterminate 1`] = `
htmlFor="id"
>
<div
className="box paddingX1 relative"
className="box paddingX1"
>
<input
checked={false}
Expand Down Expand Up @@ -304,7 +304,7 @@ exports[`Checkbox small 1`] = `
htmlFor="id"
>
<div
className="box paddingX1 relative"
className="box paddingX1"
>
<input
checked={false}
Expand Down Expand Up @@ -354,7 +354,7 @@ exports[`Checkbox with error 1`] = `
htmlFor="id"
>
<div
className="box paddingX1 relative"
className="box paddingX1"
>
<input
checked={false}
Expand Down Expand Up @@ -425,7 +425,7 @@ exports[`Checkbox without label 1`] = `
htmlFor="id"
>
<div
className="box paddingX1 relative"
className="box paddingX1"
>
<input
checked={false}
Expand Down