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.

Sign up for GitHub

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

Jump to bottom

feat(modal): deprecate iconDescription #9828

Merged
merged 12 commits into from
Oct 21, 2021
Merged

feat(modal): deprecate iconDescription #9828

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
68e114a
feat(modal): deprecate iconDescription new tests
sstrubberg Oct 8, 2021
cc849b3
feat(modal): update test
sstrubberg Oct 9, 2021
f4e2934
feat(modal): updated snappy
sstrubberg Oct 12, 2021
b9c5fb9
Update packages/react/src/components/Modal/Modal-test.js
sstrubberg Oct 18, 2021
289072c
Merge branch 'main' into feat/modal-iconDescription
sstrubberg Oct 18, 2021
3451dea
feat(modal): added test
sstrubberg Oct 18, 2021
b9884a7
feat(modal): updated snappy
sstrubberg Oct 19, 2021
a10f5af
Merge branch 'main' into feat/modal-iconDescription
sstrubberg Oct 19, 2021
a62c989
feat(Modal): aria lable on the close icon
sstrubberg Oct 20, 2021
ee1d8fc
feat(Modal): updated wrapper test
sstrubberg Oct 21, 2021
8199a34
Merge branch 'main' into feat/modal-iconDescription
sstrubberg Oct 21, 2021
67be21f
Merge branch 'main' into feat/modal-iconDescription
sstrubberg Oct 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3804,7 +3804,6 @@ Map {
"Modal" => Object {
"defaultProps": Object {
"hasScrollingContent": false,
"iconDescription": "Close",
"modalHeading": "",
"modalLabel": "",
"onKeyDown": [Function],
Expand Down Expand Up @@ -3834,9 +3833,7 @@ Map {
"hasScrollingContent": Object {
"type": "bool",
},
"iconDescription": Object {
"type": "string",
},
"iconDescription": [Function],
"id": Object {
"type": "string",
},
Expand Down
2 changes: 1 addition & 1 deletion packages/react/src/components/Modal/Modal-story.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ const props = {
'[data-modal-primary-focus]'
),
size: select('Size (size)', sizes, 'md'),
iconDescription: text('Close icon description (iconDescription)', 'Close'),
// iconDescription: text('Close icon description (iconDescription)', 'Close'),
onBlur: action('onBlur'),
onClick: action('onClick'),
onFocus: action('onFocus'),
Expand Down
16 changes: 3 additions & 13 deletions packages/react/src/components/Modal/Modal-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,19 +47,9 @@ describe('Modal', () => {
expect(getModal(modal).props().id).toEqual('modal-1');
});

it('has the expected default iconDescription', () => {
expect(mounted.props().iconDescription).toEqual('Close');
});

it('adds new iconDescription when passed via props', () => {
mounted.setProps({ iconDescription: 'new description' });
expect(mounted.props().iconDescription).toEqual('new description');
});

it('should have iconDescription match Icon component description prop', () => {
const description = mounted.find(Close20).props()['aria-label'];
const matches = mounted.props().iconDescription === description;
expect(matches).toEqual(true);
it('should have aria-hidden on the Icon component', () => {
sstrubberg marked this conversation as resolved.
Show resolved Hide resolved
const ariaHidden = mounted.find(Close20).props()['aria-hidden'];
expect(ariaHidden).toEqual('true');
sstrubberg marked this conversation as resolved.
Show resolved Hide resolved
});

it('enables primary button by default', () => {
Expand Down
12 changes: 7 additions & 5 deletions packages/react/src/components/Modal/Modal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,10 @@ export default class Modal extends Component {
/**
* Provide a description for "close" icon that can be read by screen readers
*/
iconDescription: PropTypes.string,
iconDescription: deprecate(
PropTypes.string,
'The iconDescription prop is no longer needed and can be safely removed. This prop will be removed in the next major release of Carbon.'
),

/**
* Specify the DOM element ID of the top-level node.
Expand Down Expand Up @@ -220,7 +223,6 @@ export default class Modal extends Component {
primaryButtonDisabled: false,
onKeyDown: () => {},
passiveModal: false,
iconDescription: 'Close',
modalHeading: '',
modalLabel: '',
preventCloseOnClickOutside: false,
Expand Down Expand Up @@ -417,11 +419,11 @@ export default class Modal extends Component {
className={this.modalCloseButtonClass}
type="button"
onClick={onRequestClose}
title={iconDescription}
aria-label={iconDescription}
title={ariaLabel ? ariaLabel : iconDescription}
aria-label={ariaLabel ? ariaLabel : iconDescription}
ref={this.button}>
<Close20
aria-label={iconDescription}
aria-hidden="true"
className={`${this.modalCloseButtonClass}__icon`}
/>
</button>
Expand Down
10 changes: 3 additions & 7 deletions packages/react/src/components/ModalWrapper/__snapshots__/ModalWrapper-test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ exports[`ModalWrapper should render 1`] = `
<Modal
aria-label="test modal"
hasScrollingContent={false}
iconDescription="Close"
id="modal"
modalHeading="Transactional Modal"
modalLabel="Test Modal Label"
Expand Down Expand Up @@ -110,18 +109,16 @@ exports[`ModalWrapper should render 1`] = `
Transactional Modal
</h3>
<button
aria-label="Close"
className="bx--modal-close"
onClick={[Function]}
title="Close"
type="button"
>
<ForwardRef(Close20)
aria-label="Close"
aria-hidden="true"
className="bx--modal-close__icon"
>
<Icon
aria-label="Close"
aria-hidden="true"
className="bx--modal-close__icon"
fill="currentColor"
height={20}
Expand All @@ -131,13 +128,12 @@ exports[`ModalWrapper should render 1`] = `
xmlns="http://www.w3.org/2000/svg"
>
<svg
aria-label="Close"
aria-hidden={true}
className="bx--modal-close__icon"
fill="currentColor"
focusable="false"
height={20}
preserveAspectRatio="xMidYMid meet"
role="img"
viewBox="0 0 32 32"
width={20}
xmlns="http://www.w3.org/2000/svg"
Expand Down