Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
820035d
Add useKeyboardShortcut prop to NavigableToolbar and useToolbarFocus
jeryj Apr 6, 2023
2c99327
Conditionally useKeyboardFocusShortcut in HeaderToolbar
jeryj Apr 6, 2023
3db06db
Remove accidentally added line break
jeryj Apr 6, 2023
87eabd8
Check all the conditions when a toolbar might be showing
jeryj Apr 6, 2023
b7e3091
Remove unnecesary line break
jeryj Apr 6, 2023
9222cc6
Change logic to use isBlockWithToolbarSelected instead of isUnmodifie…
jeryj Apr 7, 2023
d4ed2ba
Start on e2e tests to cover various scenarios
jeryj Apr 7, 2023
c05a396
e2e test: should focus the top level toolbar when in select mode
jeryj Apr 7, 2023
3146b4e
ede test: should focus the top level toolbar when on an empty block
jeryj Apr 7, 2023
ec44ad5
ede test: grouping into describe test sections
jeryj Apr 7, 2023
c201962
Scaffold top toolbar mode e2e tests
jeryj Apr 7, 2023
964d06d
e2e tests: Top Toolbar in edit mode
jeryj Apr 11, 2023
8050ffb
e2e tests: Top toolbar + select mode
jeryj Apr 11, 2023
a65fd0a
Refactor toggle fixed toolbar to playwright editor class
jeryj Apr 11, 2023
02cfb03
Refactor: consolodate e2e locators
jeryj Apr 11, 2023
abeb991
Refactor: Combine tests into fewer cases to save execution time
jeryj Apr 11, 2023
26d79a2
Use POM-style ToolbarUtils for e2e test
jeryj Apr 12, 2023
a4c4633
Improve naming of isFixed to setIsFixedToolbar
jeryj Apr 12, 2023
29ccd55
Improve naming to shouldUseKeyboardFocusShortcut
jeryj Apr 12, 2023
f51fb72
Simplify logic for setting isBlockWithToolbarSelected
jeryj Apr 12, 2023
6bfb00f
Rename toggleFixedToolbar util to setIsFixedToolbar
jeryj Apr 13, 2023
766ef57
Refactor shouldShowContextualToolbar and canFocusHiddenToolbar into u…
jeryj Apr 13, 2023
9657c9d
Export useShouldContextualToolbarShow
jeryj Apr 13, 2023
82784d6
Add hasClientId check, as we can't focus a block toolbar without a bl…
jeryj Apr 14, 2023
aede555
Implement useShouldContextualToolbarShow in header toolbar
jeryj Apr 14, 2023
21deaa2
Add tests for smaller viewports
jeryj Apr 14, 2023
47c6561
Update e2e tests to address new unified toolbar view
jeryj Apr 19, 2023
3eef424
Make useShouldContextualToolbarShow private
jeryj Apr 19, 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
Next Next commit
Add useKeyboardShortcut prop to NavigableToolbar and useToolbarFocus
The 'core/block-editor/focus-toolbar' shortcut (alt + F10) selects the active navigable toolbar's fi
rst tabbable item, but when there are multiple navigable toolbars, it places focus on all of the act
ive navigable toolbar components. This results in an odd state of two tooltips being open. This comm
it scaffolds the ability to disable the keyboard shortcut from applying.
  • Loading branch information
jeryj committed Apr 19, 2023
commit 820035dd61a85d74316c85a917267e056c3aea2b
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ function SelectedBlockPopover( {
},
[ clientId ]
);

const isLargeViewport = useViewportMatch( 'medium' );
const isToolbarForced = useRef( false );
const { stopTyping } = useDispatch( blockEditorStore );
Expand Down
15 changes: 12 additions & 3 deletions packages/block-editor/src/components/navigable-toolbar/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,8 @@ function useToolbarFocus(
focusOnMount,
isAccessibleToolbar,
defaultIndex,
onIndexChange
onIndexChange,
useKeyboardFocusShortcut
) {
// Make sure we don't use modified versions of this prop.
const [ initialFocusOnMount ] = useState( focusOnMount );
Expand All @@ -103,8 +104,14 @@ function useToolbarFocus(
focusFirstTabbableIn( ref.current );
}, [] );

const focusToolbarViaShortcut = () => {
if ( useKeyboardFocusShortcut ) {
focusToolbar();
}
};

// Focus on toolbar when pressing alt+F10 when the toolbar is visible.
useShortcut( 'core/block-editor/focus-toolbar', focusToolbar );
useShortcut( 'core/block-editor/focus-toolbar', focusToolbarViaShortcut );

useEffect( () => {
if ( initialFocusOnMount ) {
Expand Down Expand Up @@ -147,6 +154,7 @@ function useToolbarFocus(
function NavigableToolbar( {
children,
focusOnMount,
useKeyboardFocusShortcut = true,
Copy link
Member

Choose a reason for hiding this comment

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

Could we rename this to shouldUseKeyboardFocusShortcut as seen below? use* variable often suggests a hook in React and that might cause some confusion. shouuld* also has the benefit of suggesting the variable to be a boolean too.

__experimentalInitialIndex: initialIndex,
__experimentalOnIndexChange: onIndexChange,
...props
Expand All @@ -159,7 +167,8 @@ function NavigableToolbar( {
focusOnMount,
isAccessibleToolbar,
initialIndex,
onIndexChange
onIndexChange,
useKeyboardFocusShortcut
);

if ( isAccessibleToolbar ) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ function HeaderToolbar() {
const { getEditorMode, isFeatureActive, isListViewOpened } =
select( editPostStore );
const { getShortcutRepresentation } = select( keyboardShortcutsStore );

return {
// This setting (richEditingEnabled) should not live in the block editor's setting.
isInserterEnabled:
Expand All @@ -64,6 +63,9 @@ function HeaderToolbar() {
),
};
}, [] );

// TODO: Determine if there is a block toolbar active, as we need to change the shortcut behavior if it is
const isBlockToolbarActive = true;
const isLargeViewport = useViewportMatch( 'medium' );
const isWideViewport = useViewportMatch( 'wide' );

Expand Down Expand Up @@ -114,6 +116,7 @@ function HeaderToolbar() {
<NavigableToolbar
className="edit-post-header-toolbar"
aria-label={ toolbarAriaLabel }
useKeyboardFocusShortcut={ ! isBlockToolbarActive }
>
<div className="edit-post-header-toolbar__left">
<ToolbarItem
Expand Down