From 57c5ee27a10765129672e3d02cddba8afa676623 Mon Sep 17 00:00:00 2001 From: Andrea Fercia Date: Fri, 22 Jun 2018 15:55:16 +0200 Subject: [PATCH 1/2] Fix arrows navigation in the block more options menu. --- components/slot-fill/slot.js | 2 +- components/slot-fill/test/slot.js | 18 +++++++++--------- .../test/__snapshots__/index.js.snap | 4 +++- .../test/__snapshots__/index.js.snap | 4 +++- .../test/__snapshots__/index.js.snap | 4 +++- editor/components/block-settings-menu/index.js | 2 +- 6 files changed, 20 insertions(+), 14 deletions(-) diff --git a/components/slot-fill/slot.js b/components/slot-fill/slot.js index d36900e8b58b3..d380528bc6622 100644 --- a/components/slot-fill/slot.js +++ b/components/slot-fill/slot.js @@ -71,7 +71,7 @@ class Slot extends Component { } ); return ( -
+
{ isFunction( children ) ? children( fills.filter( Boolean ) ) : fills }
); diff --git a/components/slot-fill/test/slot.js b/components/slot-fill/test/slot.js index 3d84e9d5f0289..85cbf09141eb5 100644 --- a/components/slot-fill/test/slot.js +++ b/components/slot-fill/test/slot.js @@ -41,7 +41,7 @@ describe( 'Slot', () => { ); - expect( element.find( 'Slot > div' ).html() ).toBe( '
' ); + expect( element.find( 'Slot > div' ).html() ).toBe( '
' ); } ); it( 'should render a string Fill', () => { @@ -54,7 +54,7 @@ describe( 'Slot', () => { ); - expect( element.find( 'Slot > div' ).html() ).toBe( '
content
' ); + expect( element.find( 'Slot > div' ).html() ).toBe( '
content
' ); } ); it( 'should render a Fill containing an element', () => { @@ -67,7 +67,7 @@ describe( 'Slot', () => { ); - expect( element.find( 'Slot > div' ).html() ).toBe( '
' ); + expect( element.find( 'Slot > div' ).html() ).toBe( '
' ); } ); it( 'should render a Fill containing an array', () => { @@ -80,7 +80,7 @@ describe( 'Slot', () => { ); - expect( element.find( 'Slot > div' ).html() ).toBe( '
text
' ); + expect( element.find( 'Slot > div' ).html() ).toBe( '
text
' ); } ); it( 'calls the functions passed as the Slot\'s fillProps in the Fill', () => { @@ -118,7 +118,7 @@ describe( 'Slot', () => { ); - expect( element.find( 'Slot > div' ).html() ).toBe( '
' ); + expect( element.find( 'Slot > div' ).html() ).toBe( '
' ); } ); it( 'should render a string Fill with HTML wrapper when render props used', () => { @@ -137,7 +137,7 @@ describe( 'Slot', () => { ); - expect( element.find( 'Slot > div' ).html() ).toBe( '
content
' ); + expect( element.find( 'Slot > div' ).html() ).toBe( '
content
' ); } ); it( 'should re-render Slot when not bubbling virtually', () => { @@ -148,11 +148,11 @@ describe( 'Slot', () => { ); - expect( element.find( 'Slot > div' ).html() ).toBe( '
1
' ); + expect( element.find( 'Slot > div' ).html() ).toBe( '
1
' ); element.find( 'button' ).simulate( 'click' ); - expect( element.find( 'Slot > div' ).html() ).toBe( '
2
' ); + expect( element.find( 'Slot > div' ).html() ).toBe( '
2
' ); } ); it( 'should render in expected order', () => { @@ -185,6 +185,6 @@ describe( 'Slot', () => { ], } ); - expect( element.find( 'Slot > div' ).html() ).toBe( '
firstsecond
' ); + expect( element.find( 'Slot > div' ).html() ).toBe( '
firstsecond
' ); } ); } ); diff --git a/edit-post/components/sidebar/plugin-post-publish-panel/test/__snapshots__/index.js.snap b/edit-post/components/sidebar/plugin-post-publish-panel/test/__snapshots__/index.js.snap index b7942b67ca8f4..ab22e02bb67b7 100644 --- a/edit-post/components/sidebar/plugin-post-publish-panel/test/__snapshots__/index.js.snap +++ b/edit-post/components/sidebar/plugin-post-publish-panel/test/__snapshots__/index.js.snap @@ -1,7 +1,9 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`PluginPostPublishPanel renders fill properly 1`] = ` -
+
+
+
( // Should this just use a DropdownMenu instead of a DropDown ? - + <_BlockSettingsMenuFirstItem.Slot fillProps={ { onClose } } /> { count === 1 && } { count === 1 && } From c8f558f4acc8047b3a4e614608d0052b5c319872 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Tue, 26 Jun 2018 11:42:18 -0400 Subject: [PATCH 2/2] Components: Remove deep prop from NavigableContainer Treat as default --- components/navigable-container/README.md | 9 - components/navigable-container/index.js | 7 +- components/navigable-container/test/index.js | 218 +----------------- .../components/block-settings-menu/index.js | 2 +- editor/components/navigable-toolbar/index.js | 1 - 5 files changed, 10 insertions(+), 227 deletions(-) diff --git a/components/navigable-container/README.md b/components/navigable-container/README.md index 7cfa3756caadd..bb30c24234ac7 100644 --- a/components/navigable-container/README.md +++ b/components/navigable-container/README.md @@ -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. diff --git a/components/navigable-container/index.js b/components/navigable-container/index.js index a599c96245640..50fc4131db8df 100644 --- a/components/navigable-container/index.js +++ b/components/navigable-container/index.js @@ -41,11 +41,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 ) { @@ -114,7 +112,6 @@ class NavigableContainer extends Component { 'eventToOffset', 'onNavigate', 'cycle', - 'deep', 'onlyBrowserTabstops', ] ) } onKeyDown={ this.onKeyDown } diff --git a/components/navigable-container/test/index.js b/components/navigable-container/test/index.js index bc05f0684c5ac..6a642e1868e40 100644 --- a/components/navigable-container/test/index.js +++ b/components/navigable-container/test/index.js @@ -45,39 +45,6 @@ function fireKeyDown( container, keyCode, shiftKey ) { describe( 'NavigableMenu', () => { it( 'vertical: should navigate by up and down', () => { - let currentIndex = 0; - const wrapper = mount( ( - currentIndex = index }> - - - - - ) ); - - 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( ( currentIndex = index }> @@ -87,43 +54,7 @@ describe( 'NavigableMenu', () => { Deep Three - - ) ); - - 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( ( - currentIndex = index }> - One - Two - - Deep - - Three - + ) ); simulateVisible( wrapper, '*' ); @@ -158,7 +89,7 @@ describe( 'NavigableMenu', () => { One Two Three - + ) ); simulateVisible( wrapper, '*' ); @@ -188,82 +119,13 @@ describe( 'NavigableMenu', () => { let currentIndex = 0; const wrapper = mount( ( currentIndex = index }> - - - - - ) ); - - 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( ( - currentIndex = index }> - One - Two - - Deep - - Three - - ) ); - - 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( ( - currentIndex = index }> One Two Deep Three - + ) ); simulateVisible( wrapper, '*' ); @@ -298,7 +160,7 @@ describe( 'NavigableMenu', () => { One Two Three - + ) ); simulateVisible( wrapper, '*' ); @@ -331,7 +193,7 @@ describe( 'NavigableMenu', () => { - + ) ); simulateVisible( wrapper, '*' ); @@ -367,79 +229,13 @@ describe( 'TabbableContainer', () => { let currentIndex = 0; const wrapper = mount( ( currentIndex = index }> -
Section One
-
Section Two
-
Section to Skip
-
Section Three
- - ) ); - - 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( ( - currentIndex = index }> -
Section One
-
Section Two
-
-
Section to Skip
-
-
Section Three
- - ) ); - - 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( ( - currentIndex = index }>
Section One
Section Two
Section to not skip
Section Three
- + ) ); simulateVisible( wrapper, '*' ); @@ -472,7 +268,7 @@ describe( 'TabbableContainer', () => {
Section One
Section Two
Section Three
- + ) ); simulateVisible( wrapper, '*' ); diff --git a/editor/components/block-settings-menu/index.js b/editor/components/block-settings-menu/index.js index 00412945ac844..1c57035949118 100644 --- a/editor/components/block-settings-menu/index.js +++ b/editor/components/block-settings-menu/index.js @@ -93,7 +93,7 @@ export class BlockSettingsMenu extends Component { } } renderContent={ ( { onClose } ) => ( // Should this just use a DropdownMenu instead of a DropDown ? - + <_BlockSettingsMenuFirstItem.Slot fillProps={ { onClose } } /> { count === 1 && } { count === 1 && } diff --git a/editor/components/navigable-toolbar/index.js b/editor/components/navigable-toolbar/index.js index 6537ff11fcc2c..0597a3461cecb 100644 --- a/editor/components/navigable-toolbar/index.js +++ b/editor/components/navigable-toolbar/index.js @@ -79,7 +79,6 @@ class NavigableToolbar extends Component {