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

Fix keyboard navigation on the Image block toolbar #25127

Merged
merged 10 commits into from
Sep 24, 2020
Merged

Fix keyboard navigation on the Image block toolbar #25127

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
35c46da
Fix keyboard navigation on the Image block toolbar
diegohaz Sep 7, 2020
191e228
Add toggleProps prop to AspectMenu component
diegohaz Sep 10, 2020
dfde4c7
Use ToolbarItem on TemplateParts
diegohaz Sep 10, 2020
734d3a9
Revert "Use ToolbarItem on TemplateParts"
diegohaz Sep 10, 2020
1fd5a41
Merge branch 'master' into try/image-block-toolbar-roving-tabindex
diegohaz Sep 10, 2020
cdafb0e
Ignore warnings on TemplateParts block
diegohaz Sep 10, 2020
6c5e5de
Merge branch 'master' into try/image-block-toolbar-roving-tabindex
diegohaz Sep 17, 2020
0daa4ba
Fix styles
diegohaz Sep 17, 2020
d261eb3
Use ToolbarGroup on TemplateParts
diegohaz Sep 17, 2020
b02f72d
Remove unnecessary styles
diegohaz Sep 17, 2020
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
14 changes: 1 addition & 13 deletions packages/block-editor/src/components/navigable-toolbar/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,6 @@ import deprecated from '@wordpress/deprecated';
import { focus } from '@wordpress/dom';
import { useShortcut } from '@wordpress/keyboard-shortcuts';

function useUpdateLayoutEffect( effect, deps ) {
const mounted = useRef( false );
useLayoutEffect( () => {
if ( mounted.current ) {
return effect();
}
mounted.current = true;
}, deps );
}

function hasOnlyToolbarItem( elements ) {
const dataProp = 'toolbarItem';
return ! elements.some( ( element ) => ! ( dataProp in element.dataset ) );
Expand Down Expand Up @@ -71,9 +61,7 @@ function useIsAccessibleToolbar( ref ) {
setIsAccessibleToolbar( onlyToolbarItem );
}, [] );

useLayoutEffect( determineIsAccessibleToolbar, [] );

useUpdateLayoutEffect( () => {
useLayoutEffect( () => {
// Toolbar buttons may be rendered asynchronously, so we use
// MutationObserver to check if the toolbar subtree has been modified
const observer = new window.MutationObserver(
Expand Down
9 changes: 8 additions & 1 deletion packages/block-library/src/image/image-editor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,19 @@ function AspectGroup( { aspectRatios, isDisabled, label, onClick, value } ) {
);
}

function AspectMenu( { isDisabled, onClick, value, defaultValue } ) {
function AspectMenu( {
toggleProps,
isDisabled,
onClick,
value,
defaultValue,
} ) {
return (
<DropdownMenu
icon={ aspectRatioIcon }
label={ __( 'Aspect Ratio' ) }
popoverProps={ POPOVER_PROPS }
toggleProps={ toggleProps }
Comment on lines +63 to +75
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

toggleProps wasn't being passed to this component. That's why it wasn't being considered a ToolbarItem.

className="wp-block-image__aspect-ratio"
>
{ ( { onClose } ) => (
Expand Down
6 changes: 3 additions & 3 deletions packages/block-library/src/template-part/edit/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
BlockControls,
__experimentalBlock as Block,
} from '@wordpress/block-editor';
import { Dropdown, ToolbarButton } from '@wordpress/components';
import { Dropdown, ToolbarGroup, ToolbarButton } from '@wordpress/components';
import { __ } from '@wordpress/i18n';
import { chevronUp, chevronDown } from '@wordpress/icons';

Expand Down Expand Up @@ -71,7 +71,7 @@ export default function TemplatePartEdit( {
return (
<BlockWrapper>
<BlockControls>
<div className="wp-block-template-part__block-control-group">
<ToolbarGroup className="wp-block-template-part__block-control-group">
<TemplatePartNamePanel
postId={ postId }
setAttributes={ setAttributes }
Expand All @@ -98,7 +98,7 @@ export default function TemplatePartEdit( {
/>
) }
/>
</div>
</ToolbarGroup>
</BlockControls>
<TemplatePartInnerBlocks
postId={ postId }
Expand Down
5 changes: 0 additions & 5 deletions packages/block-library/src/template-part/editor.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@
display: flex;

.wp-block-template-part__name-panel {
background-color: $white;
outline: 1px solid transparent;
padding: $grid-unit-10 0 $grid-unit-10 $grid-unit-15;

Expand All @@ -79,10 +78,6 @@
margin-right: 8px;
}
}

.wp-block-template-part__preview-dropdown-button {
border-right: $border-width solid $gray-900;
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated the code to use ToolbarGroup instead of just a <div>, so these styles that were simulating a toolbar group aren't needed anymore.

}

.is-navigate-mode .is-selected .wp-block-template-part__name-panel {
Expand Down
7 changes: 6 additions & 1 deletion packages/components/src/toolbar-group/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,16 @@
min-height: $block-toolbar-height;
margin: 0;
border: $border-width solid $gray-900;
border-radius: $radius-block-ui;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was used in the legacy toolbar apparently to make toolbar groups rounded, but it seems that it's not needed anymore.

background-color: $white;
display: inline-flex;
flex-shrink: 0;
flex-wrap: wrap;

// Unset for nested toolbars. Increase specificity.
& .components-toolbar.components-toolbar {
border-width: 0;
margin: 0;
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This snippet is already used in the new toolbar. This code also applies it to the legacy toolbar.

}

div.components-toolbar {
Expand Down
5 changes: 5 additions & 0 deletions packages/e2e-tests/specs/experiments/multi-entity-saving.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,11 @@ describe( 'Multi-entity save flow', () => {
await page.keyboard.type( 'some words...' );

await assertMultiSaveEnabled();

// TODO: Remove when toolbar supports text fields
expect( console ).toHaveWarnedWith(
'Using custom components as toolbar controls is deprecated. Please use ToolbarItem or ToolbarButton components instead. See: https://developer.wordpress.org/block-editor/components/toolbar-button/#inside-blockcontrols'
);
} );

it( 'Should only have save panel a11y button active after child entities edited', async () => {
Expand Down
10 changes: 10 additions & 0 deletions packages/e2e-tests/specs/experiments/template-part.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,11 @@ describe( 'Template Part', () => {
await page.keyboard.type( testContent );
await page.click( savePostSelector );
await page.click( entitiesSaveSelector );

// TODO: Remove when toolbar supports text fields
expect( console ).toHaveWarnedWith(
'Using custom components as toolbar controls is deprecated. Please use ToolbarItem or ToolbarButton components instead. See: https://developer.wordpress.org/block-editor/components/toolbar-button/#inside-blockcontrols'
);
} );

it( 'Should preview newly added template part', async () => {
Expand All @@ -138,6 +143,11 @@ describe( 'Template Part', () => {
testContentSelector
);
expect( templatePartContent ).toBeTruthy();

// TODO: Remove when toolbar supports text fields
expect( console ).toHaveWarnedWith(
'Using custom components as toolbar controls is deprecated. Please use ToolbarItem or ToolbarButton components instead. See: https://developer.wordpress.org/block-editor/components/toolbar-button/#inside-blockcontrols'
);
} );
} );
} );