Skip to content

Commit

Permalink
Fix toggle list view shortcut when showListViewByDefault is enabled.
Browse files Browse the repository at this point in the history
  • Loading branch information
afercia committed Feb 19, 2025
1 parent d1a2fbc commit a90b76b
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,15 @@ export default function EditorKeyboardShortcuts() {
savePost();
} );

// Only opens the list view. Other functionality for this shortcut happens in the rendered sidebar.
// Only opens the list view. Other functionality for this shortcut happens
// in the rendered sidebar. When the `showListViewByDefault` preference is
// enabled, the sidebar is rendered by default. As such, we need to prevent
// the callback from running twice by using an additional check for
// `event.defaultPrevented` otherwise the shortcut:
// 1. It will first be invoked in the sidebar, thus closing it.
// 2. It will then run again here, reopening the sidebar unexpectedly.
useShortcut( 'core/editor/toggle-list-view', ( event ) => {
if ( ! isListViewOpened() ) {
if ( ! isListViewOpened() && ! event.defaultPrevented ) {
event.preventDefault();
setIsListViewOpened( true );
}
Expand Down
46 changes: 31 additions & 15 deletions packages/editor/src/components/list-view-sidebar/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ export default function ListViewSidebar() {
const closeOnEscape = useCallback(
( event ) => {
if ( event.keyCode === ESCAPE && ! event.defaultPrevented ) {
// Always use `event.preventDefault` before calling `closeListView`.
// This is important to prevent the `core/editor/toggle-list-view`
// shortcut callback from being twice.
event.preventDefault();
closeListView();
}
Expand Down Expand Up @@ -148,23 +151,36 @@ export default function ListViewSidebar() {
}
}

const handleToggleListViewShortcut = useCallback( () => {
// If the sidebar has focus, it is safe to close.
if (
sidebarRef.current.contains(
sidebarRef.current.ownerDocument.activeElement
)
) {
closeListView();
} else {
// If the list view or outline does not have focus, focus should be moved to it.
handleSidebarFocus( tab );
}
}, [ closeListView, tab ] );
const handleToggleListViewShortcut = useCallback(
( event ) => {
// If the sidebar has focus, it is safe to close.
if (
sidebarRef.current.contains(
sidebarRef.current.ownerDocument.activeElement
)
) {
// Always use `event.preventDefault` before calling `closeListView`.
// This is important to prevent the `core/editor/toggle-list-view`
// shortcut callback from running twice.
event.preventDefault();
closeListView();
} else {
// If the list view or outline does not have focus, focus should be moved to it.
handleSidebarFocus( tab );
}
},
[ closeListView, tab ]
);

// This only fires when the sidebar is open because of the conditional rendering.
// It is the same shortcut to open but that is defined as a global shortcut and only fires when the sidebar is closed.
useShortcut( 'core/editor/toggle-list-view', handleToggleListViewShortcut );
// It is the same shortcut to open the sidebar but that is defined as a global
// shortcut. However, when the `showListViewByDefault` preference is enabled,
// the sidebar is open by default and the shortcut callback would be invoked
// twice (here and in the global shortcut). To prevent that, we pass the event
// for some additional logic in the global shortcut based on `event.defaultPrevented`.
useShortcut( 'core/editor/toggle-list-view', ( event ) => {
handleToggleListViewShortcut( event );
} );

return (
// eslint-disable-next-line jsx-a11y/no-static-element-interactions
Expand Down

0 comments on commit a90b76b

Please sign in to comment.