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

Make the shortcut provider optional #53942

Closed
wants to merge 11 commits into from
Closed

Make the shortcut provider optional #53942

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
a7151ed
Make the shortcut provider optional
youknowriad Aug 25, 2023
88b69b2
Bubble keydown events
youknowriad Aug 25, 2023
ab7c07c
Prevent double region navigation
youknowriad Aug 25, 2023
2a6247c
Prevent bubbling when navigation mode is handled
youknowriad Aug 25, 2023
a94eb2d
Wait to register the event handler
youknowriad Aug 28, 2023
0138f13
SSR support for keyboard shortcuts
youknowriad Aug 29, 2023
93537de
Small improvements
youknowriad Aug 29, 2023
82a856b
Small improvements
youknowriad Aug 29, 2023
687c653
Small update
youknowriad Aug 29, 2023
96d4684
Fix e2e tests
youknowriad Aug 29, 2023
bd15218
Try scoped useShortcut
youknowriad Aug 30, 2023
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
2 changes: 1 addition & 1 deletion packages/block-editor/src/components/iframe/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ function bubbleEvents( doc ) {
}
}

const eventTypes = [ 'dragover', 'mousemove' ];
const eventTypes = [ 'dragover', 'mousemove', 'keydown' ];
Copy link
Member

Choose a reason for hiding this comment

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

We should definitely not add more event names to this list. Ideally we should removing this bubbling entirely.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we're totally against this, the alternative would be to add a ShortcutProvider to the Iframe component to catch the keyboard shortcuts events. The problem though is that right now ShortcutProvider also serves as a registry for event handlers useShortcut calls, so we need to have two kind of providers:

  • One that keeps track of active event handlers.
  • One that only catches event and try to match them with active event handlers.


for ( const name of eventTypes ) {
doc.addEventListener( name, bubbleEvent );
Expand Down
1 change: 1 addition & 0 deletions packages/block-editor/src/components/writing-flow/use-tab-nav.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ export default function useTabNav() {

if ( event.keyCode === ESCAPE && ! hasMultiSelection() ) {
event.preventDefault();
event.stopPropagation();
setNavigationMode( true );
return;
}
Expand Down
3 changes: 2 additions & 1 deletion packages/components/src/higher-order/navigate-regions/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ export function useNavigateRegions( shortcuts: Shortcuts = defaultShortcuts ) {
nextIndex = nextIndex === regions.length ? 0 : nextIndex;
nextRegion = regions[ nextIndex ];
}

nextRegion.focus();
setIsFocusingRegions( true );
}
Expand Down Expand Up @@ -99,12 +98,14 @@ export function useNavigateRegions( shortcuts: Shortcuts = defaultShortcuts ) {
return isKeyboardEvent[ modifier ]( event, character );
} )
) {
event.stopPropagation();
focusRegion( -1 );
} else if (
shortcuts.next.some( ( { modifier, character } ) => {
return isKeyboardEvent[ modifier ]( event, character );
} )
) {
event.stopPropagation();
focusRegion( 1 );
}
},
Expand Down
28 changes: 11 additions & 17 deletions packages/customize-widgets/src/components/customize-widgets/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
*/
import { useState, useEffect, useRef, createPortal } from '@wordpress/element';
import { SlotFillProvider, Popover } from '@wordpress/components';
import { ShortcutProvider } from '@wordpress/keyboard-shortcuts';

/**
* Internal dependencies
Expand Down Expand Up @@ -68,21 +67,16 @@ export default function CustomizeWidgets( {
);

return (
<ShortcutProvider>
<SlotFillProvider>
<SidebarControls
sidebarControls={ sidebarControls }
activeSidebarControl={ activeSidebarControl }
>
<FocusControl
api={ api }
sidebarControls={ sidebarControls }
>
{ activeSidebar }
{ popover }
</FocusControl>
</SidebarControls>
</SlotFillProvider>
</ShortcutProvider>
<SlotFillProvider>
<SidebarControls
sidebarControls={ sidebarControls }
activeSidebarControl={ activeSidebarControl }
>
<FocusControl api={ api } sidebarControls={ sidebarControls }>
{ activeSidebar }
{ popover }
</FocusControl>
</SidebarControls>
</SlotFillProvider>
);
}
9 changes: 4 additions & 5 deletions packages/edit-post/src/components/keyboard-shortcut-help-modal/test/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { render, screen } from '@testing-library/react';
* WordPress dependencies
*/
import { EditorKeyboardShortcutsRegister } from '@wordpress/editor';
import { ShortcutProvider } from '@wordpress/keyboard-shortcuts';

/**
* Internal dependencies
Expand All @@ -19,10 +18,10 @@ const noop = () => {};
describe( 'KeyboardShortcutHelpModal', () => {
it( 'should match snapshot when the modal is active', () => {
render(
<ShortcutProvider>
<>
<EditorKeyboardShortcutsRegister />
<KeyboardShortcutHelpModal isModalActive toggleModal={ noop } />
</ShortcutProvider>
</>
);

expect(
Expand All @@ -34,13 +33,13 @@ describe( 'KeyboardShortcutHelpModal', () => {

it( 'should not render the modal when inactive', () => {
render(
<ShortcutProvider>
<>
<EditorKeyboardShortcutsRegister />
<KeyboardShortcutHelpModal
isModalActive={ false }
toggleModal={ noop }
/>
</ShortcutProvider>
</>
);

expect(
Expand Down
37 changes: 17 additions & 20 deletions packages/edit-post/src/editor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import {
import { useMemo } from '@wordpress/element';
import { SlotFillProvider } from '@wordpress/components';
import { store as coreStore } from '@wordpress/core-data';
import { ShortcutProvider } from '@wordpress/keyboard-shortcuts';
import { store as preferencesStore } from '@wordpress/preferences';
import { CommandMenu } from '@wordpress/commands';
import { privateApis as coreCommandsPrivateApis } from '@wordpress/core-commands';
Expand Down Expand Up @@ -156,25 +155,23 @@ function Editor( { postId, postType, settings, initialEdits, ...props } ) {
}

return (
<ShortcutProvider>
<SlotFillProvider>
<ExperimentalEditorProvider
settings={ editorSettings }
post={ post }
initialEdits={ initialEdits }
useSubRegistry={ false }
__unstableTemplate={ isTemplateMode ? template : undefined }
{ ...props }
>
<ErrorBoundary>
<CommandMenu />
<EditorInitialization postId={ postId } />
<Layout />
</ErrorBoundary>
<PostLockedModal />
</ExperimentalEditorProvider>
</SlotFillProvider>
</ShortcutProvider>
<SlotFillProvider>
<ExperimentalEditorProvider
settings={ editorSettings }
post={ post }
initialEdits={ initialEdits }
useSubRegistry={ false }
__unstableTemplate={ isTemplateMode ? template : undefined }
{ ...props }
>
<ErrorBoundary>
<CommandMenu />
<EditorInitialization postId={ postId } />
<Layout />
</ErrorBoundary>
<PostLockedModal />
</ExperimentalEditorProvider>
</SlotFillProvider>
);
}

Expand Down
21 changes: 9 additions & 12 deletions packages/edit-site/src/components/app/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
*/
import { SlotFillProvider } from '@wordpress/components';
import { UnsavedChangesWarning } from '@wordpress/editor';
import { ShortcutProvider } from '@wordpress/keyboard-shortcuts';
import { store as noticesStore } from '@wordpress/notices';
import { useDispatch } from '@wordpress/data';
import { __, sprintf } from '@wordpress/i18n';
Expand Down Expand Up @@ -35,16 +34,14 @@ export default function App() {
}

return (
<ShortcutProvider style={ { height: '100%' } }>
Copy link
Member

Choose a reason for hiding this comment

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

It seems like this code was relying on ShortcutProvider having a div. Not sure if we should not add a div, but if things work I guess we can keep this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I think this height was there to ensure the editor was full height and the removal of the height is probably going to have the same effect. Of course checking that the site editor is still full height.

<SlotFillProvider>
<GlobalStylesProvider>
<UnsavedChangesWarning />
<RouterProvider>
<Layout />
<PluginArea onError={ onPluginAreaError } />
</RouterProvider>
</GlobalStylesProvider>
</SlotFillProvider>
</ShortcutProvider>
<SlotFillProvider>
<GlobalStylesProvider>
<UnsavedChangesWarning />
<RouterProvider>
<Layout />
<PluginArea onError={ onPluginAreaError } />
</RouterProvider>
</GlobalStylesProvider>
</SlotFillProvider>
);
}
29 changes: 13 additions & 16 deletions packages/edit-widgets/src/components/widget-areas-block-editor-provider/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import {
privateApis as blockEditorPrivateApis,
} from '@wordpress/block-editor';
import { privateApis as editPatternsPrivateApis } from '@wordpress/patterns';
import { ShortcutProvider } from '@wordpress/keyboard-shortcuts';
import { store as preferencesStore } from '@wordpress/preferences';

/**
Expand Down Expand Up @@ -98,21 +97,19 @@ export default function WidgetAreasBlockEditorProvider( {
);

return (
<ShortcutProvider>
<SlotFillProvider>
<KeyboardShortcuts.Register />
<SlotFillProvider>
<ExperimentalBlockEditorProvider
value={ blocks }
onInput={ onInput }
onChange={ onChange }
settings={ settings }
useSubRegistry={ false }
{ ...props }
>
<CopyHandler>{ children }</CopyHandler>
<PatternsMenuItems rootClientId={ widgetAreaId } />
</ExperimentalBlockEditorProvider>
</SlotFillProvider>
</ShortcutProvider>
<ExperimentalBlockEditorProvider
value={ blocks }
onInput={ onInput }
onChange={ onChange }
settings={ settings }
useSubRegistry={ false }
{ ...props }
>
<CopyHandler>{ children }</CopyHandler>
<PatternsMenuItems rootClientId={ widgetAreaId } />
</ExperimentalBlockEditorProvider>
</SlotFillProvider>
);
}
2 changes: 1 addition & 1 deletion packages/keyboard-shortcuts/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ _This package assumes that your code will run in an **ES2015+** environment. If

### ShortcutProvider

Handles callbacks added to context by `useShortcut`.
Handles callbacks added to context by `useShortcut`. Adding a provider allows to register contextual shortcuts that are only active when a certain part of the UI is focused.

_Parameters_

Expand Down
2 changes: 2 additions & 0 deletions packages/keyboard-shortcuts/src/components/shortcut-provider.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ const { Provider } = context;

/**
* Handles callbacks added to context by `useShortcut`.
* Adding a provider allows to register contextual shortcuts
* that are only active when a certain part of the UI is focused.
*
* @param {Object} props Props to pass to `div`.
*
Expand Down
11 changes: 10 additions & 1 deletion packages/keyboard-shortcuts/src/context.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,13 @@
*/
import { createContext } from '@wordpress/element';

export const context = createContext();
const globalShortcuts = { current: new Set() };
document.addEventListener( 'DOMContentLoaded', () => {
document.body.addEventListener( 'keydown', ( event ) => {
for ( const keyboardShortcut of globalShortcuts.current ) {
keyboardShortcut( event );
}
} );
} );
youknowriad marked this conversation as resolved.
Show resolved Hide resolved

export const context = createContext( globalShortcuts );
39 changes: 18 additions & 21 deletions storybook/stories/playground/index.story.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {
WritingFlow,
} from '@wordpress/block-editor';
import { registerCoreBlocks } from '@wordpress/block-library';
import { ShortcutProvider } from '@wordpress/keyboard-shortcuts';
import '@wordpress/format-library';

/**
Expand All @@ -35,26 +34,24 @@ function App() {

return (
<div className="playground">
<ShortcutProvider>
<BlockEditorProvider
value={ blocks }
onInput={ updateBlocks }
onChange={ updateBlocks }
>
<div className="playground__sidebar">
<BlockInspector />
</div>
<div className="playground__content">
<BlockTools>
<div className="editor-styles-wrapper">
<WritingFlow>
<BlockList />
</WritingFlow>
</div>
</BlockTools>
</div>
</BlockEditorProvider>
</ShortcutProvider>
<BlockEditorProvider
value={ blocks }
onInput={ updateBlocks }
onChange={ updateBlocks }
>
<div className="playground__sidebar">
<BlockInspector />
</div>
<div className="playground__content">
<BlockTools>
<div className="editor-styles-wrapper">
<WritingFlow>
<BlockList />
</WritingFlow>
</div>
</BlockTools>
</div>
</BlockEditorProvider>
</div>
);
}
Expand Down
29 changes: 13 additions & 16 deletions test/integration/helpers/integration-test-editor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import {
WritingFlow,
} from '@wordpress/block-editor';
import { registerCoreBlocks } from '@wordpress/block-library';
import { ShortcutProvider } from '@wordpress/keyboard-shortcuts';
import '@wordpress/format-library';
import {
createBlock,
Expand Down Expand Up @@ -66,21 +65,19 @@ export function Editor( { testBlocks, settings = {} } ) {
}, [] );

return (
<ShortcutProvider>
<BlockEditorProvider
value={ currentBlocks }
onInput={ updateBlocks }
onChange={ updateBlocks }
settings={ settings }
>
<BlockInspector />
<BlockTools>
<WritingFlow>
<BlockList />
</WritingFlow>
</BlockTools>
</BlockEditorProvider>
</ShortcutProvider>
<BlockEditorProvider
value={ currentBlocks }
onInput={ updateBlocks }
onChange={ updateBlocks }
settings={ settings }
>
<BlockInspector />
<BlockTools>
<WritingFlow>
<BlockList />
</WritingFlow>
</BlockTools>
</BlockEditorProvider>
);
}

Expand Down
Loading