Skip to content

Commit

Permalink
fix(FormCheck): fix Form.Check.Label spacing (#5983)
Browse files Browse the repository at this point in the history
* Fix `Form.Check.Label` spacing

Currently, when using a `Form.Check.Label` component to customize
`Form.Check` rendering, there will be no space between the checkbox and
the label. This is because `Form.Check` is currently relying on the
presence of a `label` prop to apply the `form-check` class name to the
wrapping `<div>`, because checkboxes without labels [don't need the
wrapping element to have the `form-check` class name][1].

This commit adds a utility to check whether an element has a child of a
certain type. It then uses that utility to check if a `Form.Check`
element has a `Form.Check.Label` child and takes that into account when
determining whether the checkbox has a label.

Adding a special property (currently called `typeName`, but that can
certainly change) to components for this utility is necessary because
React minifies the `displayName` property in production.

[1]: #5938 (comment)

* Add error handling to `includesType`

* Support mixing auto and custom child components

Currently, `Form.Check` doesn't allow users to use a custom
`Form.Check.Input` with a label generated by the `label` prop (and
requires that a custom `Form.Check.Input` be used if a custom
`Form.Check.Label` is also used) because the presence of _any_ children
in `Form.Check` prevents automatic inputs and labels (and feedback) from
generating. This approach allows mixing and matching by extracting any
custom input, label, and feedback components from `Form.Check`'s
`children` prop, then separately for each, using the custom component if
provided or an automatic component otherwise.

* Simply check `child.type`

The `type` property of React elements have reference equality with
matching imported component variables (`JSXElementConstructor` objects,
to be precise)! For some reason I was not expecting that, but it makes
sense to me now, since these constructor objects are being imported from
a singular source, so there's no reason React would create multiple
instances of them. So `child.type === type` all we need to check in
`getChildOfType`; the `typeName` prop I added earlier is not at all
needed.

* Add scripts for testing production build of docs

Just as a convenience. I can revert this commit if these scripts aren't
desired.

* Just check immediate children for `FormCheckLabel`

* Use correct variable

* Revert empty `.husky/pre-commit` change

No idea how that got into 0754b49.

* Revert "Add scripts for testing production build of docs"

This reverts commit 5343b0a.
  • Loading branch information
TyMick committed Nov 29, 2021
1 parent b3f880a commit 250655c
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 3 deletions.
15 changes: 14 additions & 1 deletion src/ElementChildren.tsx
Expand Up @@ -35,4 +35,17 @@ function forEach<P = any>(
});
}

export { map, forEach };
/**
* Finds whether a component's `children` prop includes a React element of the
* specified type.
*/
function hasChildOfType<P = any>(
children: React.ReactNode,
type: string | React.JSXElementConstructor<P>,
): boolean {
return React.Children.toArray(children).some(
(child) => React.isValidElement(child) && child.type === type,
);
}

export { map, forEach, hasChildOfType };
7 changes: 5 additions & 2 deletions src/FormCheck.tsx
Expand Up @@ -8,6 +8,7 @@ import FormCheckLabel from './FormCheckLabel';
import FormContext from './FormContext';
import { useBootstrapPrefix } from './ThemeProvider';
import { BsPrefixProps, BsPrefixRefForwardingComponent } from './helpers';
import { hasChildOfType } from './ElementChildren';

export type FormCheckType = 'checkbox' | 'radio' | 'switch';

Expand Down Expand Up @@ -152,7 +153,9 @@ const FormCheck: BsPrefixRefForwardingComponent<'input', FormCheckProps> =
[controlId, id],
);

const hasLabel = label != null && label !== false && !children;
const hasLabel =
(!children && label != null && label !== false) ||
hasChildOfType(children, FormCheckLabel);

const input = (
<FormCheckInput
Expand All @@ -172,7 +175,7 @@ const FormCheck: BsPrefixRefForwardingComponent<'input', FormCheckProps> =
style={style}
className={classNames(
className,
label && bsPrefix,
hasLabel && bsPrefix,
inline && `${bsPrefix}-inline`,
type === 'switch' && bsSwitchPrefix,
)}
Expand Down

0 comments on commit 250655c

Please sign in to comment.