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

Render non-editable preview of template part when user does not have capability to edit template part #60326

Merged
merged 2 commits into from
Apr 2, 2024
Merged

Render non-editable preview of template part when user does not have capability to edit template part #60326

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
654a021
render non editable preview of template part when user does not have …
fabiankaegy Mar 31, 2024
352aa9e
fix simplify logic in NonEditableTemplatePartPreview because in the v…
fabiankaegy Apr 1, 2024
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
100 changes: 98 additions & 2 deletions packages/block-library/src/template-part/edit/inner-blocks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* WordPress dependencies
*/
import { useEntityBlockEditor } from '@wordpress/core-data';
import { useEntityBlockEditor, store as coreStore } from '@wordpress/core-data';
import {
InnerBlocks,
useInnerBlocksProps,
Expand All @@ -10,6 +10,8 @@ import {
useBlockEditingMode,
} from '@wordpress/block-editor';
import { useSelect } from '@wordpress/data';
import { useMemo } from '@wordpress/element';
import { parse } from '@wordpress/blocks';

function useRenderAppender( hasInnerBlocks ) {
const blockEditingMode = useBlockEditingMode();
Expand All @@ -36,7 +38,62 @@ function useLayout( layout ) {
}
}

export default function TemplatePartInnerBlocks( {
function NonEditableTemplatePartPreview( {
postId: id,
layout,
tagName: TagName,
blockProps,
} ) {
useBlockEditingMode( 'disabled' );
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we replace this with templateLock: 'all' ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried that first. But template lock all only prevents moving / deleting / inserting blocks. Not changing the attributes of any of the blocks that are already in there.

Copy link
Contributor

Choose a reason for hiding this comment

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

I wish we could make another "lock" level rather than a hook like that but that's fine for now (declarative means less bugs in general)


const { content, editedBlocks } = useSelect(
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the difference between all the code that is here and the useEntityBlockEditor hook?

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually I missed something here. The difference should be that the getEntityRecord call here uses context: 'view' which the useEntityBlockEditor hook does not support

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add a "readonly" option to the useEntityBlockEditor hook to address this?

Copy link
Member Author

Choose a reason for hiding this comment

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

@youknowriad updated this in 352aa9e. This removes a lot of the complexity because after testing a bit more it seems we can pretty much omit edits for user roles that cannot edit the template.

This also fixes the request to use the view context as I mentioned

Copy link
Contributor

Choose a reason for hiding this comment

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

That's better but I still wonder whether we should just reuse useEntityBlockEditor with an option? Wdyt?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm yeah, I hear your concerns. I'll have to see whether I can update the useEntityBlockEditor hook without adding too much complexity. It's not obvious to me how to best approach it right now but will experiment a bit.

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay I'm having a hard time modifying the useEntityBlockEditor hook. We can't call any of the hooks there conditionally but want to avoid doing a bunch of the work in case of the readonly mode.

@youknowriad you let me know how you want to handle this. Do you want to push something to this branch? Do you want me to merge as is and then simplify it later? Open to whatever you think makes the most sense

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's merge as is. It doesn't introduce any API and address a need. We can consider the refactoring later if needed.

( select ) => {
if ( ! id ) {
return {};
}
const { getEditedEntityRecord } = select( coreStore );
const editedRecord = getEditedEntityRecord(
'postType',
'wp_template_part',
id,
{ context: 'view' }
);
return {
editedBlocks: editedRecord.blocks,
content: editedRecord.content,
};
},
[ id ]
);

const blocks = useMemo( () => {
if ( ! id ) {
return undefined;
}

if ( editedBlocks ) {
return editedBlocks;
}

if ( ! content || typeof content !== 'string' ) {
return [];
}

return parse( content );
}, [ id, editedBlocks, content ] );

const innerBlocksProps = useInnerBlocksProps( blockProps, {
value: blocks,
onInput: () => {},
onChange: () => {},
renderAppender: false,
layout: useLayout( layout ),
} );

return <TagName { ...innerBlocksProps } />;
}

function EditableTemplatePartInnerBlocks( {
postId: id,
hasInnerBlocks,
layout,
Expand All @@ -59,3 +116,42 @@ export default function TemplatePartInnerBlocks( {

return <TagName { ...innerBlocksProps } />;
}

export default function TemplatePartInnerBlocks( {
postId: id,
hasInnerBlocks,
layout,
tagName: TagName,
blockProps,
} ) {
const { canViewTemplatePart, canEditTemplatePart } = useSelect(
( select ) => {
return {
canViewTemplatePart:
select( coreStore ).canUser( 'read', 'templates' ) ?? false,
canEditTemplatePart:
select( coreStore ).canUser( 'create', 'templates' ) ??
false,
};
Comment on lines +129 to +135
Copy link
Member

Choose a reason for hiding this comment

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

@fabiankaegy, I've got a similar question here. Should this check if a user can read/edit this particular template part, or is it just a general check for templates?

},
[]
);

if ( ! canViewTemplatePart ) {
return null;
}

const TemplatePartInnerBlocksComponent = canEditTemplatePart
? EditableTemplatePartInnerBlocks
: NonEditableTemplatePartPreview;

return (
<TemplatePartInnerBlocksComponent
postId={ id }
hasInnerBlocks={ hasInnerBlocks }
layout={ layout }
tagName={ TagName }
blockProps={ blockProps }
/>
);
}
Loading