Major update to Hall theme #7688
Conversation
This is a major update to Hall theme (not yet launched)
| @@ -2,13 +2,13 @@ | |||
| <div class="wp-block-group alignwide"><!-- wp:group {"align":"wide","layout":{"type":"constrained"}} --> | |||
| <div class="wp-block-group alignwide"><!-- wp:columns {"align":"wide","style":{"spacing":{"blockGap":{"left":"1.5rem"}}}} --> | |||
| <div class="wp-block-columns alignwide"><!-- wp:column {"verticalAlignment":"center"} --> | |||
| <div class="wp-block-column is-vertically-aligned-center"><!-- wp:paragraph {"align":"left","style":{"spacing":{"blockGap":{"left":"1.5rem"}},"typography":{"textTransform":"uppercase"}}} --> | |||
| <p class="has-text-align-left" style="text-transform:uppercase">Designed with <a rel="nofollow" href="https://wordpress.org">WordPress</a></p> | |||
| <div class="wp-block-column is-vertically-aligned-center"><!-- wp:paragraph {"align":"left","style":{"spacing":{"blockGap":{"left":"1.5rem"}}}} --> | |||
There was a problem hiding this comment.
The theme credit should be turned into a pattern and wrapped in a translation function, like this: https://github.com/Automattic/themes/blob/trunk/allez/patterns/footer.php
There was a problem hiding this comment.
As I understand it, template parts and patterns work the same here. In fact, TPs are Patterns on the Editor (screenshot). Create Block Theme doesn't export our custom sets as Header or Footer as intended but there they are, in General.
There was a problem hiding this comment.
To offer context, Allez, the theme you mentioned, was reviewed by a developer (Jeff Ong) before launch. That is why these issues are covered on that one.
There was a problem hiding this comment.
As I understand it, template parts and patterns work the same here.
This is true. The reason we always convert this bit to a pattern is that it contains a string that needs to be translatable, and that requires making it a pattern.
| <!-- /wp:paragraph --></div> | ||
| <!-- /wp:column --> | ||
|
|
||
| <!-- wp:column {"verticalAlignment":"center"} --> | ||
| <div class="wp-block-column is-vertically-aligned-center"><!-- wp:navigation {"overlayMenu":"never","layout":{"type":"flex","justifyContent":"left"},"style":{"spacing":{"blockGap":"2rem"}}} /--></div> | ||
| <div class="wp-block-column is-vertically-aligned-center"><!-- wp:navigation {"ref":34,"overlayMenu":"never","layout":{"type":"flex","justifyContent":"left"},"style":{"spacing":{"blockGap":"2rem"}}} /--></div> |
There was a problem hiding this comment.
The ref attribute should be removed.
| <!-- /wp:paragraph --></div> | ||
| <!-- /wp:column --> | ||
|
|
||
| <!-- wp:column {"verticalAlignment":"center"} --> | ||
| <div class="wp-block-column is-vertically-aligned-center"><!-- wp:navigation {"overlayMenu":"never","style":{"typography":{"fontStyle":"normal","fontWeight":"600","textTransform":"uppercase"}},"fontSize":"small"} /--></div> | ||
| <div class="wp-block-column is-vertically-aligned-center"><!-- wp:navigation {"ref":34,"overlayMenu":"never","layout":{"type":"flex","justifyContent":"left"},"style":{"spacing":{"blockGap":"2rem"}}} /--></div> |
There was a problem hiding this comment.
The ref attribute should be removed.
| @@ -4,31 +4,13 @@ | |||
| * Slug: hall/404 | |||
| * Inserter: no | |||
| */ | |||
| declare( strict_types = 1 ); | |||
There was a problem hiding this comment.
We do need to keep this bit of code in all PHP files.
| <!-- /wp:heading --> | ||
|
|
||
| <!-- wp:paragraph --> | ||
| <p><?php echo esc_html__( 'Founded in the heart of Buenos Aires in 1896, the International Art Museum has, from its inception, housed a rich diversity of works spanning different periods and artistic movements globally. It emerged with the mission not only to safeguard the richness of international art but also to foster the flourishing of Argentine art within a context of cultural renewal.', 'hall' ); ?></p> | ||
| <!-- wp:paragraph {"style":{"typography":{"lineHeight":1.4}}} --> |
There was a problem hiding this comment.
All the strings in this file need to be wrapped in a translation function.
There was a problem hiding this comment.
I don't know how to do that. I work on the Editor, with quite limited code skills. But I can learn some best practices to help if I can.
| <!-- /wp:heading --> | ||
|
|
||
| <!-- wp:paragraph --> | ||
| <p>It looks like nothing was found at this location.<br>Maybe try a search?</p> |
There was a problem hiding this comment.
The 404.php pattern needs to be inserted here instead.
| @@ -1 +1,195 @@ | |||
| <!-- wp:pattern {"slug":"hall/home"} /--> | |||
There was a problem hiding this comment.
I think the old version was right - this file should include the home.php pattern.
There was a problem hiding this comment.
I need to figure out how to fix this. I use only the Editor, create everything and export with the Create Block Theme plugin. Please let me know what to do to fix it.
There was a problem hiding this comment.
Unfortunately the CBT plugin doesn't cover all things required to make a theme launch-ready (on .com or .org).
This issue should take care of, but until then it's manual work I'm afraid.
For now, once I export the theme with CBT I go through the files and look for these known issues. You'll notice that it's the same things coming up over and over again - mostly issues around content that needs to be translatable and thus requires turning it into a pattern, removing the ref from navigation, making sure that all assets were properly included and referenced in the files, etc.
|
Sorry for the mess, @alaczek. I tried to commit new changes to the add/hall, but it's not working. This conflicting file holding the review is unnecessary, and I would like to eliminate it. I want to merge this theme with the changes committed last month. |
MaggieCabrera
added
the
Ready to launch
This is a major update to Hall theme (not yet launched).
(Previous PR: #7319)