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

Jaida: Add theme for review #7016

Merged
merged 12 commits into from
Jun 23, 2023
Merged

Jaida: Add theme for review #7016

merged 12 commits into from
Jun 23, 2023

Conversation

matiasbenedetto
Copy link
Member

Changes proposed in this Pull Request:

Jaida: Add theme for review
Created by @beafialho

image

@madhusudhand
Copy link
Member

madhusudhand commented Jun 14, 2023

Review notes:

  • Template parts such as header, footer are not rendered because of the additional theme attribute.
    <!-- wp:template-part {"slug":"secondary-header","theme":"jaida"} /-->
    I tried creating a blank theme using CBT and exported. It has no such attribute. Created Theme created using plugin doesn't render template parts WordPress/create-block-theme#396 to investigate further.
    Fixed by removing the attribute here: b47f723

  • queryId is found in the templates. It should be removed when the theme is exported.
    <!-- wp:query {"queryId":14,"query": {...} -->
    Created: Remove QueryId from the exported themes WordPress/create-block-theme#397
    Fixed by removing the attribute here: cc06710

  • A 404 pattern existed, but it is not used in 404 template. This can be resolved by saving the block as pattern.
    And this 404 pattern should have not created by CBT. @beafialho Did you use a block theme (not a blank theme) as base while creating this theme?
    This also the case for footer, and comments patterns. They are not used.
    Related commits: f5df793 04556d3 23e782d

  • Index and Archive pages has their own query definitions instead of a pattern. It's nothing wrong to have them this way. Moving them to a pattern is just an improvement. Leaving it as is.

  • Minor issue: empty height is set for spacer block. This could be a fix for gutenberg itself. 1f821ea

  • the assets such as images, screenshot can be compressed while exporting theme.

  • optionally, check if the assets used in theme and include only those in the exported theme.

Copy link
Member

@madhusudhand madhusudhand left a comment

Choose a reason for hiding this comment

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

Tested and fixed the issues. Ready to 🚢

@madhusudhand madhusudhand merged commit 835255f into trunk Jun 23, 2023
@madhusudhand madhusudhand deleted the add/jaida branch June 23, 2023 05:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants