From 3e9862cd65563f692ee431dca558747caf4c2157 Mon Sep 17 00:00:00 2001 From: Akira Sudoh Date: Mon, 29 Apr 2019 23:06:01 +0900 Subject: [PATCH] chore(components): better name with forwardRef (#2281) This change updates React component name for various forward-ref'ed components from just `` to something like ``. Refs #1768. --- src/components/Button/Button.js | 176 +++++++------- src/components/Checkbox/Checkbox.js | 104 ++++---- .../__snapshots__/DataTable-test.js.snap | 24 +- .../TableBatchAction-test.js.snap | 4 +- .../TableBatchActions-test.js.snap | 8 +- .../__snapshots__/ModalWrapper-test.js.snap | 12 +- src/components/Select/Select.js | 224 +++++++++--------- src/components/TextInput/TextInput.js | 206 ++++++++-------- src/components/UIShell/HeaderMenuItem.js | 33 +-- src/components/UIShell/Link.js | 2 +- src/components/UIShell/SideNavMenuItem.js | 2 +- src/components/UIShell/SideNavSwitcher.js | 2 +- .../__snapshots__/HeaderMenu-test.js.snap | 12 +- .../__snapshots__/HeaderMenuItem-test.js.snap | 4 +- .../SideNavMenuItem-test.js.snap | 12 +- .../SideNavSwitcher-test.js.snap | 4 +- 16 files changed, 411 insertions(+), 418 deletions(-) diff --git a/src/components/Button/Button.js b/src/components/Button/Button.js index b8a880021c..2334c3e4a5 100644 --- a/src/components/Button/Button.js +++ b/src/components/Button/Button.js @@ -18,99 +18,97 @@ const { prefix } = settings; let didWarnAboutDeprecation = false; -const Button = React.forwardRef( - ( - { - children, - as, - className, - disabled, - small, - kind, - href, - tabIndex, - type, - inputref, - renderIcon, - icon, - iconDescription, - ...other - }, - ref - ) => { - const buttonClasses = classNames(className, { - [`${prefix}--btn`]: true, - [`${prefix}--btn--sm`]: small, - [`${prefix}--btn--primary`]: kind === 'primary', - [`${prefix}--btn--danger`]: kind === 'danger', - [`${prefix}--btn--secondary`]: kind === 'secondary', - [`${prefix}--btn--ghost`]: kind === 'ghost', - [`${prefix}--btn--danger--primary`]: kind === 'danger--primary', - [`${prefix}--btn--tertiary`]: kind === 'tertiary', - [`${prefix}--btn--disabled`]: disabled, - }); - - const commonProps = { - tabIndex, - className: buttonClasses, - ref: breakingChangesX ? ref : ref || inputref, - }; - - if (__DEV__ && breakingChangesX && icon) { - warning( - didWarnAboutDeprecation, - 'The `icon` property in the `Button` component is being removed in the next release of ' + - '`carbon-components-react`. Please use `renderIcon` instead.' - ); - didWarnAboutDeprecation = true; - } - - const hasRenderIcon = Object(renderIcon) === renderIcon; - const ButtonImageElement = hasRenderIcon - ? renderIcon - : !breakingChangesX && icon && Icon; - const buttonImage = !ButtonImageElement ? null : ( - +const Button = React.forwardRef(function Button( + { + children, + as, + className, + disabled, + small, + kind, + href, + tabIndex, + type, + inputref, + renderIcon, + icon, + iconDescription, + ...other + }, + ref +) { + const buttonClasses = classNames(className, { + [`${prefix}--btn`]: true, + [`${prefix}--btn--sm`]: small, + [`${prefix}--btn--primary`]: kind === 'primary', + [`${prefix}--btn--danger`]: kind === 'danger', + [`${prefix}--btn--secondary`]: kind === 'secondary', + [`${prefix}--btn--ghost`]: kind === 'ghost', + [`${prefix}--btn--danger--primary`]: kind === 'danger--primary', + [`${prefix}--btn--tertiary`]: kind === 'tertiary', + [`${prefix}--btn--disabled`]: disabled, + }); + + const commonProps = { + tabIndex, + className: buttonClasses, + ref: breakingChangesX ? ref : ref || inputref, + }; + + if (__DEV__ && breakingChangesX && icon) { + warning( + didWarnAboutDeprecation, + 'The `icon` property in the `Button` component is being removed in the next release of ' + + '`carbon-components-react`. Please use `renderIcon` instead.' ); + didWarnAboutDeprecation = true; + } - let component = 'button'; - let otherProps = { - disabled, - type, + const hasRenderIcon = Object(renderIcon) === renderIcon; + const ButtonImageElement = hasRenderIcon + ? renderIcon + : !breakingChangesX && icon && Icon; + const buttonImage = !ButtonImageElement ? null : ( + + ); + + let component = 'button'; + let otherProps = { + disabled, + type, + }; + const anchorProps = { + role: 'button', + href, + }; + if (as) { + component = as; + otherProps = { + ...otherProps, + ...anchorProps, }; - const anchorProps = { - role: 'button', - href, - }; - if (as) { - component = as; - otherProps = { - ...otherProps, - ...anchorProps, - }; - } else if (href) { - component = 'a'; - otherProps = anchorProps; - } - return React.createElement( - component, - { - ...other, - ...commonProps, - ...otherProps, - }, - children, - buttonImage - ); + } else if (href) { + component = 'a'; + otherProps = anchorProps; } -); + return React.createElement( + component, + { + ...other, + ...commonProps, + ...otherProps, + }, + children, + buttonImage + ); +}); Button.propTypes = { /** diff --git a/src/components/Checkbox/Checkbox.js b/src/components/Checkbox/Checkbox.js index ee31262cad..f3379c24cd 100644 --- a/src/components/Checkbox/Checkbox.js +++ b/src/components/Checkbox/Checkbox.js @@ -12,59 +12,57 @@ import { settings } from 'carbon-components'; const { prefix } = settings; -const Checkbox = React.forwardRef( - ( - { - className, - id, - labelText, - onChange, - indeterminate, - hideLabel, - wrapperClassName, - title = '', - ...other - }, - ref - ) => { - const labelClasses = classNames(`${prefix}--checkbox-label`, className); - const innerLabelClasses = classNames({ - [`${prefix}--visually-hidden`]: hideLabel, - }); - const wrapperClasses = classNames( - `${prefix}--form-item`, - `${prefix}--checkbox-wrapper`, - wrapperClassName - ); - - return ( -
- { - onChange(evt.target.checked, id, evt); - }} - className={`${prefix}--checkbox`} - id={id} - ref={el => { - if (el) { - el.indeterminate = indeterminate; - } - if (typeof ref === 'function') { - ref(el); - } else if (Object(ref) === ref) { - ref.current = el; - } - }} - /> - -
- ); - } -); +const Checkbox = React.forwardRef(function Checkbox( + { + className, + id, + labelText, + onChange, + indeterminate, + hideLabel, + wrapperClassName, + title = '', + ...other + }, + ref +) { + const labelClasses = classNames(`${prefix}--checkbox-label`, className); + const innerLabelClasses = classNames({ + [`${prefix}--visually-hidden`]: hideLabel, + }); + const wrapperClasses = classNames( + `${prefix}--form-item`, + `${prefix}--checkbox-wrapper`, + wrapperClassName + ); + + return ( +
+ { + onChange(evt.target.checked, id, evt); + }} + className={`${prefix}--checkbox`} + id={id} + ref={el => { + if (el) { + el.indeterminate = indeterminate; + } + if (typeof ref === 'function') { + ref(el); + } else if (Object(ref) === ref) { + ref.current = el; + } + }} + /> + +
+ ); +}); Checkbox.propTypes = { /** diff --git a/src/components/DataTable/__tests__/__snapshots__/DataTable-test.js.snap b/src/components/DataTable/__tests__/__snapshots__/DataTable-test.js.snap index f23d1d9a83..228da4beef 100644 --- a/src/components/DataTable/__tests__/__snapshots__/DataTable-test.js.snap +++ b/src/components/DataTable/__tests__/__snapshots__/DataTable-test.js.snap @@ -1867,7 +1867,7 @@ exports[`DataTable should render 1`] = ` Action 3 - Add new - + - - + - - + - - + - Cancel - +
- Add new - +
diff --git a/src/components/DataTable/__tests__/__snapshots__/TableBatchAction-test.js.snap b/src/components/DataTable/__tests__/__snapshots__/TableBatchAction-test.js.snap index 4c7da1c035..5d02f5d416 100644 --- a/src/components/DataTable/__tests__/__snapshots__/TableBatchAction-test.js.snap +++ b/src/components/DataTable/__tests__/__snapshots__/TableBatchAction-test.js.snap @@ -32,7 +32,7 @@ exports[`DataTable.TableBatchAction should render 1`] = ` } } > - - + `; diff --git a/src/components/DataTable/__tests__/__snapshots__/TableBatchActions-test.js.snap b/src/components/DataTable/__tests__/__snapshots__/TableBatchActions-test.js.snap index 77ea17c76f..fd3f06215f 100644 --- a/src/components/DataTable/__tests__/__snapshots__/TableBatchActions-test.js.snap +++ b/src/components/DataTable/__tests__/__snapshots__/TableBatchActions-test.js.snap @@ -15,7 +15,7 @@ exports[`DataTable.TableBatchActions should render 1`] = `
- Cancel - +
- Cancel - +
- Test Modal - + - Cancel - - + Save - +
diff --git a/src/components/Select/Select.js b/src/components/Select/Select.js index 429401514d..41b764ca59 100644 --- a/src/components/Select/Select.js +++ b/src/components/Select/Select.js @@ -17,123 +17,121 @@ import WarningFilled16 from '@carbon/icons-react/lib/warning--filled/16'; const { prefix } = settings; -const Select = React.forwardRef( - ( - { - className, - id, - inline, - labelText, - disabled, - children, - noLabel, // reserved for use with component - iconDescription, - hideLabel, - invalid, - invalidText, - helperText, - light, - ...other - }, - ref - ) => { - const selectClasses = classNames({ - [`${prefix}--select`]: true, - [`${prefix}--select--inline`]: inline, - [`${prefix}--select--light`]: light, - [`${prefix}--select--invalid`]: invalid, - [className]: className, - }); - const labelClasses = classNames(`${prefix}--label`, { - [`${prefix}--visually-hidden`]: hideLabel, - [`${prefix}--label--disabled`]: disabled, - }); - const errorId = `${id}-error-msg`; - const error = invalid ? ( -
- {invalidText} -
- ) : null; - const helperTextClasses = classNames(`${prefix}--form__helper-text`, { - [`${prefix}--form__helper-text--disabled`]: disabled, - }); - const helper = helperText ? ( -
{helperText}
- ) : null; - const ariaProps = {}; - if (invalid) { - ariaProps['aria-describedby'] = errorId; - } - const input = (() => { - return ( - <> - - {componentsX ? ( - - {iconDescription} - - ) : ( - - )} - {componentsX && invalid && ( - - )} - - ); - })(); +const Select = React.forwardRef(function Select( + { + className, + id, + inline, + labelText, + disabled, + children, + noLabel, // reserved for use with component + iconDescription, + hideLabel, + invalid, + invalidText, + helperText, + light, + ...other + }, + ref +) { + const selectClasses = classNames({ + [`${prefix}--select`]: true, + [`${prefix}--select--inline`]: inline, + [`${prefix}--select--light`]: light, + [`${prefix}--select--invalid`]: invalid, + [className]: className, + }); + const labelClasses = classNames(`${prefix}--label`, { + [`${prefix}--visually-hidden`]: hideLabel, + [`${prefix}--label--disabled`]: disabled, + }); + const errorId = `${id}-error-msg`; + const error = invalid ? ( +
+ {invalidText} +
+ ) : null; + const helperTextClasses = classNames(`${prefix}--form__helper-text`, { + [`${prefix}--form__helper-text--disabled`]: disabled, + }); + const helper = helperText ? ( +
{helperText}
+ ) : null; + const ariaProps = {}; + if (invalid) { + ariaProps['aria-describedby'] = errorId; + } + const input = (() => { return ( -
-
- {!noLabel && ( - - )} - {!inline && helper} - {componentsX && inline && ( - <> -
-
- {input} -
- {error} + <> + + {componentsX ? ( + + {iconDescription} + + ) : ( + + )} + {componentsX && invalid && ( + + )} + + ); + })(); + return ( +
+
+ {!noLabel && ( + + )} + {!inline && helper} + {componentsX && inline && ( + <> +
+
+ {input}
- {helper} - - )} - {componentsX && !inline && ( -
- {input} + {error}
- )} - {!componentsX && input} - {!componentsX && inline && helper} - {(!componentsX || (componentsX && !inline)) && error} -
+ {helper} + + )} + {componentsX && !inline && ( +
+ {input} +
+ )} + {!componentsX && input} + {!componentsX && inline && helper} + {(!componentsX || (componentsX && !inline)) && error}
- ); - } -); +
+ ); +}); Select.propTypes = { /** diff --git a/src/components/TextInput/TextInput.js b/src/components/TextInput/TextInput.js index 7ac0ddb906..482e6cbbb8 100644 --- a/src/components/TextInput/TextInput.js +++ b/src/components/TextInput/TextInput.js @@ -14,111 +14,109 @@ import WarningFilled16 from '@carbon/icons-react/lib/warning--filled/16'; const { prefix } = settings; -const TextInput = React.forwardRef( - ( - { - labelText, - className = `${prefix}--text__input`, - id, - placeholder, - type, - onChange, - onClick, - hideLabel, - invalid, - invalidText, - helperText, - light, - ...other +const TextInput = React.forwardRef(function TextInput( + { + labelText, + className = `${prefix}--text__input`, + id, + placeholder, + type, + onChange, + onClick, + hideLabel, + invalid, + invalidText, + helperText, + light, + ...other + }, + ref +) { + const textInputProps = { + id, + onChange: evt => { + if (!other.disabled) { + onChange(evt); + } }, - ref - ) => { - const textInputProps = { - id, - onChange: evt => { - if (!other.disabled) { - onChange(evt); - } - }, - onClick: evt => { - if (!other.disabled) { - onClick(evt); - } - }, - placeholder, - type, - ref, - }; - - const errorId = id + '-error-msg'; - const textInputClasses = classNames(`${prefix}--text-input`, className, { - [`${prefix}--text-input--light`]: light, - [`${prefix}--text-input--invalid`]: invalid, - }); - const labelClasses = classNames(`${prefix}--label`, { - [`${prefix}--visually-hidden`]: hideLabel, - [`${prefix}--label--disabled`]: other.disabled, - }); - const helperTextClasses = classNames(`${prefix}--form__helper-text`, { - [`${prefix}--form__helper-text--disabled`]: other.disabled, - }); - - const label = labelText ? ( - - ) : null; - - const error = invalid ? ( -
- {invalidText} -
- ) : null; - - const input = invalid ? ( - - ) : ( - - ); - - const helper = helperText ? ( -
{helperText}
- ) : null; - - const textInputWrapperClasses = classNames(`${prefix}--form-item`, { - [`${prefix}--text-input-wrapper`]: componentsX, - }); - - return ( -
- {label} - {helper} - {componentsX ? ( -
- {invalid && ( - - )} - {input} -
- ) : ( - input - )} - {error} -
- ); - } -); + onClick: evt => { + if (!other.disabled) { + onClick(evt); + } + }, + placeholder, + type, + ref, + }; + + const errorId = id + '-error-msg'; + const textInputClasses = classNames(`${prefix}--text-input`, className, { + [`${prefix}--text-input--light`]: light, + [`${prefix}--text-input--invalid`]: invalid, + }); + const labelClasses = classNames(`${prefix}--label`, { + [`${prefix}--visually-hidden`]: hideLabel, + [`${prefix}--label--disabled`]: other.disabled, + }); + const helperTextClasses = classNames(`${prefix}--form__helper-text`, { + [`${prefix}--form__helper-text--disabled`]: other.disabled, + }); + + const label = labelText ? ( + + ) : null; + + const error = invalid ? ( +
+ {invalidText} +
+ ) : null; + + const input = invalid ? ( + + ) : ( + + ); + + const helper = helperText ? ( +
{helperText}
+ ) : null; + + const textInputWrapperClasses = classNames(`${prefix}--form-item`, { + [`${prefix}--text-input-wrapper`]: componentsX, + }); + + return ( +
+ {label} + {helper} + {componentsX ? ( +
+ {invalid && ( + + )} + {input} +
+ ) : ( + input + )} + {error} +
+ ); +}); TextInput.propTypes = { /** diff --git a/src/components/UIShell/HeaderMenuItem.js b/src/components/UIShell/HeaderMenuItem.js index 06f101e428..0eec75e10e 100644 --- a/src/components/UIShell/HeaderMenuItem.js +++ b/src/components/UIShell/HeaderMenuItem.js @@ -12,22 +12,23 @@ import Link, { LinkPropTypes } from './Link'; const { prefix } = settings; -const HeaderMenuItem = React.forwardRef( - ({ className, children, role, ...rest }, ref) => { - return ( -
  • - - {children} - -
  • - ); - } -); +const HeaderMenuItem = React.forwardRef(function HeaderMenuItem( + { className, children, role, ...rest }, + ref +) { + return ( +
  • + + {children} + +
  • + ); +}); HeaderMenuItem.propTypes = { /** diff --git a/src/components/UIShell/Link.js b/src/components/UIShell/Link.js index 69b4bab0fa..e862ca7601 100644 --- a/src/components/UIShell/Link.js +++ b/src/components/UIShell/Link.js @@ -14,7 +14,7 @@ import React from 'react'; * in their own components to support use-cases like `react-router` or * `@reach/router` */ -const Link = React.forwardRef((props, ref) => { +const Link = React.forwardRef(function Link(props, ref) { const { element, ...rest } = props; return React.createElement(element, { ...rest, ref }); }); diff --git a/src/components/UIShell/SideNavMenuItem.js b/src/components/UIShell/SideNavMenuItem.js index 26340306cf..2ad279363c 100644 --- a/src/components/UIShell/SideNavMenuItem.js +++ b/src/components/UIShell/SideNavMenuItem.js @@ -14,7 +14,7 @@ import Link from './Link'; const { prefix } = settings; -const SideNavMenuItem = React.forwardRef((props, ref) => { +const SideNavMenuItem = React.forwardRef(function SideNavMenuItem(props, ref) { const { children, className: customClassName, isActive, ...rest } = props; const className = cx(`${prefix}--side-nav__menu-item`, customClassName); const linkClassName = cx({ diff --git a/src/components/UIShell/SideNavSwitcher.js b/src/components/UIShell/SideNavSwitcher.js index f73d54036d..2feb6cea58 100644 --- a/src/components/UIShell/SideNavSwitcher.js +++ b/src/components/UIShell/SideNavSwitcher.js @@ -13,7 +13,7 @@ import React from 'react'; const { prefix } = settings; -const SideNavSwitcher = React.forwardRef((props, ref) => { +const SideNavSwitcher = React.forwardRef(function SideNavSwitcher(props, ref) { const { className: customClassName, labelText, onChange, options } = props; const className = cx(`${prefix}--side-nav__switcher`, customClassName); // Note for usage around `onBlur`: https://github.com/evcohen/eslint-plugin-jsx-a11y/blob/master/docs/rules/no-onchange.md diff --git a/src/components/UIShell/__tests__/__snapshots__/HeaderMenu-test.js.snap b/src/components/UIShell/__tests__/__snapshots__/HeaderMenu-test.js.snap index 47e3dc5f8e..0f78b66a24 100644 --- a/src/components/UIShell/__tests__/__snapshots__/HeaderMenu-test.js.snap +++ b/src/components/UIShell/__tests__/__snapshots__/HeaderMenu-test.js.snap @@ -56,7 +56,7 @@ exports[`HeaderMenu should render 1`] = ` className="bx--header__menu" role="menu" > - - - + - - + - + diff --git a/src/components/UIShell/__tests__/__snapshots__/HeaderMenuItem-test.js.snap b/src/components/UIShell/__tests__/__snapshots__/HeaderMenuItem-test.js.snap index a7f587fd80..a417de4195 100644 --- a/src/components/UIShell/__tests__/__snapshots__/HeaderMenuItem-test.js.snap +++ b/src/components/UIShell/__tests__/__snapshots__/HeaderMenuItem-test.js.snap @@ -1,7 +1,7 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`HeaderMenuItem should render 1`] = ` - @@ -28,5 +28,5 @@ exports[`HeaderMenuItem should render 1`] = ` - + `; diff --git a/src/components/UIShell/__tests__/__snapshots__/SideNavMenuItem-test.js.snap b/src/components/UIShell/__tests__/__snapshots__/SideNavMenuItem-test.js.snap index ced3cf27cc..ace0d43c96 100644 --- a/src/components/UIShell/__tests__/__snapshots__/SideNavMenuItem-test.js.snap +++ b/src/components/UIShell/__tests__/__snapshots__/SideNavMenuItem-test.js.snap @@ -1,7 +1,7 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`SideNavMenuItem should render 1`] = ` - - + `; exports[`SideNavMenuItem should render active styles with \`isActive\` or \`aria-current="page"\` 1`] = ` - - + `; exports[`SideNavMenuItem should render active styles with \`isActive\` or \`aria-current="page"\` 2`] = ` - - + `; diff --git a/src/components/UIShell/__tests__/__snapshots__/SideNavSwitcher-test.js.snap b/src/components/UIShell/__tests__/__snapshots__/SideNavSwitcher-test.js.snap index b38f77b48b..551006c10b 100644 --- a/src/components/UIShell/__tests__/__snapshots__/SideNavSwitcher-test.js.snap +++ b/src/components/UIShell/__tests__/__snapshots__/SideNavSwitcher-test.js.snap @@ -1,7 +1,7 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`SideNavSwitcher should render 1`] = ` -
    - + `;