Skip to content

Conversation

@gigitux
Copy link
Contributor

@gigitux gigitux commented Sep 17, 2024

What?

This PR migrates the duplicateTemplatePart action to the @wordpress/fields package. The primary issue with this action is its dependency on the CreateTemplatePartModalContents component, which resides in the @wordpress/editor package. Due to this, it can't be directly used within the @wordpress/fields package.

For the purpose of the @wordpress/fields package work, I have temporarily moved the component to this package. However, I'm not certain if this is the best long-term solution. Ideally, this component (and potentially others) should reside in a dedicated package, such as @wordpress/template, which would be more appropriate for components related to template and template-parts. A similar package for the patterns already exists: https://github.com/WordPress/gutenberg/blob/d6fcf4aea4aff17cf0910862bf10090e20ba2ab0/packages/patterns/src

Please review and provide feedback on whether this approach is suitable or if there are alternative recommendations for handling these dependencies. Thanks! 🙏 cc @youknowriad @oandregal

Testing Instructions

Ensure that Custom Dataviews is enabled.

  1. Visit the Site Editor.
  2. Visit the patterns view.
  3. Click on header
  4. Duplicate a header template part.
  5. Ensure that the duplicated template part is created.

Testing Instructions for Keyboard

Screenshots or screencast

@gigitux gigitux requested a review from youknowriad November 26, 2024 10:52
@gigitux
Copy link
Contributor Author

gigitux commented Nov 26, 2024

This PR is ready to be reviewed! cc @oandregal @youknowriad @louwie17

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

Thanks LGTM

@gigitux
Copy link
Contributor Author

gigitux commented Dec 3, 2024

Thanks LGTM

Thanks for your review! 🙇

@gigitux gigitux enabled auto-merge (squash) December 3, 2024 09:29
@gigitux gigitux disabled auto-merge December 3, 2024 09:41
@gigitux gigitux enabled auto-merge (squash) December 3, 2024 09:41
@gigitux gigitux merged commit 5900cf6 into WordPress:trunk Dec 3, 2024
66 of 67 checks passed
@github-actions github-actions bot added this to the Gutenberg 19.9 milestone Dec 3, 2024
im3dabasia pushed a commit to im3dabasia/gutenberg that referenced this pull request Dec 4, 2024
michalczaplinski pushed a commit that referenced this pull request Dec 5, 2024
const defaultModalTitle = useSelect(
( select ) =>
select( coreStore ).getPostType( TEMPLATE_PART_POST_TYPE )?.labels
// @ts-ignore
Copy link
Contributor

@ciampo ciampo Dec 6, 2024

Choose a reason for hiding this comment

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

Is this // @ts-ignore necessary?

In general, I usually recommend using @ts-expect-error instead of @ts-ignore (this article explains the reason).

It's also usually a good idea to leave an inline comment explaining why we're ignoring/expecting a given TS error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch! I overlooked it when I iterate on this PR. Fixed with #67709

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Package] DataViews /packages/dataviews [Type] Experimental Experimental feature or API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants