Skip to content

Commit

Permalink
Fix arrows navigation in the block more options menu (#7480)
Browse files Browse the repository at this point in the history 
* Fix arrows navigation in the block more options menu.

* Components: Remove deep prop from NavigableContainer

Treat as default
  • Loading branch information
@afercia @gziolo
afercia authored and gziolo committed Jul 5, 2018
1 parent 18d832e commit 9a23989
Show file tree
Hide file tree
Showing 9 changed files with 28 additions and 239 deletions.
9 changes: 0 additions & 9 deletions components/navigable-container/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,6 @@ A callback invoked when the menu navigates to one of its children passing the in
- Type: `Function`
- Required: No

### deep

A boolean to look for navigable children in the direct children or any descendant. True means that any descendant can be considered navigable, and false means only direct children are considered.

- Type: `Boolean`
- Required: No
- default: false


### cycle

A boolean which tells the component whether or not to cycle from the end back to the beginning and vice versa.
Expand Down
7 changes: 2 additions & 5 deletions components/navigable-container/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,9 @@ class NavigableContainer extends Component {
}

getFocusableContext( target ) {
const { deep = false, onlyBrowserTabstops } = this.props;
const { onlyBrowserTabstops } = this.props;
const finder = onlyBrowserTabstops ? focus.tabbable : focus.focusable;
const focusables = finder
.find( this.container )
.filter( ( node ) => deep || node.parentElement === this.container );
const focusables = finder.find( this.container );

const index = this.getFocusableIndex( focusables, target );
if ( index > -1 && target ) {
Expand Down Expand Up @@ -109,7 +107,6 @@ class NavigableContainer extends Component {
'eventToOffset',
'onNavigate',
'cycle',
'deep',
'onlyBrowserTabstops',
] ) }
onKeyDown={ this.onKeyDown }
Expand Down
218 changes: 7 additions & 211 deletions components/navigable-container/test/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,39 +43,6 @@ function fireKeyDown( container, keyCode, shiftKey ) {

describe( 'NavigableMenu', () => {
it( 'vertical: should navigate by up and down', () => {
let currentIndex = 0;
const wrapper = mount( (
<NavigableMenu onNavigate={ ( index ) => currentIndex = index }>
<button id="btn1">One</button>
<button id="btn2">Two</button>
<button id="btn3">Three</button>
</NavigableMenu >
) );

simulateVisible( wrapper, '*' );

const container = wrapper.find( 'div' );
wrapper.getDOMNode().querySelector( '#btn1' ).focus();

// Navigate options
function assertKeyDown( keyCode, expectedActiveIndex, expectedStop ) {
const interaction = fireKeyDown( container, keyCode );
expect( currentIndex ).toBe( expectedActiveIndex );
expect( interaction.stopped ).toBe( expectedStop );
}

assertKeyDown( DOWN, 1, true );
assertKeyDown( DOWN, 2, true );
assertKeyDown( DOWN, 0, true );
assertKeyDown( UP, 2, true );
assertKeyDown( UP, 1, true );
assertKeyDown( UP, 0, true );
assertKeyDown( LEFT, 0, false );
assertKeyDown( RIGHT, 0, false );
assertKeyDown( SPACE, 0, false );
} );

it( 'vertical: should navigate by up and down, and skip deep candidates', () => {
let currentIndex = 0;
const wrapper = mount( (
<NavigableMenu orientation="vertical" onNavigate={ ( index ) => currentIndex = index }>
Expand All @@ -85,43 +52,7 @@ describe( 'NavigableMenu', () => {
<span id="btn-deep" tabIndex="-1">Deep</span>
</span>
<span tabIndex="-1" id="btn3">Three</span>
</NavigableMenu >
) );

simulateVisible( wrapper, '*' );

const container = wrapper.find( 'div' );
wrapper.getDOMNode().querySelector( '#btn1' ).focus();

// Navigate options
function assertKeyDown( keyCode, expectedActiveIndex, expectedStop ) {
const interaction = fireKeyDown( container, keyCode, false );
expect( currentIndex ).toBe( expectedActiveIndex );
expect( interaction.stopped ).toBe( expectedStop );
}

assertKeyDown( DOWN, 1, true );
assertKeyDown( DOWN, 2, true );
assertKeyDown( DOWN, 0, true );
assertKeyDown( UP, 2, true );
assertKeyDown( UP, 1, true );
assertKeyDown( UP, 0, true );
assertKeyDown( LEFT, 0, false );
assertKeyDown( RIGHT, 0, false );
assertKeyDown( SPACE, 0, false );
} );

it( 'vertical: should navigate by up and down, and explore deep candidates', () => {
let currentIndex = 0;
const wrapper = mount( (
<NavigableMenu deep={ true } orientation="vertical" onNavigate={ ( index ) => currentIndex = index }>
<span tabIndex="-1" id="btn1">One</span>
<span tabIndex="-1" id="btn2">Two</span>
<span id="btn-deep-wrapper">
<span id="btn-deep" tabIndex="-1">Deep</span>
</span>
<span tabIndex="-1" id="btn3">Three</span>
</NavigableMenu >
</NavigableMenu>
) );

simulateVisible( wrapper, '*' );
Expand Down Expand Up @@ -156,7 +87,7 @@ describe( 'NavigableMenu', () => {
<span tabIndex="-1" id="btn1">One</span>
<span tabIndex="-1" id="btn2">Two</span>
<span tabIndex="-1" id="btn3">Three</span>
</NavigableMenu >
</NavigableMenu>
) );

simulateVisible( wrapper, '*' );
Expand Down Expand Up @@ -186,82 +117,13 @@ describe( 'NavigableMenu', () => {
let currentIndex = 0;
const wrapper = mount( (
<NavigableMenu orientation="horizontal" onNavigate={ ( index ) => currentIndex = index }>
<button id="btn1">One</button>
<button id="btn2">Two</button>
<button id="btn3">Three</button>
</NavigableMenu >
) );

simulateVisible( wrapper, '*' );

const container = wrapper.find( 'div' );
wrapper.getDOMNode().querySelector( '#btn1' ).focus();

// Navigate options
function assertKeyDown( keyCode, expectedActiveIndex, expectedStop ) {
const interaction = fireKeyDown( container, keyCode, false );
expect( currentIndex ).toBe( expectedActiveIndex );
expect( interaction.stopped ).toBe( expectedStop );
}

assertKeyDown( RIGHT, 1, true );
assertKeyDown( RIGHT, 2, true );
assertKeyDown( RIGHT, 0, true );
assertKeyDown( LEFT, 2, true );
assertKeyDown( LEFT, 1, true );
assertKeyDown( LEFT, 0, true );
assertKeyDown( UP, 0, false );
assertKeyDown( DOWN, 0, false );
assertKeyDown( SPACE, 0, false );
} );

it( 'horizontal: should navigate by left and right, and skip deep candidates', () => {
let currentIndex = 0;
const wrapper = mount( (
<NavigableMenu orientation="horizontal" onNavigate={ ( index ) => currentIndex = index }>
<span tabIndex="-1" id="btn1">One</span>
<span tabIndex="-1" id="btn2">Two</span>
<span id="btn-deep-wrapper">
<span id="btn-deep" tabIndex="-1">Deep</span>
</span>
<span tabIndex="-1" id="btn3">Three</span>
</NavigableMenu >
) );

simulateVisible( wrapper, '*' );

const container = wrapper.find( 'div' );
wrapper.getDOMNode().querySelector( '#btn1' ).focus();

// Navigate options
function assertKeyDown( keyCode, expectedActiveIndex, expectedStop ) {
const interaction = fireKeyDown( container, keyCode, false );
expect( currentIndex ).toBe( expectedActiveIndex );
expect( interaction.stopped ).toBe( expectedStop );
}

assertKeyDown( RIGHT, 1, true );
assertKeyDown( RIGHT, 2, true );
assertKeyDown( RIGHT, 0, true );
assertKeyDown( LEFT, 2, true );
assertKeyDown( LEFT, 1, true );
assertKeyDown( LEFT, 0, true );
assertKeyDown( UP, 0, false );
assertKeyDown( DOWN, 0, false );
assertKeyDown( SPACE, 0, false );
} );

it( 'horizontal: should navigate by left and right, and explore deep candidates', () => {
let currentIndex = 0;
const wrapper = mount( (
<NavigableMenu deep={ true } orientation="horizontal" onNavigate={ ( index ) => currentIndex = index }>
<span tabIndex="-1" id="btn1">One</span>
<span tabIndex="-1" id="btn2">Two</span>
<span id="btn-deep-wrapper">
<span id="btn-deep" tabIndex="-1">Deep</span>
</span>
<span tabIndex="-1" id="btn3">Three</span>
</NavigableMenu >
</NavigableMenu>
) );

simulateVisible( wrapper, '*' );
Expand Down Expand Up @@ -296,7 +158,7 @@ describe( 'NavigableMenu', () => {
<span tabIndex="-1" id="btn1">One</span>
<span tabIndex="-1" id="btn2">Two</span>
<span tabIndex="-1" id="btn3">Three</span>
</NavigableMenu >
</NavigableMenu>
) );

simulateVisible( wrapper, '*' );
Expand Down Expand Up @@ -329,7 +191,7 @@ describe( 'NavigableMenu', () => {
<button id="btn1">One</button>
<button id="btn2">Two</button>
<button id="btn3">Three</button>
</NavigableMenu >
</NavigableMenu>
) );

simulateVisible( wrapper, '*' );
Expand Down Expand Up @@ -365,79 +227,13 @@ describe( 'TabbableContainer', () => {
let currentIndex = 0;
const wrapper = mount( (
<TabbableContainer className="wrapper" onNavigate={ ( index ) => currentIndex = index }>
<div className="section" id="section1" tabIndex="0">Section One</div>
<div className="section" id="section2a" tabIndex="0">Section Two</div>
<div className="section" id="section2b" tabIndex="-1">Section to Skip</div>
<div className="section" id="section3" tabIndex="0">Section Three</div>
</TabbableContainer >
) );

simulateVisible( wrapper, '*' );

const container = wrapper.find( 'div.wrapper' );
wrapper.getDOMNode().querySelector( '#section1' ).focus();

// Navigate options
function assertKeyDown( keyCode, shiftKey, expectedActiveIndex, expectedStop ) {
const interaction = fireKeyDown( container, keyCode, shiftKey );
expect( currentIndex ).toBe( expectedActiveIndex );
expect( interaction.stopped ).toBe( expectedStop );
}

assertKeyDown( TAB, false, 1, true );
assertKeyDown( TAB, false, 2, true );
assertKeyDown( TAB, false, 0, true );
assertKeyDown( TAB, true, 2, true );
assertKeyDown( TAB, true, 1, true );
assertKeyDown( TAB, true, 0, true );
assertKeyDown( SPACE, false, 0, false );
} );

it( 'should navigate by keypresses and overlook deep candidates', () => {
let currentIndex = 0;
const wrapper = mount( (
<TabbableContainer className="wrapper" onNavigate={ ( index ) => currentIndex = index }>
<div className="section" id="section1" tabIndex="0">Section One</div>
<div className="section" id="section2" tabIndex="0">Section Two</div>
<div className="deep-section">
<div className="section" id="section-deep" tabIndex="0">Section to Skip</div>
</div>
<div className="section" id="section3" tabIndex="0">Section Three</div>
</TabbableContainer >
) );

simulateVisible( wrapper, '*' );

const container = wrapper.find( 'div.wrapper' );
wrapper.getDOMNode().querySelector( '#section1' ).focus();

// Navigate options
function assertKeyDown( keyCode, shiftKey, expectedActiveIndex, expectedStop ) {
const interaction = fireKeyDown( container, keyCode, shiftKey );
expect( currentIndex ).toBe( expectedActiveIndex );
expect( interaction.stopped ).toBe( expectedStop );
}

assertKeyDown( TAB, false, 1, true );
assertKeyDown( TAB, false, 2, true );
assertKeyDown( TAB, false, 0, true );
assertKeyDown( TAB, true, 2, true );
assertKeyDown( TAB, true, 1, true );
assertKeyDown( TAB, true, 0, true );
assertKeyDown( SPACE, false, 0, false );
} );

it( 'should navigate by keypresses and explore deep candidates', () => {
let currentIndex = 0;
const wrapper = mount( (
<TabbableContainer deep={ true } className="wrapper" onNavigate={ ( index ) => currentIndex = index }>
<div className="section" id="section1" tabIndex="0">Section One</div>
<div className="section" id="section2" tabIndex="0">Section Two</div>
<div className="deep-section-wrapper">
<div className="section" id="section-deep" tabIndex="0">Section to <strong>not</strong> skip</div>
</div>
<div className="section" id="section3" tabIndex="0">Section Three</div>
</TabbableContainer >
</TabbableContainer>
) );

simulateVisible( wrapper, '*' );
Expand Down Expand Up @@ -470,7 +266,7 @@ describe( 'TabbableContainer', () => {
<div className="section" id="section1" tabIndex="0">Section One</div>
<div className="section" id="section2" tabIndex="0">Section Two</div>
<div className="section" id="section3" tabIndex="0">Section Three</div>
</TabbableContainer >
</TabbableContainer>
) );

simulateVisible( wrapper, '*' );
Expand Down
2 changes: 1 addition & 1 deletion components/slot-fill/slot.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ class Slot extends Component {
} );

return (
<div ref={ this.bindNode }>
<div ref={ this.bindNode } role="presentation">
{ isFunction( children ) ? children( fills.filter( Boolean ) ) : fills }
</div>
);
Expand Down
Loading

0 comments on commit 9a23989

Please sign in to comment.