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.

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

Make accessible toolbar stable and deprecate old usage #23316

Merged
merged 28 commits into from
Aug 11, 2020
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
fc54ccd
Deprecate legacy toolbar usage
diegohaz Jun 19, 2020
e29ae96
Update e2e tests
diegohaz Jun 19, 2020
867543d
Merge branch 'master' into try/accessible-toolbar
diegohaz Jul 13, 2020
b40177f
Merge branch 'master' into try/accessible-toolbar
diegohaz Jul 16, 2020
d5201eb
Remove __experimental flag from Toolbar accessibilityLabel prop
diegohaz Jul 16, 2020
ac60181
Merge branch 'master' into try/accessible-toolbar
diegohaz Jul 20, 2020
fde5aa1
Merge branch 'master' into try/accessible-toolbar
diegohaz Jul 23, 2020
7aecd5e
Use label instead of accessibilityLabel
diegohaz Jul 23, 2020
94d5ca2
Deprecate <Toolbar> without label prop
diegohaz Jul 23, 2020
26dda7e
Make ToolbarItem component stable
diegohaz Jul 23, 2020
5efe0db
Deprecate ToolbarButton usage without a parent Toolbar
diegohaz Jul 23, 2020
f48467e
Simplify jsdocs
diegohaz Jul 23, 2020
b464950
data-experimental-toolbar-item to data-toolbar-item
diegohaz Jul 23, 2020
1813a7d
Remove deprecation warning from ToolbarButton
diegohaz Jul 23, 2020
e16144a
Update unit test
diegohaz Jul 23, 2020
e387239
Update Toolbar README
diegohaz Jul 23, 2020
696279d
Update tests
diegohaz Jul 23, 2020
cf740b4
Remove ToolbarGroup test from Toolbar
diegohaz Jul 23, 2020
c184881
Merge branch 'master' into try/accessible-toolbar
diegohaz Jul 28, 2020
dff1660
Replace Toolbar by ToolbarGroup on the Classic Editor
diegohaz Jul 28, 2020
dbc4da3
Merge branch 'master' into try/accessible-toolbar
diegohaz Aug 6, 2020
2b5d232
Add Toolbar, ToolbarButton, ToolbarGroup and ToolbarItem docs
diegohaz Aug 6, 2020
5931985
Add links to depreaction notices and warnings
diegohaz Aug 6, 2020
cf8483d
Update ToolbarButton docs
diegohaz Aug 6, 2020
df663c6
Improve documentation around BlockControls usage
diegohaz Aug 10, 2020
dd7ead5
Update deprecation warning link on Navigable Toolbar
diegohaz Aug 10, 2020
e177fb1
Update ToolbarItem README
diegohaz Aug 10, 2020
c0acb82
Update ToolbarItem README
diegohaz Aug 10, 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
11 changes: 9 additions & 2 deletions packages/block-editor/src/components/navigable-toolbar/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
useEffect,
useCallback,
} from '@wordpress/element';
import deprecated from '@wordpress/deprecated';
import { focus } from '@wordpress/dom';
import { useShortcut } from '@wordpress/keyboard-shortcuts';

Expand Down Expand Up @@ -43,7 +44,13 @@ function useIsAccessibleToolbar( ref ) {

const determineIsAccessibleToolbar = useCallback( () => {
const tabbables = focus.tabbable.find( ref.current );
setIsAccessibleToolbar( hasOnlyToolbarItem( tabbables ) );
const onlyToolbarItem = hasOnlyToolbarItem( tabbables );
if ( ! onlyToolbarItem ) {
deprecated( 'Using custom components as toolbar controls', {
diegohaz marked this conversation as resolved.
Show resolved Hide resolved
alternative: 'ToolbarItem or ToolbarButton components',
} );
}
setIsAccessibleToolbar( onlyToolbarItem );
}, [] );

useLayoutEffect( determineIsAccessibleToolbar, [] );
Expand Down Expand Up @@ -90,7 +97,7 @@ function NavigableToolbar( { children, focusOnMount, ...props } ) {
if ( isAccessibleToolbar ) {
return (
<Toolbar
__experimentalAccessibilityLabel={ props[ 'aria-label' ] }
accessibilityLabel={ props[ 'aria-label' ] }
diegohaz marked this conversation as resolved.
Show resolved Hide resolved
ref={ wrapper }
{ ...props }
>
Expand Down
4 changes: 1 addition & 3 deletions packages/block-library/src/heading/heading-level-dropdown.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,7 @@ export default function HeadingLevelDropdown( { selectedLevel, onChange } ) {
renderContent={ () => (
<Toolbar
className="block-library-heading-level-toolbar"
__experimentalAccessibilityLabel={ __(
'Change heading level'
) }
accessibilityLabel={ __( 'Change heading level' ) }
>
<ToolbarGroup
isCollapsed={ false }
Expand Down
3 changes: 2 additions & 1 deletion packages/components/src/toolbar-button/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ function ToolbarButton(
const accessibleToolbarState = useContext( ToolbarContext );

if ( ! accessibleToolbarState ) {
// This should be deprecated when <Toolbar __experimentalAccessibilityLabel="label">
// TODO
// This should be deprecated when <Toolbar accessibilityLabel="label">
// becomes stable.
return (
<ToolbarButtonContainer className={ containerClassName }>
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/toolbar-button/stories/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export const _default = () => {
const icon = text( 'Icon', 'wordpress' );

return (
<Toolbar __experimentalAccessibilityLabel="Example Toolbar">
<Toolbar accessibilityLabel="Example Toolbar">
<ToolbarButton icon={ icon } label={ label } />
</Toolbar>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import ToolbarItem from '../toolbar-item';

function ToolbarGroupCollapsed( { controls = [], ...props } ) {
// It'll contain state if `ToolbarGroup` is being used within
// `<Toolbar __experimentalAccessibilityLabel="label" />`
// `<Toolbar accessibilityLabel="label" />`
const accessibleToolbarState = useContext( ToolbarContext );

const renderDropdownMenu = ( toggleProps ) => (
Expand Down
17 changes: 7 additions & 10 deletions packages/components/src/toolbar/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,30 +21,27 @@ import ToolbarContainer from './toolbar-container';
*
* @param {Object} props Component props.
* @param {string} [props.className] Class to set on the container div.
* @param {string} [props.__experimentalAccessibilityLabel] ARIA label for toolbar container.
* @param {string} [props.accessibilityLabel] ARIA label for toolbar container.
* @param {Object} ref React Element ref.
*/
function Toolbar(
{ className, __experimentalAccessibilityLabel, ...props },
ref
) {
if ( __experimentalAccessibilityLabel ) {
function Toolbar( { className, accessibilityLabel, ...props }, ref ) {
if ( accessibilityLabel ) {
return (
<ToolbarContainer
// `ToolbarGroup` already uses components-toolbar for compatibility reasons
className={ classnames(
'components-accessible-toolbar',
className
) }
accessibilityLabel={ __experimentalAccessibilityLabel }
accessibilityLabel={ accessibilityLabel }
ref={ ref }
{ ...props }
/>
);
}
// When the __experimentalAccessibilityLabel prop is not passed, Toolbar will
// fallback to ToolbarGroup. This should be deprecated as soon as the new API
// gets stable.
// TODO
// When the accessibilityLabel prop is not passed, Toolbar will fallback to
// ToolbarGroup. This should be deprecated as soon as the new API gets stable.
// See https://github.com/WordPress/gutenberg/pull/20008#issuecomment-624503410
return <ToolbarGroup { ...props } className={ className } />;
}
Expand Down
7 changes: 2 additions & 5 deletions packages/components/src/toolbar/stories/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,7 @@ function InlineImageIcon() {
export const _default = () => {
return (
// id is required for server side rendering
<Toolbar
__experimentalAccessibilityLabel="Options"
id="options-toolbar"
>
<Toolbar accessibilityLabel="Options" id="options-toolbar">
<ToolbarGroup>
<ToolbarButton icon={ paragraph } label="Paragraph" />
</ToolbarGroup>
Expand Down Expand Up @@ -116,7 +113,7 @@ export const withoutGroup = () => {
return (
// id is required for server side rendering
<Toolbar
__experimentalAccessibilityLabel="Options"
accessibilityLabel="Options"
id="options-toolbar-without-group"
>
<ToolbarButton icon={ formatBold } label="Bold" isPressed />
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/toolbar/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ describe( 'Toolbar', () => {
describe( 'basic rendering', () => {
it( 'should render a toolbar with toolbar buttons', () => {
const { getByLabelText } = render(
<Toolbar __experimentalAccessibilityLabel="blocks">
<Toolbar accessibilityLabel="blocks">
<ToolbarButton label="control1" />
<ToolbarButton label="control2" />
</Toolbar>
Expand Down
4 changes: 2 additions & 2 deletions packages/e2e-tests/specs/editor/various/is-typing.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ describe( 'isTyping', () => {
it( 'should not close the dropdown when typing in it', async () => {
// Adds a Dropdown with an input to all blocks
await page.evaluate( () => {
const { Dropdown, Button, Fill } = wp.components;
const { Dropdown, ToolbarButton, Fill } = wp.components;
const { createElement: el, Fragment } = wp.element;
function AddDropdown( BlockListBlock ) {
return ( props ) => {
Expand All @@ -56,7 +56,7 @@ describe( 'isTyping', () => {
el( Dropdown, {
renderToggle: ( { onToggle } ) =>
el(
Button,
ToolbarButton,
{
onClick: onToggle,
className: 'dropdown-open',
Expand Down