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

Paragraph: store subscription for selected block only #56967

Merged
merged 1 commit into from
Dec 12, 2023
Merged

Paragraph: store subscription for selected block only #56967

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
85 changes: 49 additions & 36 deletions packages/block-library/src/paragraph/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,48 @@ function hasDropCapDisabled( align ) {
return align === ( isRTL() ? 'left' : 'right' ) || align === 'center';
}

function DropCapControl( { clientId, attributes, setAttributes } ) {
// Please do no add a useSelect call to the paragraph block unconditionaly.
// Every useSelect added to a (frequestly used) block will degrade the load
// and type bit. By moving it within InspectorControls, the subscription is
// now only added for the selected block(s).
const [ isDropCapFeatureEnabled ] = useSettings( 'typography.dropCap' );

if ( ! isDropCapFeatureEnabled ) {
return null;
}

const { align, dropCap } = attributes;

let helpText;
if ( hasDropCapDisabled( align ) ) {
helpText = __( 'Not available for aligned text.' );
} else if ( dropCap ) {
helpText = __( 'Showing large initial letter.' );
} else {
helpText = __( 'Toggle to show a large initial letter.' );
}

return (
<ToolsPanelItem
hasValue={ () => !! dropCap }
label={ __( 'Drop cap' ) }
onDeselect={ () => setAttributes( { dropCap: undefined } ) }
resetAllFilter={ () => ( { dropCap: undefined } ) }
panelId={ clientId }
>
<ToggleControl
__nextHasNoMarginBottom
label={ __( 'Drop cap' ) }
checked={ !! dropCap }
onChange={ () => setAttributes( { dropCap: ! dropCap } ) }
help={ helpText }
disabled={ hasDropCapDisabled( align ) ? true : false }
/>
</ToolsPanelItem>
);
}

function ParagraphBlock( {
attributes,
mergeBlocks,
Expand All @@ -58,7 +100,6 @@ function ParagraphBlock( {
clientId,
} ) {
const { align, content, direction, dropCap, placeholder } = attributes;
const [ isDropCapFeatureEnabled ] = useSettings( 'typography.dropCap' );
const blockProps = useBlockProps( {
ref: useOnEnter( { clientId, content } ),
className: classnames( {
Expand All @@ -68,15 +109,6 @@ function ParagraphBlock( {
style: { direction },
} );

let helpText;
if ( hasDropCapDisabled( align ) ) {
helpText = __( 'Not available for aligned text.' );
} else if ( dropCap ) {
helpText = __( 'Showing large initial letter.' );
} else {
helpText = __( 'Toggle to show a large initial letter.' );
}

return (
<>
<BlockControls group="block">
Expand All @@ -98,32 +130,13 @@ function ParagraphBlock( {
}
/>
</BlockControls>
{ isDropCapFeatureEnabled && (
<InspectorControls group="typography">
<ToolsPanelItem
hasValue={ () => !! dropCap }
label={ __( 'Drop cap' ) }
onDeselect={ () =>
setAttributes( { dropCap: undefined } )
}
resetAllFilter={ () => ( { dropCap: undefined } ) }
panelId={ clientId }
>
<ToggleControl
__nextHasNoMarginBottom
label={ __( 'Drop cap' ) }
checked={ !! dropCap }
onChange={ () =>
setAttributes( { dropCap: ! dropCap } )
}
help={ helpText }
disabled={
hasDropCapDisabled( align ) ? true : false
}
/>
</ToolsPanelItem>
</InspectorControls>
) }
<InspectorControls group="typography">
<DropCapControl
Copy link
Contributor

Choose a reason for hiding this comment

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

So you think the useSetting call is more expensive than rendering InspectorControls?

Copy link
Contributor

Choose a reason for hiding this comment

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

oh the useSetting is only done for the selected block now. Makes sense.

Copy link
Member Author

@ellatrix ellatrix Dec 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, because (1) drop cap is supported by default and (2) individual InspectorControls instances no longer make select calls but read from context instead, so they're much cheaper.

clientId={ clientId }
attributes={ attributes }
setAttributes={ setAttributes }
/>
</InspectorControls>
<RichText
identifier="content"
tagName="p"
Expand Down
Loading