From 35e38e6448ca4b8f7325f80b3417b9320040221b Mon Sep 17 00:00:00 2001 From: Supreeth Date: Wed, 23 Aug 2023 15:57:12 +0530 Subject: [PATCH] [terra-form-radio] Fixes issue of `onChange` not getting triggered on Radio buttons (#3868) Co-authored-by: SM051274 --- packages/terra-core-docs/CHANGELOG.md | 3 +- .../example/field/ControlledRadioField.jsx | 2 + packages/terra-form-radio/CHANGELOG.md | 3 + packages/terra-form-radio/src/Radio.jsx | 14 +- packages/terra-form-radio/src/RadioField.jsx | 58 ++-- packages/terra-form-radio/src/_RadioUtil.js | 24 +- .../tests/jest/Radio.test.jsx | 20 +- .../tests/jest/RadioField.test.jsx | 39 ++- .../__snapshots__/RadioField.test.jsx.snap | 300 +++++++++++++++++- 9 files changed, 421 insertions(+), 42 deletions(-) diff --git a/packages/terra-core-docs/CHANGELOG.md b/packages/terra-core-docs/CHANGELOG.md index a99b50a99a5..de30fa3bd63 100644 --- a/packages/terra-core-docs/CHANGELOG.md +++ b/packages/terra-core-docs/CHANGELOG.md @@ -18,7 +18,8 @@ * Changed * Updated `iconAll` test to accommodate new icons added from OCS icon library v1.51.0. * Updated default search delay to 2500ms. - * Update Search field examples to be more functionality focused. + * Update Search field examples to be more functionality focused. + * Updated `terra-form-radio-field` example to display selected value. ## 1.36.0 - (August 11, 2023) diff --git a/packages/terra-core-docs/src/terra-dev-site/doc/form-radio/example/field/ControlledRadioField.jsx b/packages/terra-core-docs/src/terra-dev-site/doc/form-radio/example/field/ControlledRadioField.jsx index 432761aa596..81ecace0687 100644 --- a/packages/terra-core-docs/src/terra-dev-site/doc/form-radio/example/field/ControlledRadioField.jsx +++ b/packages/terra-core-docs/src/terra-dev-site/doc/form-radio/example/field/ControlledRadioField.jsx @@ -59,6 +59,8 @@ export default class extends React.Component { + Selected day: + {this.state.selectedAnswer}
diff --git a/packages/terra-form-radio/CHANGELOG.md b/packages/terra-form-radio/CHANGELOG.md index ccec64a3b4a..43100c605cf 100644 --- a/packages/terra-form-radio/CHANGELOG.md +++ b/packages/terra-form-radio/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased +* Fixed + * Fixed issue of `onChange` not triggered on first and last item of radio field while keyboard navigation. + ## 4.39.0 - (August 11, 2023) * Changed diff --git a/packages/terra-form-radio/src/Radio.jsx b/packages/terra-form-radio/src/Radio.jsx index 529809047c2..4fb52777770 100644 --- a/packages/terra-form-radio/src/Radio.jsx +++ b/packages/terra-form-radio/src/Radio.jsx @@ -4,7 +4,7 @@ import classNames from 'classnames'; import classNamesBind from 'classnames/bind'; import ThemeContext from 'terra-theme-context'; import styles from './Radio.module.scss'; -import RadioUtil from './_RadioUtil'; +import { isConsideredMobileDevice } from './_RadioUtil'; const cx = classNamesBind.bind(styles); @@ -52,16 +52,16 @@ const propTypes = { */ name: PropTypes.string, /** - * Function to trigger when focus is lost from the radio button. - */ + * Function to trigger when focus is lost from the radio button. + */ onBlur: PropTypes.func, /** * Function to trigger when user clicks on the radio button. Provide a function to create a controlled input. */ onChange: PropTypes.func, /** - * Function to trigger when user focuses on the radio button. - */ + * Function to trigger when user focuses on the radio button. + */ onFocus: PropTypes.func, /** * The value of the input element. @@ -125,7 +125,7 @@ const Radio = ({ 'label', { 'is-disabled': disabled }, { 'is-hidden': isLabelHidden }, - { 'is-mobile': RadioUtil.isConsideredMobileDevice() }, + { 'is-mobile': isConsideredMobileDevice() }, labelTextAttrs.className, ]); @@ -140,7 +140,7 @@ const Radio = ({ const outerRingClasses = cx([ 'outer-ring', - { 'is-mobile': RadioUtil.isConsideredMobileDevice() }, + { 'is-mobile': isConsideredMobileDevice() }, ]); const innerRingClasses = cx([ diff --git a/packages/terra-form-radio/src/RadioField.jsx b/packages/terra-form-radio/src/RadioField.jsx index 79c761931b6..bbb8b0d8c26 100644 --- a/packages/terra-form-radio/src/RadioField.jsx +++ b/packages/terra-form-radio/src/RadioField.jsx @@ -10,6 +10,7 @@ import VisualyHiddenText from 'terra-visually-hidden-text'; import { VALUE_UP, VALUE_DOWN, VALUE_RIGHT, VALUE_LEFT, } from 'keycode-js'; +import { findFirstFocusableItem, findLastFocusableItem } from './_RadioUtil'; import styles from './RadioField.module.scss'; const cx = classNamesBind.bind(styles); @@ -113,7 +114,7 @@ const RadioField = (props) => { legendAttrs.className, ]); - const fieldSetId = `terra-radio-group-${uniqueid()}`; + const fieldSetId = customProps.id || `terra-radio-group-${uniqueid()}`; const legendAriaDescriptionId = `terra-radio-field-description-${uniqueid()}`; const helpAriaDescriptionId = help ? `terra-radio-field-description-help-${uniqueid()}` : ''; const errorAriaDescriptionId = error ? `terra-radio-field-description-error-${uniqueid()}` : ''; @@ -144,34 +145,51 @@ const RadioField = (props) => { ); - const handleKeyDown = (event) => { + /* + * Note: Cyclic Navigation of Radio button is not supported in Safari browser hence adding keydown event handler to support cyclic navigation. + * this handler will use native mouse event to set focus back to first radio button when we press down or right arrow key on last radio button and vise versa. + */ + const handleKeyDown = (event, radio) => { const radioGroup = document.getElementById(fieldSetId); - const radioItems = radioGroup.querySelectorAll('[type=radio]'); - const itemIndex = Array.from(radioItems).indexOf(event.currentTarget); - if (event.key === VALUE_DOWN || event.key === VALUE_RIGHT) { - if (itemIndex === radioItems.length - 1) { - radioItems[0].focus(); - radioItems[0].checked = true; - } else { - radioItems[itemIndex + 1].focus(); - radioItems[itemIndex + 1].checked = true; - } - } else if (event.key === VALUE_UP || event.key === VALUE_LEFT) { - if (itemIndex === 0) { - radioItems[radioItems.length - 1].focus(); - radioItems[radioItems.length - 1].checked = true; - } else { - radioItems[itemIndex - 1].focus(); - radioItems[itemIndex - 1].checked = true; + if (radioGroup) { + const radioItems = radioGroup.querySelectorAll('[type=radio]'); + const itemIndex = Array.from(radioItems).indexOf(event.currentTarget); + const onClick = new MouseEvent('click', { bubbles: true, cancelable: false }); + const firstItemIndex = findFirstFocusableItem(radioItems); + const lastItemIndex = findLastFocusableItem(radioItems); + + if (event.nativeEvent.key === VALUE_DOWN || event.nativeEvent.key === VALUE_RIGHT) { + if (itemIndex === lastItemIndex) { + radioItems[firstItemIndex].dispatchEvent(onClick); + } + } else if (event.nativeEvent.key === VALUE_UP || event.nativeEvent.key === VALUE_LEFT) { + if (itemIndex === firstItemIndex) { + radioItems[lastItemIndex].dispatchEvent(onClick); + } } } + if (radio && radio.props.onKeyDown) { + radio.props.onKeyDown(); + } + }; + + /* + * Focus gets lost when radio button's are selected via mouse in Safari browser. + * This set focus back on the radio button on mouse click + */ + const handleClick = (event, radio) => { + event?.currentTarget?.focus(); + if (radio && radio.props.onClick) { + radio.props.onClick(); + } }; const content = React.Children.map(children, (child) => { if (child && child.type.isRadio) { + const eventHandlersForSafari = (isSafari) ? { onKeyDown: (event) => handleKeyDown(event, child), onClick: (event) => handleClick(event, child) } : undefined; return React.cloneElement(child, { inputAttrs: { - ...child.props.inputAttrs, 'aria-describedby': ariaDescriptionIds, onKeyDown: handleKeyDown, + ...child.props.inputAttrs, 'aria-describedby': ariaDescriptionIds, ...eventHandlersForSafari, }, }); } diff --git a/packages/terra-form-radio/src/_RadioUtil.js b/packages/terra-form-radio/src/_RadioUtil.js index 4fa06d816c9..785b9a1a017 100644 --- a/packages/terra-form-radio/src/_RadioUtil.js +++ b/packages/terra-form-radio/src/_RadioUtil.js @@ -8,6 +8,28 @@ const isConsideredMobileDevice = () => window.matchMedia('(max-width: 1024px)'). || navigator.msMaxTouchPoints > 0 ); -export default { +const findLastFocusableItem = (radioBtns) => { + let itemIndex = radioBtns.length - 1; + + while (radioBtns[itemIndex] && radioBtns[itemIndex].hasAttribute('disabled') && itemIndex > -1) { + itemIndex -= 1; + } + + return (itemIndex) || undefined; +}; + +const findFirstFocusableItem = (radioBtns) => { + let itemIndex = 0; + + while (radioBtns[itemIndex] && radioBtns[itemIndex].hasAttribute('disabled') && itemIndex < radioBtns.length) { + itemIndex += 1; + } + + return (itemIndex < radioBtns.length) ? itemIndex : undefined; +}; + +export { isConsideredMobileDevice, + findLastFocusableItem, + findFirstFocusableItem, }; diff --git a/packages/terra-form-radio/tests/jest/Radio.test.jsx b/packages/terra-form-radio/tests/jest/Radio.test.jsx index ade2196e982..848d7ea2f54 100644 --- a/packages/terra-form-radio/tests/jest/Radio.test.jsx +++ b/packages/terra-form-radio/tests/jest/Radio.test.jsx @@ -6,32 +6,32 @@ import Radio from '../../src/Radio'; window.matchMedia = () => ({ matches: true }); it('should render a radio', () => { - const checkBox = (); - const wrapper = shallow(checkBox); + const radioButton = (); + const wrapper = shallow(radioButton); expect(wrapper).toMatchSnapshot(); }); it('should render an uncontrolled radio', () => { - const checkBox = (); - const wrapper = shallow(checkBox); + const radioButton = (); + const wrapper = shallow(radioButton); expect(wrapper).toMatchSnapshot(); }); it('should render a controlled radio', () => { - const checkBox = ( {}} labelText="Radio" />); - const wrapper = shallow(checkBox); + const radioButton = ( {}} labelText="Radio" />); + const wrapper = shallow(radioButton); expect(wrapper).toMatchSnapshot(); }); it('should render a disabled radio', () => { - const checkBox = ( {}} labelText="Radio" disabled />); - const wrapper = shallow(checkBox); + const radioButton = ( {}} labelText="Radio" disabled />); + const wrapper = shallow(radioButton); expect(wrapper).toMatchSnapshot(); }); it('should render a radio with a hidden label', () => { - const checkBox = ( {}} labelText="Radio" isLabelHidden />); - const wrapper = shallow(checkBox); + const radioButton = ( {}} labelText="Radio" isLabelHidden />); + const wrapper = shallow(radioButton); expect(wrapper).toMatchSnapshot(); }); diff --git a/packages/terra-form-radio/tests/jest/RadioField.test.jsx b/packages/terra-form-radio/tests/jest/RadioField.test.jsx index a646bfcfecb..c68f73699e3 100644 --- a/packages/terra-form-radio/tests/jest/RadioField.test.jsx +++ b/packages/terra-form-radio/tests/jest/RadioField.test.jsx @@ -10,11 +10,15 @@ window.matchMedia = () => ({ matches: true }); jest.mock('lodash.uniqueid'); let userAgentGetter; -beforeAll(() => { +beforeEach(() => { userAgentGetter = jest.spyOn(window.navigator, 'userAgent', 'get'); uniqueid.mockReturnValue('uuid123'); }); +afterEach(() => { + jest.restoreAllMocks(); +}); + it('should render a default radio field', () => { const radioField = ; const wrapper = shallowWithIntl(radioField); @@ -67,6 +71,39 @@ it('should render a help message', () => { expect(wrapper).toMatchSnapshot(); }); +it('should render onkeydown and onclick event on radio button for safari browser', () => { + userAgentGetter.mockReturnValue('Safari'); + const radioField = ( + + + + ); + const wrapper = mountWithIntl(radioField); + expect(wrapper.find('input').prop('onKeyDown')).toBeDefined(); + expect(wrapper.find('input').prop('onClick')).toBeDefined(); + expect(wrapper).toMatchSnapshot(); +}); + +it('should not render onkeydown and onclick event on radio button for non-safari browser', () => { + const radioField = ( + + + + ); + const wrapper = mountWithIntl(radioField); + expect(wrapper.find('input').prop('onKeyDown')).toBeUndefined(); + expect(wrapper.find('input').prop('onClick')).toBeUndefined(); + expect(wrapper).toMatchSnapshot(); +}); + it('should render an optional part on the label', () => { const radioField = ; const wrapper = shallowWithIntl(radioField); diff --git a/packages/terra-form-radio/tests/jest/__snapshots__/RadioField.test.jsx.snap b/packages/terra-form-radio/tests/jest/__snapshots__/RadioField.test.jsx.snap index 689a6c85128..e7774265f2a 100644 --- a/packages/terra-form-radio/tests/jest/__snapshots__/RadioField.test.jsx.snap +++ b/packages/terra-form-radio/tests/jest/__snapshots__/RadioField.test.jsx.snap @@ -94,7 +94,6 @@ exports[`correctly applies "inputAttrs" property to the Radio component 1`] = ` Object { "aria-describedby": "terra-radio-field-description-uuid123 ", "data-custom-attr": "attr data", - "onKeyDown": [Function], } } isInline={false} @@ -116,7 +115,6 @@ exports[`correctly applies "inputAttrs" property to the Radio component 1`] = ` data-custom-attr="attr data" disabled={false} name={null} - onKeyDown={[Function]} type="radio" /> `; +exports[`should not render onkeydown and onclick event on radio button for non-safari browser 1`] = ` + + +
+ +
+ Help RadioField + +
+ + +
+ +
+ +
+ This will help up determine how many chairs to request +
+
+ + +`; + exports[`should render a default radio field 1`] = ` `; +exports[`should render onkeydown and onclick event on radio button for safari browser 1`] = ` + + +
+
+
+ Help RadioField + +
+
+ +
+ +
+ +
+ This will help up determine how many chairs to request +
+
+ + +`; + exports[`should render radio field with div element for Safari browser or Edg browser 1`] = `