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

Blockbase: make 404 into a php template so it can be translated #4332

Merged
merged 2 commits into from
Jul 27, 2021

Conversation

jffng
Copy link
Contributor

@jffng jffng commented Jul 26, 2021

Changes proposed in this Pull Request:

This makes it so Blockbase and its children's 404 template is php-based so it can be translated. I

I could not just add a 404.php template to Quadrat. Blockbase's 404.html template would win in the template resolution logic. So I had to replace Blockbase's template with a php-based one to get this approach to work.

Related issue(s):

#3653

@@ -198,6 +198,9 @@
}
}
},
"layout": {
"contentSize": "620px"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should probably also look into making it so the layout key generates a CSS variable, because this is redundant.

Copy link
Contributor

Choose a reason for hiding this comment

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

Couldn't find anything so opened this: WordPress/gutenberg#33705

@@ -3,10 +3,6 @@
<!-- wp:group {"layout":{"inherit":true}, "tagName":"main"} -->
<main class="wp-block-group">

<!-- wp:heading -->
<h2>Results:</h2>
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we do the same for blockbase? That would make all 4 blockbase themes i11n-ready.

Copy link
Contributor

@pbking pbking left a comment

Choose a reason for hiding this comment

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

Other than removing Results: from blockbase as well I think this is good-to-go. I'm glad it's only one (and a simple) page.

Copy link
Contributor

@MaggieCabrera MaggieCabrera left a comment

Choose a reason for hiding this comment

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

This LGTM! should we create a Glotpress project for Blockbase? I think we only did so for Quadrat

@mikachan
Copy link
Member

should we create a Glotpress project for Blockbase? I think we only did so for Quadrat

Yeah we've only created one for Quadrat so far, guessing we should do one for Blockbase too.

@jffng jffng merged commit ae7e63d into trunk Jul 27, 2021
@jffng jffng deleted the add/quadrat-404-php branch July 27, 2021 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants