feat(components): support automatic focus sentinel #5260
There are no files selected for viewing
Binary file not shown.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,11 +37,6 @@ function Container({ story }) { | |
| }}> | ||
| {story()} | ||
| </div> | ||
| </React.StrictMode> | ||
| ); | ||
| } | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,10 +10,13 @@ import React, { Component } from 'react'; | |
| import classNames from 'classnames'; | ||
| import { settings } from 'carbon-components'; | ||
| import { Close20 } from '@carbon/icons-react'; | ||
| import toggleClass from '../../tools/toggleClass'; | ||
| import Button from '../Button'; | ||
| import deprecate from '../../prop-types/deprecate'; | ||
| import requiredIfGivenPropExists from '../../prop-types/requiredIfGivenPropExists'; | ||
| import wrapFocus, { | ||
| elementOrParentIsFloatingMenu, | ||
| } from '../../internal/wrapFocus'; | ||
| import setupGetInstanceId from '../../tools/setupGetInstanceId'; | ||
|
|
||
| const { prefix } = settings; | ||
|
|
@@ -138,10 +141,13 @@ export default class Modal extends Component { | |
| size: PropTypes.oneOf(['xs', 'sm', 'lg']), | ||
|
|
||
| /** | ||
| * Deprecated; Used for advanced focus-wrapping feature using 3rd party library, | ||
| * but it's now achieved without a 3rd party library. | ||
| */ | ||
| focusTrap: deprecate( | ||
| PropTypes.bool, | ||
| `\nThe prop \`focusTrap\` for Modal has been deprecated, as the feature of \`persistent\` runs by default.` | ||
| ), | ||
|
|
||
| /** | ||
| * Specify whether the modal contains scrolling content | ||
|
|
@@ -167,30 +173,18 @@ export default class Modal extends Component { | |
| modalHeading: '', | ||
| modalLabel: '', | ||
| selectorPrimaryFocus: '[data-modal-primary-focus]', | ||
| hasScrollingContent: false, | ||
| }; | ||
|
|
||
| button = React.createRef(); | ||
| outerModal = React.createRef(); | ||
| innerModal = React.createRef(); | ||
| startSentinel = React.createRef(); | ||
|
There was a problem hiding this comment. Is there a clearer plain-language way we can name this as opposed to "sentinel". It's difficult jargon to even Google a simple definition of. I assume you're meaning like a sentinel node, but there's probably a friendlier way to describe that. There was a problem hiding this comment. @dakahn Hope this link shed some light: https://developers.google.com/web/fundamentals/accessibility/focus/using-tabindex There was a problem hiding this comment. I understand the concept of sentinel elements in a focus-trap, but my question was if we could name these in plainer and simpler to understand language? If not that's cool too, but maybe a quick comment explaining the concept of sentinels to somebody who -- like myself -- has implemented wrapping focus but was totally unfamiliar with this term.
There was a problem hiding this comment. Maybe "start trap" and "end trap"? |
||
| endSentinel = React.createRef(); | ||
| modalInstanceId = `modal-${getInstanceId()}`; | ||
| modalLabelId = `${prefix}--modal-header__label--${this.modalInstanceId}`; | ||
| modalHeadingId = `${prefix}--modal-header__heading--${this.modalInstanceId}`; | ||
|
|
||
| handleKeyDown = evt => { | ||
| if (this.props.open) { | ||
| if (evt.which === 27) { | ||
|
|
@@ -206,28 +200,32 @@ export default class Modal extends Component { | |
| if ( | ||
| this.innerModal.current && | ||
| !this.innerModal.current.contains(evt.target) && | ||
| !elementOrParentIsFloatingMenu( | ||
| evt.target, | ||
| this.props.selectorsFloatingMenus | ||
| ) | ||
| ) { | ||
| this.props.onRequestClose(evt); | ||
| } | ||
| }; | ||
|
|
||
| handleBlur = ({ | ||
| target: oldActiveNode, | ||
| relatedTarget: currentActiveNode, | ||
| }) => { | ||
| const { open, selectorsFloatingMenus } = this.props; | ||
| if (open && currentActiveNode && oldActiveNode) { | ||
| const { current: modalNode } = this.innerModal; | ||
| const { current: startSentinelNode } = this.startSentinel; | ||
| const { current: endSentinelNode } = this.endSentinel; | ||
| wrapFocus({ | ||
| modalNode, | ||
| startSentinelNode, | ||
| endSentinelNode, | ||
| currentActiveNode, | ||
| oldActiveNode, | ||
| selectorsFloatingMenus, | ||
| }); | ||
| } | ||
| }; | ||
|
|
||
|
|
@@ -277,9 +275,7 @@ export default class Modal extends Component { | |
| if (!this.props.open) { | ||
| return; | ||
| } | ||
| this.focusButton(this.innerModal.current); | ||
| } | ||
|
|
||
| handleTransitionEnd = evt => { | ||
|
|
@@ -290,9 +286,7 @@ export default class Modal extends Component { | |
| this.outerModal.current.offsetHeight && | ||
| this.beingOpen | ||
| ) { | ||
| this.focusButton(evt.currentTarget); | ||
| this.beingOpen = false; | ||
| } | ||
| }; | ||
|
|
@@ -317,7 +311,6 @@ export default class Modal extends Component { | |
| selectorsFloatingMenus, // eslint-disable-line | ||
| shouldSubmitOnEnter, // eslint-disable-line | ||
| size, | ||
| hasScrollingContent, | ||
| ...other | ||
| } = this.props; | ||
|
|
@@ -379,7 +372,8 @@ export default class Modal extends Component { | |
| role="dialog" | ||
| className={containerClasses} | ||
| aria-label={ariaLabel} | ||
| aria-modal="true" | ||
| tabIndex="-1"> | ||
| <div className={`${prefix}--modal-header`}> | ||
| {passiveModal && modalButton} | ||
| {modalLabel && ( | ||
|
|
@@ -422,30 +416,34 @@ export default class Modal extends Component { | |
| </div> | ||
| ); | ||
|
|
||
| return ( | ||
| <div | ||
| {...other} | ||
| onKeyDown={this.handleKeyDown} | ||
| onMouseDown={this.handleMousedown} | ||
| onBlur={this.handleBlur} | ||
| className={modalClasses} | ||
| role="presentation" | ||
| onTransitionEnd={this.props.open ? this.handleTransitionEnd : undefined} | ||
| ref={this.outerModal}> | ||
| {/* Non-translatable: Focus-wrap code makes this `<span>` not actually read by screen readers */} | ||
| <span | ||
| ref={this.startSentinel} | ||
| tabIndex="0" | ||
| role="link" | ||
| className={`${prefix}--visually-hidden`}> | ||
| Focus sentinel | ||
| </span> | ||
| {modalBody} | ||
| {/* Non-translatable: Focus-wrap code makes this `<span>` not actually read by screen readers */} | ||
| <span | ||
| ref={this.endSentinel} | ||
| tabIndex="0" | ||
| role="link" | ||
| className={`${prefix}--visually-hidden`}> | ||
| Focus sentinel | ||
| </span> | ||
| </div> | ||
| ); | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we're deprecating this prop, are we not going to give the user the ability to choose whether or not they want the modal focus-wrapped?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for asking @abbeyhrt - Focus-wrapping behavior runs regardless of
focusTrapproperty, even before it was introduced or after it was introduced. Some time ago the codebase introduced a third-partyfocus-trap-reactlibrary for better focus-wrapping behavior, but it introduced lots of side effects as the PR description of this PR links to.focusTrapproperty was introduced to disable such third-party library to avoid the side effects. This PR removes the property because the third-party is no longer needed for focus-wrapping.