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

Edit Mode: Prevent editable text selection on first click #65702

Open
wants to merge 8 commits into
base: trunk
Choose a base branch
from
Open
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 @@ -69,7 +69,6 @@ export function PrivateBlockToolbar( {
hasParentPattern,
hasContentOnlyLocking,
showShuffleButton,
showSlots,
showGroupButtons,
showLockButtons,
} = useSelect( ( select ) => {
Expand Down Expand Up @@ -138,7 +137,6 @@ export function PrivateBlockToolbar( {
hasParentPattern: _hasParentPattern,
hasContentOnlyLocking: _hasTemplateLock,
showShuffleButton: isZoomOut(),
showSlots: ! isZoomOut(),
showGroupButtons: ! isZoomOut(),
showLockButtons: ! isZoomOut(),
};
Expand Down Expand Up @@ -227,7 +225,7 @@ export function PrivateBlockToolbar( {
/>
</ToolbarGroup>
) }
{ shouldShowVisualToolbar && showSlots && (
{ shouldShowVisualToolbar && (
<>
<BlockControls.Slot
group="parent"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,29 @@ import { unlock } from '../../lock-unlock';
function ZoomOutModeInserters() {
const [ isReady, setIsReady ] = useState( false );
const {
hasSelection,
selectedSectionClientId,
blockOrder,
setInserterIsOpened,
sectionRootClientId,
selectedBlockClientId,
} = useSelect( ( select ) => {
const {
getSettings,
getBlockOrder,
getSelectionStart,
getSelectedBlockClientId,
getSectionRootClientId,
isSectionBlock,
getParentSectionBlock,
} = unlock( select( blockEditorStore ) );

const root = getSectionRootClientId();
const selectionBlockClientId = getSelectedBlockClientId();
const _selectedSectionClientId =
! selectionBlockClientId || isSectionBlock( selectionBlockClientId )
? selectionBlockClientId
: getParentSectionBlock( selectionBlockClientId );

return {
hasSelection: !! getSelectionStart().clientId,
selectedSectionClientId: _selectedSectionClientId,
blockOrder: getBlockOrder( root ),
sectionRootClientId: root,
setInserterIsOpened:
Expand All @@ -54,13 +59,13 @@ function ZoomOutModeInserters() {
};
}, [] );

if ( ! isReady || ! hasSelection ) {
if ( ! isReady || ! selectedSectionClientId ) {
return null;
}

const previousClientId = selectedBlockClientId;
const previousClientId = selectedSectionClientId;
const index = blockOrder.findIndex(
( clientId ) => selectedBlockClientId === clientId
( clientId ) => selectedSectionClientId === clientId
);
const nextClientId = blockOrder[ index + 1 ];

Expand Down
6 changes: 0 additions & 6 deletions packages/block-editor/src/components/inner-blocks/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,11 @@ function UncontrolledInnerBlocks( props ) {
layout,
name,
blockType,
parentLock,
defaultLayout,
} = props;

useNestedSettingsUpdate(
clientId,
parentLock,
allowedBlocks,
prioritizedInserterBlocks,
defaultBlock,
Expand Down Expand Up @@ -196,7 +194,6 @@ export function useInnerBlocksProps( props = {}, options = {} ) {
const {
getBlockName,
isZoomOut,
getTemplateLock,
getBlockRootClientId,
getBlockEditingMode,
getBlockSettings,
Expand Down Expand Up @@ -233,7 +230,6 @@ export function useInnerBlocksProps( props = {}, options = {} ) {
),
name: blockName,
blockType: getBlockType( blockName ),
parentLock: getTemplateLock( parentClientId ),
parentClientId,
isDropZoneDisabled: _isDropZoneDisabled,
defaultLayout,
Expand All @@ -245,7 +241,6 @@ export function useInnerBlocksProps( props = {}, options = {} ) {
__experimentalCaptureToolbars,
name,
blockType,
parentLock,
parentClientId,
isDropZoneDisabled,
defaultLayout,
Expand All @@ -272,7 +267,6 @@ export function useInnerBlocksProps( props = {}, options = {} ) {
layout,
name,
blockType,
parentLock,
defaultLayout,
...options,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,21 +105,18 @@ function UncontrolledInnerBlocks( props ) {

const context = useBlockContext( clientId );

const { nestingLevel, parentLock } = useSelect(
const { nestingLevel } = useSelect(
( select ) => {
const { getBlockParents, getTemplateLock, getBlockRootClientId } =
select( blockEditorStore );
const { getBlockParents } = select( blockEditorStore );
return {
nestingLevel: getBlockParents( clientId )?.length,
parentLock: getTemplateLock( getBlockRootClientId( clientId ) ),
};
},
[ clientId ]
);

useNestedSettingsUpdate(
clientId,
parentLock,
allowedBlocks,
prioritizedInserterBlocks,
defaultBlock,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ function useShallowMemo( value ) {
* came from props.
*
* @param {string} clientId The client ID of the block to update.
* @param {string} parentLock
* @param {string[]} allowedBlocks An array of block names which are permitted
* in inner blocks.
* @param {string[]} prioritizedInserterBlocks Block names and/or block variations to be prioritized in the inserter, in the format {blockName}/{variationName}.
Expand All @@ -63,7 +62,6 @@ function useShallowMemo( value ) {
*/
export default function useNestedSettingsUpdate(
clientId,
parentLock,
Copy link
Member

Choose a reason for hiding this comment

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

Just curious: why is this no longer changes and what required the changes to the selectors?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Basically I did a refactor to how "templateLock" works. Instead of doing the "inheritance" of template lock when setting the templateLock in the state, I moved the inheritance to the getTemplateLock selector instead.

The reason is that we want to know precisely what container have an explicit templateLock and what container inherit templateLock. Because the ones that have an explicit "contentOnly" templateLock are considered sections but not the ones within them.

Copy link
Member

Choose a reason for hiding this comment

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

I think at some point we may want to investigate these selectors (and the other few similar recursive selectors), I suspect they're not performant, which can be seen in the difference between the "preview" block selector and the normal one.

allowedBlocks,
prioritizedInserterBlocks,
defaultBlock,
Expand All @@ -90,16 +88,11 @@ export default function useNestedSettingsUpdate(
prioritizedInserterBlocks
);

const _templateLock =
templateLock === undefined || parentLock === 'contentOnly'
? parentLock
: templateLock;

useLayoutEffect( () => {
const newSettings = {
allowedBlocks: _allowedBlocks,
prioritizedInserterBlocks: _prioritizedInserterBlocks,
templateLock: _templateLock,
templateLock,
};

// These values are not defined for RN, so only include them if they
Expand Down Expand Up @@ -176,7 +169,7 @@ export default function useNestedSettingsUpdate(
clientId,
_allowedBlocks,
_prioritizedInserterBlocks,
_templateLock,
templateLock,
defaultBlock,
directInsert,
__experimentalDefaultBlock,
Expand Down
12 changes: 9 additions & 3 deletions packages/block-editor/src/store/private-selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ import {
getBlockName,
getTemplateLock,
getClientIdsWithDescendants,
isNavigationMode,
getBlockRootClientId,
__unstableGetEditorMode,
getBlockListSettings,
} from './selectors';
import {
checkAllowListRecursive,
Expand Down Expand Up @@ -522,8 +523,13 @@ export function isSectionBlock( state, clientId ) {
const sectionClientIds = getBlockOrder( state, sectionRootClientId );
return (
getBlockName( state, clientId ) === 'core/block' ||
getTemplateLock( state, clientId ) === 'contentOnly' ||
( isNavigationMode( state ) && sectionClientIds.includes( clientId ) )
// This is different than getTemplateLock
// because children of sections are not sections automatically.
getBlockListSettings( state, clientId )?.templateLock ===
'contentOnly' ||
( ( __unstableGetEditorMode( state ) === 'navigation' ||
isZoomOut( state ) ) &&
sectionClientIds.includes( clientId ) )
);
}

Expand Down
70 changes: 22 additions & 48 deletions packages/block-editor/src/store/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -1572,7 +1572,17 @@ export function getTemplateLock( state, rootClientId ) {
return state.settings.templateLock ?? false;
}

return getBlockListSettings( state, rootClientId )?.templateLock ?? false;
const currentLock = getBlockListSettings(
state,
rootClientId
)?.templateLock;
if ( currentLock !== undefined ) {
return currentLock;
}
return getTemplateLock(
state,
getBlockRootClientId( state, rootClientId )
);
}

/**
Expand Down Expand Up @@ -2934,36 +2944,21 @@ export const __unstableGetVisibleBlocks = createSelector(
);

export function __unstableHasActiveBlockOverlayActive( state, clientId ) {
// Prevent overlay on blocks with a non-default editing mode. If the mdoe is
// 'disabled' then the overlay is redundant since the block can't be
// selected. If the mode is 'contentOnly' then the overlay is redundant
// since there will be no controls to interact with once selected.
if ( getBlockEditingMode( state, clientId ) !== 'default' ) {
return false;
}

// If the block editing is locked, the block overlay is always active.
if ( ! canEditBlock( state, clientId ) ) {
return true;
}

// In zoom-out mode, the block overlay is always active for section level blocks.
if ( isZoomOut( state ) ) {
const sectionRootClientId = getSectionRootClientId( state );
if ( sectionRootClientId ) {
const sectionClientIds = getBlockOrder(
state,
sectionRootClientId
);
if ( sectionClientIds?.includes( clientId ) ) {
return true;
}
} else if ( clientId && ! getBlockRootClientId( state, clientId ) ) {
return true;
}
// Section blocks need to be selected first before being able to select their children.
if (
isSectionBlock( state, clientId ) &&
! isBlockSelected( state, clientId ) &&
! hasSelectedInnerBlock( state, clientId, true )
) {
draganescu marked this conversation as resolved.
Show resolved Hide resolved
return true;
}

// In navigation mode, the block overlay is active when the block is not
// For sections, the block overlay is active when the block is not
// selected (and doesn't contain a selected child). The same behavior is
// also enabled in all modes for blocks that have controlled children
// (reusable block, template part, navigation), unless explicitly disabled
Expand All @@ -2975,7 +2970,8 @@ export function __unstableHasActiveBlockOverlayActive( state, clientId ) {
);
const shouldEnableIfUnselected = blockSupportDisable
? false
: areInnerBlocksControlled( state, clientId );
: areInnerBlocksControlled( state, clientId ) &&
clientId !== getSectionRootClientId( state, clientId );

return (
shouldEnableIfUnselected &&
Expand Down Expand Up @@ -3047,30 +3043,8 @@ export const getBlockEditingMode = createRegistrySelector(
// In zoom-out mode, override the behavior set by
// __unstableSetBlockEditingMode to only allow editing the top-level
// sections.
if ( isZoomOut( state ) ) {
const sectionRootClientId = getSectionRootClientId( state );

if ( clientId === '' /* ROOT_CONTAINER_CLIENT_ID */ ) {
return sectionRootClientId ? 'disabled' : 'contentOnly';
}
if ( clientId === sectionRootClientId ) {
return 'contentOnly';
}
const sectionsClientIds = getBlockOrder(
state,
sectionRootClientId
);

// Sections are always contentOnly.
if ( sectionsClientIds?.includes( clientId ) ) {
return 'contentOnly';
}

return 'disabled';
}

const editorMode = __unstableGetEditorMode( state );
if ( editorMode === 'navigation' ) {
if ( editorMode === 'navigation' || isZoomOut( state ) ) {
const sectionRootClientId = getSectionRootClientId( state );

// The root section is "default mode"
Expand Down
33 changes: 29 additions & 4 deletions packages/block-editor/src/store/test/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -3830,28 +3830,45 @@ describe( 'selectors', () => {

it( 'should return false if the specified clientId was not found', () => {
const state = {
settings: { templateLock: 'all' },
settings: {},
blockListSettings: {
chicken: {
templateLock: 'insert',
},
ribs: {},
},
blocks: {
parents: new Map(
Object.entries( {
chicken: '',
ribs: '',
} )
),
},
};

expect( getTemplateLock( state, 'ribs' ) ).toBe( false );
} );

it( 'should return false if template lock was not set on the specified block', () => {
it( 'should return the parent lock if the specified clientId was not found', () => {
const state = {
settings: { templateLock: 'all' },
blockListSettings: {
chicken: {
test: 'tes1t',
templateLock: 'insert',
},
},
blocks: {
parents: new Map(
Object.entries( {
chicken: '',
ribs: '',
} )
),
},
};

expect( getTemplateLock( state, 'chicken' ) ).toBe( false );
expect( getTemplateLock( state, 'ribs' ) ).toBe( 'all' );
} );

it( 'should return the template lock for the specified clientId', () => {
Expand All @@ -3862,6 +3879,14 @@ describe( 'selectors', () => {
templateLock: 'insert',
},
},
blocks: {
parents: new Map(
Object.entries( {
chicken: '',
ribs: '',
} )
),
},
};

expect( getTemplateLock( state, 'chicken' ) ).toBe( 'insert' );
Expand Down
Loading
Loading