-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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.
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
Components: Shift focus into popover when opened #2323
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
2f3b99a
Utils: Add focusable search utilities
aduth d259b2b
Components: Shift focus into popover when opened
aduth 974e2d5
Inserter: Leverage Popover isOpen to focus search
aduth 14ddec7
Components: Use ally.js for first tabbable selection
aduth 8c86b96
Utils: Implement focusable utilities by selector
aduth 76875ee
Components: Use wrapper as fallback for popover focus
aduth File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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 |
---|---|---|
@@ -0,0 +1,93 @@ | ||
/** | ||
* External dependencies | ||
*/ | ||
import 'element-closest'; | ||
|
||
/** | ||
* References: | ||
* | ||
* Focusable: | ||
* - https://www.w3.org/TR/html5/editing.html#focus-management | ||
* | ||
* Sequential focus navigation: | ||
* - https://www.w3.org/TR/html5/editing.html#sequential-focus-navigation-and-the-tabindex-attribute | ||
* | ||
* Disabled elements: | ||
* - https://www.w3.org/TR/html5/disabled-elements.html#disabled-elements | ||
* | ||
* getClientRects algorithm (requiring layout box): | ||
* - https://www.w3.org/TR/cssom-view-1/#extension-to-the-element-interface | ||
* | ||
* AREA elements associated with an IMG: | ||
* - https://w3c.github.io/html/editing.html#data-model | ||
*/ | ||
|
||
const SELECTOR = [ | ||
'[tabindex]', | ||
'a[href]', | ||
'button:not([disabled])', | ||
'input:not([type="hidden"]):not([disabled])', | ||
'select:not([disabled])', | ||
'textarea:not([disabled])', | ||
'iframe', | ||
'object', | ||
'embed', | ||
'area[href]', | ||
'[contenteditable]', | ||
].join( ',' ); | ||
|
||
/** | ||
* Returns true if the specified element is visible (i.e. neither display: none | ||
* nor visibility: hidden). | ||
* | ||
* @param {Element} element DOM element to test | ||
* @return {Boolean} Whether element is visible | ||
*/ | ||
function isVisible( element ) { | ||
return ( | ||
element.offsetWidth > 0 || | ||
element.offsetHeight > 0 || | ||
element.getClientRects().length > 0 | ||
); | ||
} | ||
|
||
/** | ||
* Returns true if the specified area element is a valid focusable element, or | ||
* false otherwise. Area is only focusable if within a map where a named map | ||
* referenced by an image somewhere in the document. | ||
* | ||
* @param {Element} element DOM area element to test | ||
* @return {Boolean} Whether area element is valid for focus | ||
*/ | ||
function isValidFocusableArea( element ) { | ||
const map = element.closest( 'map[name]' ); | ||
if ( ! map ) { | ||
return false; | ||
} | ||
|
||
const img = document.querySelector( 'img[usemap="#' + map.name + '"]' ); | ||
return !! img && isVisible( img ); | ||
} | ||
|
||
/** | ||
* Returns all focusable elements within a given context. | ||
* | ||
* @param {Element} context Element in which to search | ||
* @return {Element[]} Focusable elements | ||
*/ | ||
export function find( context ) { | ||
const elements = context.querySelectorAll( SELECTOR ); | ||
|
||
return [ ...elements ].filter( ( element ) => { | ||
if ( ! isVisible( element ) ) { | ||
return false; | ||
} | ||
|
||
const { nodeName } = element; | ||
if ( 'AREA' === nodeName ) { | ||
return isValidFocusableArea( element ); | ||
} | ||
|
||
return true; | ||
} ); | ||
} |
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 |
---|---|---|
@@ -0,0 +1,4 @@ | ||
import * as focusable from './focusable'; | ||
import * as tabbable from './tabbable'; | ||
|
||
export { focusable, tabbable }; |
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 |
---|---|---|
@@ -0,0 +1,64 @@ | ||
/** | ||
* Internal dependencies | ||
*/ | ||
import { find as findFocusable } from './focusable'; | ||
|
||
/** | ||
* Returns true if the specified element is tabbable, or false otherwise. | ||
* | ||
* @param {Element} element Element to test | ||
* @return {Boolean} Whether element is tabbable | ||
*/ | ||
function isTabbableIndex( element ) { | ||
return element.tabIndex !== -1; | ||
} | ||
|
||
/** | ||
* An array map callback, returning an object with the element value and its | ||
* array index location as properties. This is used to emulate a proper stable | ||
* sort where equal tabIndex should be left in order of their occurrence in the | ||
* document. | ||
* | ||
* @param {Element} element Element | ||
* @param {Number} index Array index of element | ||
* @return {Object} Mapped object with element, index | ||
*/ | ||
function mapElementToObjectTabbable( element, index ) { | ||
return { element, index }; | ||
} | ||
|
||
/** | ||
* An array map callback, returning an element of the given mapped object's | ||
* element value. | ||
* | ||
* @param {Object} object Mapped object with index | ||
* @return {Element} Mapped object element | ||
*/ | ||
function mapObjectTabbableToElement( object ) { | ||
return object.element; | ||
} | ||
|
||
/** | ||
* A sort comparator function used in comparing two objects of mapped elements. | ||
* | ||
* @see mapElementToObjectTabbable | ||
* | ||
* @param {Object} a First object to compare | ||
* @param {Object} b Second object to compare | ||
* @return {Number} Comparator result | ||
*/ | ||
function compareObjectTabbables( a, b ) { | ||
if ( a.element.tabIndex === b.element.tabIndex ) { | ||
return a.index - b.index; | ||
} | ||
|
||
return a.element.tabIndex - b.element.tabIndex; | ||
} | ||
|
||
export function find( context ) { | ||
return findFocusable( context ) | ||
.filter( isTabbableIndex ) | ||
.map( mapElementToObjectTabbable ) | ||
.sort( compareObjectTabbables ) | ||
.map( mapObjectTabbableToElement ); | ||
} |
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 |
---|---|---|
@@ -0,0 +1,140 @@ | ||
/** | ||
* Internal dependencies | ||
*/ | ||
import createElement from './utils/create-element'; | ||
import { find } from '../focusable'; | ||
|
||
describe( 'focusable', () => { | ||
beforeEach( () => { | ||
document.body.innerHTML = ''; | ||
} ); | ||
|
||
describe( 'find()', () => { | ||
it( 'returns empty array if no children', () => { | ||
const node = createElement( 'div' ); | ||
|
||
expect( find( node ) ).toEqual( [] ); | ||
} ); | ||
|
||
it( 'returns empty array if no focusable children', () => { | ||
const node = createElement( 'div' ); | ||
node.appendChild( createElement( 'div' ) ); | ||
|
||
expect( find( node ) ).toEqual( [] ); | ||
} ); | ||
|
||
it( 'returns array of focusable children', () => { | ||
const node = createElement( 'div' ); | ||
node.appendChild( createElement( 'input' ) ); | ||
|
||
const focusable = find( node ); | ||
|
||
expect( focusable ).toHaveLength( 1 ); | ||
expect( focusable[ 0 ].nodeName ).toBe( 'INPUT' ); | ||
} ); | ||
|
||
it( 'finds nested focusable child', () => { | ||
const node = createElement( 'div' ); | ||
node.appendChild( createElement( 'div' ) ); | ||
node.firstChild.appendChild( createElement( 'input' ) ); | ||
|
||
const focusable = find( node ); | ||
|
||
expect( focusable ).toHaveLength( 1 ); | ||
expect( focusable[ 0 ].nodeName ).toBe( 'INPUT' ); | ||
} ); | ||
|
||
it( 'finds link with no href but tabindex', () => { | ||
const node = createElement( 'div' ); | ||
const link = createElement( 'a' ); | ||
link.tabIndex = 0; | ||
node.appendChild( link ); | ||
|
||
expect( find( node ) ).toEqual( [ link ] ); | ||
} ); | ||
|
||
it( 'finds valid area focusable', () => { | ||
const map = createElement( 'map' ); | ||
map.name = 'testfocus'; | ||
const area = createElement( 'area' ); | ||
area.href = ''; | ||
map.appendChild( area ); | ||
const img = createElement( 'img' ); | ||
img.setAttribute( 'usemap', '#testfocus' ); | ||
document.body.appendChild( map ); | ||
document.body.appendChild( img ); | ||
|
||
const focusable = find( map ); | ||
|
||
expect( focusable ).toHaveLength( 1 ); | ||
expect( focusable[ 0 ].nodeName ).toBe( 'AREA' ); | ||
} ); | ||
|
||
it( 'ignores invalid area focusable', () => { | ||
const map = createElement( 'map' ); | ||
map.name = 'testfocus'; | ||
const area = createElement( 'area' ); | ||
area.href = ''; | ||
map.appendChild( area ); | ||
const img = createElement( 'img' ); | ||
img.setAttribute( 'usemap', '#testfocus' ); | ||
img.style.display = 'none'; | ||
document.body.appendChild( map ); | ||
document.body.appendChild( img ); | ||
|
||
expect( find( map ) ).toEqual( [] ); | ||
} ); | ||
|
||
it( 'ignores invisible inputs', () => { | ||
const node = createElement( 'div' ); | ||
const input = createElement( 'input' ); | ||
node.appendChild( input ); | ||
|
||
input.style.visibility = 'hidden'; | ||
expect( find( node ) ).toEqual( [] ); | ||
|
||
input.style.visibility = 'visible'; | ||
input.style.display = 'none'; | ||
expect( find( node ) ).toEqual( [] ); | ||
|
||
input.style.display = 'inline-block'; | ||
const focusable = find( node ); | ||
expect( focusable ).toHaveLength( 1 ); | ||
expect( focusable[ 0 ].nodeName ).toBe( 'INPUT' ); | ||
} ); | ||
|
||
it( 'ignores inputs in invisible ancestors', () => { | ||
const node = createElement( 'div' ); | ||
const input = createElement( 'input' ); | ||
node.appendChild( input ); | ||
|
||
node.style.visibility = 'hidden'; | ||
expect( find( node ) ).toEqual( [] ); | ||
|
||
node.style.visibility = 'visible'; | ||
node.style.display = 'none'; | ||
expect( find( node ) ).toEqual( [] ); | ||
|
||
node.style.display = 'block'; | ||
const focusable = find( node ); | ||
expect( focusable ).toHaveLength( 1 ); | ||
expect( focusable[ 0 ].nodeName ).toBe( 'INPUT' ); | ||
} ); | ||
|
||
it( 'does not return context even if focusable', () => { | ||
const node = createElement( 'div' ); | ||
node.tabIndex = 0; | ||
|
||
expect( find( node ) ).toEqual( [] ); | ||
} ); | ||
|
||
it( 'limits found focusables to specific context', () => { | ||
const node = createElement( 'div' ); | ||
node.appendChild( createElement( 'div' ) ); | ||
document.body.appendChild( node ); | ||
document.body.appendChild( createElement( 'input' ) ); | ||
|
||
expect( find( node ) ).toEqual( [] ); | ||
} ); | ||
} ); | ||
} ); |
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.
We could try to reuse this utility in
WritingFlow
. I don't know how well this will behave compared to our current selector.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.
I agree, it would be good to have a consistent, well-tested approach for this.
Better (or more accurate anyways), I hope 😬
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.
I'll try it ;)