diff --git a/.changeset/tonic-ui-927.md b/.changeset/tonic-ui-927.md new file mode 100644 index 0000000000..322dc8b9b1 --- /dev/null +++ b/.changeset/tonic-ui-927.md @@ -0,0 +1,5 @@ +--- +"@tonic-ui/react": minor +--- + +feat: improve `disabled` prop handling in `Button` and `ButtonGroup` diff --git a/package.json b/package.json index 8d97fd7a37..b73a1b56e5 100644 --- a/package.json +++ b/package.json @@ -7,6 +7,7 @@ ], "scripts": { "build": "lerna exec -- yarn build", + "build-public": "lerna exec --no-private -- yarn build", "clean": "lerna exec --parallel -- yarn clean && lerna clean --yes", "changeset": "changeset", "ci-publish": "yarn lerna-publish-from-package --yes", diff --git a/packages/react-docs/components/Sidebar.js b/packages/react-docs/components/Sidebar.js index 27c8e416b1..1fafadcaf6 100644 --- a/packages/react-docs/components/Sidebar.js +++ b/packages/react-docs/components/Sidebar.js @@ -208,7 +208,7 @@ const Sidebar = forwardRef(( if (heading) { return ( ${title}`} color={colorStyle?.color?.tertiary} fontSize="xs" lineHeight="xs" @@ -231,7 +231,7 @@ const Sidebar = forwardRef(( return ( ') })}`} isActive={isActive} diff --git a/packages/react-docs/components/TableOfContents.js b/packages/react-docs/components/TableOfContents.js index 7858978978..ce878ecaea 100644 --- a/packages/react-docs/components/TableOfContents.js +++ b/packages/react-docs/components/TableOfContents.js @@ -55,7 +55,6 @@ const TableOfContents = (props) => { if (mainContent) { // Use the `:scope` pseudo-class to select all direct child heading elements of the main content, excluding h1 const headingSelectors = ['h2', 'h3', 'h4', 'h5', 'h6'].map(h => `:scope>${h}`).join(','); - console.log(headingSelectors); setNodes(Array.from(mainContent.querySelectorAll(headingSelectors))); } diff --git a/packages/react-docs/pages/components/button-group/index.page.mdx b/packages/react-docs/pages/components/button-group/index.page.mdx index d13ebe7a9d..fcafd45aea 100644 --- a/packages/react-docs/pages/components/button-group/index.page.mdx +++ b/packages/react-docs/pages/components/button-group/index.page.mdx @@ -47,6 +47,7 @@ The following example shows different states (i.e. normal, disabled, and selecte | Name | Type | Default | Description | | :--- | :--- | :------ | :---------- | | children | ReactNode \| `(context) => ReactNode` | | A function child can be used intead of a React element. This function is called with the context object. | +| disabled | boolean | | If `true`, all buttons will be disabled. | | orientation | string | 'horizontal' | The orientation of the button group. One of: 'horizontal', 'vertical' | | size | string | 'md' | The size of the button group. One of: 'sm', 'md', 'lg' | | variant | string | 'default' | The variant of the button group. One of: 'emphasis', 'primary', 'default', 'secondary', 'ghost' | diff --git a/packages/react-docs/pages/components/checkbox-group/index.page.mdx b/packages/react-docs/pages/components/checkbox-group/index.page.mdx index 52e293da7d..15001deb19 100644 --- a/packages/react-docs/pages/components/checkbox-group/index.page.mdx +++ b/packages/react-docs/pages/components/checkbox-group/index.page.mdx @@ -52,7 +52,7 @@ Use the `size` prop to change the size of the `CheckboxGroup`. You can set the v | :--- | :--- | :------ | :---------- | | children | ReactNode \| `(context) => ReactNode` | | A function child can be used intead of a React element. This function is called with the context object. | | defaultValue | (string\|number)[] | | The initial value of the checkbox group. | -| disabled | boolean | false | If `true`, all checkboxes will be disabled. | +| disabled | boolean | | If `true`, all checkboxes will be disabled. | | name | string | | The name used to reference the value of the control. If you don't provide this prop, it falls back to a randomly generated name. | | onChange | (event) => void | | A callback fired when any descendant `Checkbox` is checked or unchecked. | | size | string | 'md' | The size (width and height) of the checkbox. One of: 'sm', 'md', 'lg' | diff --git a/packages/react-docs/pages/components/radio-group/index.page.mdx b/packages/react-docs/pages/components/radio-group/index.page.mdx index a339bab527..6014b75478 100644 --- a/packages/react-docs/pages/components/radio-group/index.page.mdx +++ b/packages/react-docs/pages/components/radio-group/index.page.mdx @@ -52,7 +52,7 @@ Use the `size` prop to change the size of the `RadioGroup`. You can set the valu | :--- | :--- | :------ | :---------- | | children | ReactNode \| `(context) => ReactNode` | | A function child can be used intead of a React element. This function is called with the context object. | | defaultValue | string \| number | | The default `input` element value. Use when the component is not controlled. | -| disabled | boolean | false | If `true`, all radios will be disabled. | +| disabled | boolean | | If `true`, all radios will be disabled. | | name | string | | The name used to reference the value of the control. If you don't provide this prop, it falls back to a randomly generated name. | | onChange | function | | A callback called when the state of the radio changes. | | size | string | 'md' | The size (width and height) of the radio. One of: 'sm', 'md', 'lg' | diff --git a/packages/react/src/button/Button.js b/packages/react/src/button/Button.js index 9d1278d41f..1dd2bd2b34 100644 --- a/packages/react/src/button/Button.js +++ b/packages/react/src/button/Button.js @@ -11,28 +11,31 @@ const defaultOrientation = 'horizontal'; const Button = forwardRef((inProps, ref) => { const { - disabled, + disabled: disabledProp, selected, size: sizeProp, variant: variantProp, ...rest } = useDefaultProps({ props: inProps, name: 'Button' }); - let orientation; // orientation for ButtonGroup + let disabled = disabledProp; + let orientation; // No default value assigned; orientation is determined by `ButtonGroup` let size = sizeProp; let variant = variantProp; const buttonGroupContext = useButtonGroup(); if (buttonGroupContext) { const { + disabled: buttonGroupDisabled, + orientation: buttonGroupOrientation, size: buttonGroupSize, variant: buttonGroupVariant, - orientation: buttonGroupOrientation, } = { ...buttonGroupContext }; - // Use fallback values if values are null or undefined + disabled = (disabled ?? buttonGroupDisabled); + orientation = buttonGroupOrientation ?? defaultOrientation; + // Use the default value if the value is null or undefined size = (size ?? buttonGroupSize) ?? defaultSize; variant = (variant ?? buttonGroupVariant) ?? defaultVariant; - orientation = buttonGroupOrientation ?? defaultOrientation; } else { // Use the default value if the value is null or undefined size = size ?? defaultSize; @@ -56,7 +59,7 @@ const Button = forwardRef((inProps, ref) => { }; const styleProps = useButtonStyle({ - orientation, // No default value if not specified + orientation, // No default value if not used within `ButtonGroup` size, variant, }); diff --git a/packages/react/src/button/ButtonGroup.js b/packages/react/src/button/ButtonGroup.js index 3889942da3..927461aca6 100644 --- a/packages/react/src/button/ButtonGroup.js +++ b/packages/react/src/button/ButtonGroup.js @@ -15,13 +15,19 @@ const defaultVariant = 'default'; const ButtonGroup = forwardRef((inProps, ref) => { const { children, + disabled, orientation = defaultOrientation, size = defaultSize, variant = defaultVariant, ...rest } = useDefaultProps({ props: inProps, name: 'ButtonGroup' }); const styleProps = useButtonGroupStyle({ orientation }); - const context = getMemoizedState({ orientation, size, variant }); + const context = getMemoizedState({ + disabled, + orientation, + size, + variant, + }); return ( diff --git a/packages/react/src/checkbox/Checkbox.js b/packages/react/src/checkbox/Checkbox.js index ab3bfe1624..3b9c24287f 100644 --- a/packages/react/src/checkbox/Checkbox.js +++ b/packages/react/src/checkbox/Checkbox.js @@ -41,6 +41,7 @@ const Checkbox = forwardRef((inProps, ref) => { const inputRef = useRef(); const combinedInputRef = useMergeRefs(inputRefProp, inputRef); const checkboxGroupContext = useCheckboxGroup(); + const isNameConflictRef = useRef(false); if (checkboxGroupContext) { const { @@ -54,8 +55,19 @@ const Checkbox = forwardRef((inProps, ref) => { if (checkboxGroupValue !== undefined) { checked = ensureArray(checkboxGroupValue).includes(value); } - disabled = checkboxGroupDisabled || disabled; - name = checkboxGroupName ?? name; + disabled = (disabled ?? checkboxGroupDisabled); + + const isNameConflict = (!isNullish(name) && !isNullish(checkboxGroupName) && (name !== checkboxGroupName)); + if (process.env.NODE_ENV !== 'production' && isNameConflict && !isNameConflict.current) { + // Log the warning message only once + console.error( + `Warning: The \`Checkbox\` has a \`name\` prop ("${name}") that conflicts with the \`CheckboxGroup\`'s \`name\` prop ("${checkboxGroupName}")` + ); + isNameConflictRef.current = true; + } + + name = name ?? checkboxGroupName; + onChange = callAll( onChange, checkboxGroupOnChange, diff --git a/packages/react/src/checkbox/__tests__/Checkbox.test.js b/packages/react/src/checkbox/__tests__/Checkbox.test.js index be13dcceb5..8070a89b7a 100644 --- a/packages/react/src/checkbox/__tests__/Checkbox.test.js +++ b/packages/react/src/checkbox/__tests__/Checkbox.test.js @@ -1,6 +1,6 @@ import { testA11y } from '@tonic-ui/react/test-utils/accessibility'; import { render } from '@tonic-ui/react/test-utils/render'; -import { Checkbox } from '@tonic-ui/react/src'; +import { Checkbox, CheckboxGroup } from '@tonic-ui/react/src'; import React, { useEffect, useRef } from 'react'; describe('Checkbox', () => { @@ -53,4 +53,24 @@ describe('Checkbox', () => { ); }); + + it('should output a warning message when the Checkbox\'s name prop conflicts with the CheckboxGroup\'s name prop', () => { + // Mock console.error + const consoleErrorMock = jest.spyOn(console, 'error').mockImplementation(() => {}); + + const checkboxGroupName = 'checkbox-group'; + const checkboxName = 'checkbox'; + + render( + + + + ); + + const expectedErrorMessage = `Warning: The \`Checkbox\` has a \`name\` prop ("${checkboxName}") that conflicts with the \`CheckboxGroup\`'s \`name\` prop ("${checkboxGroupName}")`; + expect(consoleErrorMock).toHaveBeenLastCalledWith(expectedErrorMessage); + + // Restore the original console.error + consoleErrorMock.mockRestore(); + }); }); diff --git a/packages/react/src/radio/Radio.js b/packages/react/src/radio/Radio.js index fec721800c..216dbe6049 100644 --- a/packages/react/src/radio/Radio.js +++ b/packages/react/src/radio/Radio.js @@ -39,6 +39,7 @@ const Radio = forwardRef((inProps, ref) => { const inputRef = useRef(); const combinedInputRef = useMergeRefs(inputRefProp, inputRef); const radioGroupContext = useRadioGroup(); + const isNameConflictRef = useRef(false); if (radioGroupContext) { const { @@ -53,8 +54,18 @@ const Radio = forwardRef((inProps, ref) => { if (radioGroupValue !== undefined) { checked = (radioGroupValue === value); } - disabled = radioGroupDisabled || disabled; - name = radioGroupName ?? name; + disabled = (disabled ?? radioGroupDisabled); + + const isNameConflict = (!isNullish(name) && !isNullish(radioGroupName) && (name !== radioGroupName)); + if (process.env.NODE_ENV !== 'production' && isNameConflict && !isNameConflict.current) { + // Log the warning message only once + console.error( + `Warning: The \`Radio\` has a \`name\` prop ("${name}") that conflicts with the \`RadioGroup\`'s \`name\` prop ("${radioGroupName}")` + ); + isNameConflictRef.current = true; + } + name = (name ?? radioGroupName); + onChange = callAll( onChange, radioGroupOnChange, diff --git a/packages/react/src/radio/__tests__/Radio.test.js b/packages/react/src/radio/__tests__/Radio.test.js index 38a9b10590..cafc02f694 100644 --- a/packages/react/src/radio/__tests__/Radio.test.js +++ b/packages/react/src/radio/__tests__/Radio.test.js @@ -1,6 +1,6 @@ import { testA11y } from '@tonic-ui/react/test-utils/accessibility'; import { render } from '@tonic-ui/react/test-utils/render'; -import { Radio } from '@tonic-ui/react/src'; +import { Radio, RadioGroup } from '@tonic-ui/react/src'; import React, { useEffect, useRef } from 'react'; describe('Radio', () => { @@ -51,4 +51,24 @@ describe('Radio', () => { ); }); + + it('should output a warning message when the Radio\'s name prop conflicts with the RadioGroup\'s name prop', () => { + // Mock console.error + const consoleErrorMock = jest.spyOn(console, 'error').mockImplementation(() => {}); + + const radioGroupName = 'radio-group'; + const radioName = 'radio'; + + render( + + + + ); + + const expectedErrorMessage = `Warning: The \`Radio\` has a \`name\` prop ("${radioName}") that conflicts with the \`RadioGroup\`'s \`name\` prop ("${radioGroupName}")`; + expect(consoleErrorMock).toHaveBeenLastCalledWith(expectedErrorMessage); + + // Restore the original console.error + consoleErrorMock.mockRestore(); + }); });