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

List View: Allow right-click to open block settings dropdown, add editor setting #50273

Merged
merged 5 commits into from
Dec 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ function ListViewBlockSelectButton(
className,
block: { clientId },
onClick,
onContextMenu,
onMouseDown,
onToggleExpanded,
tabIndex,
onFocus,
Expand Down Expand Up @@ -237,7 +239,9 @@ function ListViewBlockSelectButton(
className
) }
onClick={ onClick }
onContextMenu={ onContextMenu }
onKeyDown={ onKeyDownHandler }
onMouseDown={ onMouseDown }
ref={ ref }
tabIndex={ tabIndex }
onFocus={ onFocus }
Expand Down
72 changes: 71 additions & 1 deletion packages/block-editor/src/components/list-view/block.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,13 @@ import {
} from '@wordpress/components';
import { useInstanceId } from '@wordpress/compose';
import { moreVertical } from '@wordpress/icons';
import { useState, useRef, useCallback, memo } from '@wordpress/element';
import {
useCallback,
useMemo,
useState,
useRef,
memo,
} from '@wordpress/element';
import { useDispatch, useSelect } from '@wordpress/data';
import { sprintf, __ } from '@wordpress/i18n';
import { ESCAPE } from '@wordpress/keycodes';
Expand Down Expand Up @@ -53,7 +59,9 @@ function ListViewBlock( {
} ) {
const cellRef = useRef( null );
const rowRef = useRef( null );
const settingsRef = useRef( null );
const [ isHovered, setIsHovered ] = useState( false );
const [ settingsAnchorRect, setSettingsAnchorRect ] = useState();

const { isLocked, canEdit } = useBlockLock( clientId );

Expand Down Expand Up @@ -82,6 +90,11 @@ function ListViewBlock( {
},
[ clientId ]
);
const allowRightClickOverrides = useSelect(
( select ) =>
select( blockEditorStore ).getSettings().allowRightClickOverrides,
[]
);

const showBlockActions =
// When a block hides its toolbar it also hides the block settings menu,
Expand Down Expand Up @@ -190,6 +203,56 @@ function ListViewBlock( {
[ clientId, expand, collapse, isExpanded ]
);

// Allow right-clicking an item in the List View to open up the block settings dropdown.
const onContextMenu = useCallback(
( event ) => {
if ( showBlockActions && allowRightClickOverrides ) {
settingsRef.current?.click();
// Ensure the position of the settings dropdown is at the cursor.
setSettingsAnchorRect(
new window.DOMRect( event.clientX, event.clientY, 0, 0 )
);
event.preventDefault();
}
},
[ allowRightClickOverrides, settingsRef, showBlockActions ]
);

const onMouseDown = useCallback(
( event ) => {
// Prevent right-click from focusing the block,
// because focus will be handled when opening the block settings dropdown.
if ( allowRightClickOverrides && event.button === 2 ) {
event.preventDefault();
}
},
[ allowRightClickOverrides ]
);

const settingsPopoverAnchor = useMemo( () => {
const { ownerDocument } = rowRef?.current || {};

// If no custom position is set, the settings dropdown will be anchored to the
// DropdownMenu toggle button.
if ( ! settingsAnchorRect || ! ownerDocument ) {
Copy link
Member

Choose a reason for hiding this comment

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

Is it that ownerDocument can never be falsy due to the fallback value of {}?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one's slightly subtle — ownerDocument is set via destructuring, so if the fallback value after the assignment operator is {} then ownerDocument will be undefined as it doesn't exist on {} 🙂

Copy link
Member

Choose a reason for hiding this comment

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

Oh right, sorry I read that line wrong. {} is the fallback for rowRef?.current 🤦🏻

return undefined;
}

// Position the settings dropdown at the cursor when right-clicking a block.
return {
ownerDocument,
getBoundingClientRect() {
return settingsAnchorRect;
},
};
}, [ settingsAnchorRect ] );

const clearSettingsAnchorRect = useCallback( () => {
// Clear the custom position for the settings dropdown so that it is restored back
// to being anchored to the DropdownMenu toggle button.
setSettingsAnchorRect( undefined );
}, [ setSettingsAnchorRect ] );

let colSpan;
if ( hasRenderedMovers ) {
colSpan = 2;
Expand Down Expand Up @@ -257,6 +320,8 @@ function ListViewBlock( {
<ListViewBlockContents
block={ block }
onClick={ selectEditorBlock }
onContextMenu={ onContextMenu }
onMouseDown={ onMouseDown }
onToggleExpanded={ toggleExpanded }
isSelected={ isSelected }
position={ position }
Expand Down Expand Up @@ -315,17 +380,22 @@ function ListViewBlock( {
<TreeGridCell
className={ listViewBlockSettingsClassName }
aria-selected={ !! isSelected }
ref={ settingsRef }
>
{ ( { ref, tabIndex, onFocus } ) => (
<BlockSettingsMenu
clientIds={ dropdownClientIds }
block={ block }
icon={ moreVertical }
label={ settingsAriaLabel }
popoverProps={ {
anchor: settingsPopoverAnchor, // Used to position the settings at the cursor on right-click.
} }
toggleProps={ {
ref,
className: 'block-editor-list-view-block__menu',
tabIndex,
onClick: clearSettingsAnchorRect,
onFocus,
} }
disableOpenOnArrowDown
Expand Down
9 changes: 9 additions & 0 deletions packages/edit-post/src/components/preferences-modal/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,15 @@ export default function EditPostPreferencesModal() {
label={ __( 'Show block breadcrumbs' ) }
/>
) }
<EnableFeature
featureName="allowRightClickOverrides"
help={ __(
'Allows contextual list view menus via right-click, overriding browser defaults.'
) }
label={ __(
'Allow right-click contextual menus'
) }
/>
</PreferencesModalSection>
<PreferencesModalSection
title={ __( 'Document settings' ) }
Expand Down
6 changes: 6 additions & 0 deletions packages/edit-post/src/editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ function Editor( { postId, postType, settings, initialEdits, ...props } ) {
const isLargeViewport = useViewportMatch( 'medium' );

const {
allowRightClickOverrides,
hasFixedToolbar,
focusMode,
isDistractionFree,
Expand Down Expand Up @@ -70,6 +71,9 @@ function Editor( { postId, postType, settings, initialEdits, ...props } ) {
const isViewable = getPostType( postType )?.viewable ?? false;
const canEditTemplate = canUser( 'create', 'templates' );
return {
allowRightClickOverrides: isFeatureActive(
'allowRightClickOverrides'
),
hasFixedToolbar:
isFeatureActive( 'fixedToolbar' ) || ! isLargeViewport,
focusMode: isFeatureActive( 'focusMode' ),
Expand Down Expand Up @@ -106,6 +110,7 @@ function Editor( { postId, postType, settings, initialEdits, ...props } ) {
focusMode,
isDistractionFree,
hasInlineToolbar,
allowRightClickOverrides,

// This is marked as experimental to give time for the quick inserter to mature.
__experimentalSetIsInserterOpened: setIsInserterOpened,
Expand Down Expand Up @@ -133,6 +138,7 @@ function Editor( { postId, postType, settings, initialEdits, ...props } ) {
return result;
}, [
settings,
allowRightClickOverrides,
hasFixedToolbar,
hasInlineToolbar,
focusMode,
Expand Down
1 change: 1 addition & 0 deletions packages/edit-post/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export function initializeEditor(
const root = createRoot( target );

dispatch( preferencesStore ).setDefaults( 'core/edit-post', {
allowRightClickOverrides: true,
editorMode: 'visual',
fixedToolbar: false,
fullscreenMode: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ export function useSpecificEditorSettings() {
const {
templateSlug,
focusMode,
allowRightClickOverrides,
isDistractionFree,
hasFixedToolbar,
keepCaretInsideBlock,
Expand Down Expand Up @@ -126,6 +127,10 @@ export function useSpecificEditorSettings() {
'core/edit-site',
'distractionFree'
),
allowRightClickOverrides: !! getPreference(
'core/edit-site',
'allowRightClickOverrides'
),
hasFixedToolbar:
!! getPreference( 'core/edit-site', 'fixedToolbar' ) ||
! isLargeViewport,
Expand All @@ -149,6 +154,7 @@ export function useSpecificEditorSettings() {
supportsTemplateMode: true,
__experimentalSetIsInserterOpened: setIsInserterOpened,
focusMode: canvasMode === 'view' && focusMode ? false : focusMode,
allowRightClickOverrides,
isDistractionFree,
hasFixedToolbar,
keepCaretInsideBlock,
Expand All @@ -162,6 +168,7 @@ export function useSpecificEditorSettings() {
settings,
setIsInserterOpened,
focusMode,
allowRightClickOverrides,
isDistractionFree,
hasFixedToolbar,
keepCaretInsideBlock,
Expand Down
7 changes: 7 additions & 0 deletions packages/edit-site/src/components/preferences-modal/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,13 @@ export default function EditSitePreferencesModal() {
) }
label={ __( 'Display block breadcrumbs' ) }
/>
<EnableFeature
featureName="allowRightClickOverrides"
Copy link
Member

Choose a reason for hiding this comment

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

To me, enableListViewRightClick or allowListViewContextualMenus or the equivalent specific descriptor might describe the feature. allowRightClickOverrides might be too generic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question! My thinking was to keep it generic so that if we wish to expand other areas to also have right click overrides, we don't wind up with a proliferation of allowRightClick settings in the preferences 🤔

help={ __(
'Allows contextual list view menus via right-click, overriding browser defaults.'
) }
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to communicate that it's specific to the List view here? I don't think it has effect anywhere else in the editor (?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, for the user facing help text, that's a great idea! Perhaps Allows contextual list view menus via right-click... or something like that.

label={ __( 'Allow right-click contextual menus' ) }
/>
</PreferencesModalSection>
),
},
Expand Down
1 change: 1 addition & 0 deletions packages/edit-site/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ export function initializeEditor( id, settings ) {
// We dispatch actions and update the store synchronously before rendering
// so that we won't trigger unnecessary re-renders with useEffect.
dispatch( preferencesStore ).setDefaults( 'core/edit-site', {
allowRightClickOverrides: true,
editorMode: 'visual',
fixedToolbar: false,
focusMode: false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ const BLOCK_EDITOR_SETTINGS = [
'__unstableGalleryWithImageBlocks',
'alignWide',
'allowedBlockTypes',
'allowRightClickOverrides',
'blockInspectorTabs',
'allowedMimeTypes',
'bodyPlaceholder',
Expand Down
25 changes: 23 additions & 2 deletions test/e2e/specs/editor/various/a11y.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,13 @@ test.describe( 'a11y (@firefox, @webkit)', () => {
page,
pageUtils,
} ) => {
// Note: this test depends on a particular viewport height to determine whether or not
// the modal content is scrollable. If this tests fails and needs to be debugged locally,
// double-check the viewport height when running locally versus in CI. Additionally,
// when adding or removing items from the preference menu, this test may need to be updated
// if the height of panels has changed. It would be good to find a more robust way to test
// this behavior.

// Open the top bar Options menu.
await page.click(
'role=region[name="Editor top bar"i] >> role=button[name="Options"i]'
Expand All @@ -145,6 +152,9 @@ test.describe( 'a11y (@firefox, @webkit)', () => {
const generalTab = preferencesModal.locator(
'role=tab[name="General"i]'
);
const accessibilityTab = preferencesModal.locator(
'role=tab[name="Accessibility"i]'
);
const blocksTab = preferencesModal.locator(
'role=tab[name="Blocks"i]'
);
Expand All @@ -165,9 +175,20 @@ test.describe( 'a11y (@firefox, @webkit)', () => {
await tab.focus();
}

// The General tab panel content is short and not scrollable.
// Check it's not focusable.
// The Accessibility tab is currently short and not scrollable.
// Check that it cannot be focused by tabbing. Note: this test depends
// on a particular viewport height to determine whether or not the
// modal content is scrollable. If additional Accessibility options are
// added, then eventually this test will fail.
// TODO: find a more robust way to test this behavior.
await clickAndFocusTab( generalTab );
// Navigate down to the Accessibility tab.
await pageUtils.pressKeys( 'ArrowDown', { times: 2 } );
// Check the Accessibility tab panel is visible.
await expect(
preferencesModal.locator( 'role=tabpanel[name="Accessibility"i]' )
).toBeVisible();
await expect( accessibilityTab ).toBeFocused();
await pageUtils.pressKeys( 'Shift+Tab' );
await expect( closeButton ).toBeFocused();
await pageUtils.pressKeys( 'Shift+Tab' );
Expand Down
Loading