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

Template hierarchy filter: Preserve existing paths #141

Merged
merged 9 commits into from
Mar 18, 2024

Conversation

strarsis
Copy link
Contributor

@strarsis strarsis commented Nov 20, 2021

@retlehs: Instead of solely returning the blade template path locate result,
append it to the array of existing files passed to the template hierarchy filter handler,
allowing Gutenberg Full Site Editing (FSE) to find and use block templates/template parts in the (Sage 10) theme.

This fix lets the Gutenberg Full Site Editing/FSE templates load in Gutenberg,
while still allowing classic (blade) templates to be used (as for WooCommerce that currently doesn't fully use Gutenberg yet).

Useful resources

PSA

Double-check whether remove_theme_support('block-templates') is absent from your theme setup, as otherwise FSE will not work.

@retlehs retlehs self-assigned this Dec 4, 2021
@QWp6t QWp6t removed their request for review December 5, 2021 04:42
@retlehs
Copy link
Member

retlehs commented Dec 7, 2021

@strarsis thanks for the PR! i'm seeing some really odd behavior when trying to test FSE

could you share a repo/branch with me that has sage 10 with your FSE changes applied to it?

@strarsis
Copy link
Contributor Author

@retlehs: I see, yes I can prepare something that uses this patch and also has FSE features so this can be tested.

@strarsis
Copy link
Contributor Author

strarsis commented Dec 12, 2021

@retlehs: Alright, this repository includes this patch, enables Gutenberg Full Site Editing/FSE and adds FSE block templates and block template parts as demonstration:
https://github.com/strarsis/sage10-fse (edit)

@stefanfisk
Copy link
Contributor

@strarsis did you forget the link, and if so is this the one https://github.com/strarsis/sage10-fse?

@strarsis
Copy link
Contributor Author

strarsis commented Dec 15, 2021

@stefanfisk: Yes, indeed! I forgot to link the repo: https://github.com/strarsis/sage10-fse

(WordPress site themes directory)

git clone https://github.com/strarsis/sage10-fse
cd sage10-fse
composer install
# to prevent the `Bootloader::findPath() must be of the type string` issue
npm install
npm run build

Activate theme.

@strarsis
Copy link
Contributor Author

@stefanfisk: The repo was updated and works now with latest WordPress, Gutenberg plugin and Sage 10 theme.
Discussion: https://discourse.roots.io/t/full-site-editing-fse-frontend-doesnt-load-template/21574/16?u=strarsis

@ouun
Copy link
Contributor

ouun commented Nov 25, 2022

Any news on this? FSE support with Acorn/Sage would be really great.

@oxyc
Copy link
Contributor

oxyc commented Nov 25, 2022

I'm running it fine by overriding the SageServiceProvider and not binding the template filters, so that all templates are in FSE. Running it with WooCommerce

@strarsis
Copy link
Contributor Author

strarsis commented Nov 25, 2022

@oxyc: Would this be a better approach than modifying the acorn template filter as in this PR?
@ouun: If you have the time, could you try out the linked Sage 10 FSE theme example and see whether it works? It is Sage 10 with the patched acorn library.

@oxyc
Copy link
Contributor

oxyc commented Dec 8, 2022

To me it makes sense to not the bind the filters in the first place. I have no php templates in my theme since I opted in to FSE. I can still use blade in blocks

@strarsis
Copy link
Contributor Author

strarsis commented Dec 8, 2022

@oxyc: Speaking of unbinding or short-circuiting filters, I had to force the Blade-PHP template filter to skip when a PHP-file already exists. Without this additional adjustment, some part of the HTML page markup would be rendered twice (<header>), which wouldn't be noticeable visually, but cause unexpected behavior (Popup maker).

So ideally the Sage template filter can use Blade-PHP templates, fall back to block templates and finally usual PHP files.
The adjustments in this PR would allow for this behavior.

Instead of solely returning the blade template path locate result, 
append it to the array of existing files passed to the template hierarchy filter handler,
allowing Gutenberg Full Site Editing (FSE) to find and use block templates/template parts in the (Sage 10) theme.
Allow existing files to be used instead of Blade-PHP template (notably `template-canvas.php`).
@strarsis
Copy link
Contributor Author

strarsis commented Mar 16, 2023

Side note/FWIW: fellow devs who still need PHP 7.x support, the patch-2 branch has this fix also applied, but on the highest possible acorn version that still supported PHP 7.3: https://github.com/strarsis/acorn/tree/fse-php7

@broskees
Copy link
Contributor

broskees commented May 1, 2023

Does this need more work to get into acorn core?

@strarsis
Copy link
Contributor Author

strarsis commented May 1, 2023

There is a modification to one of the PR changes by @r-chrzan which appears to improve the loading behavior (Blade-PHP templates override FSE template files): strarsis/sage10-fse#1 (comment)

@strarsis strarsis closed this Oct 12, 2023
@strarsis strarsis deleted the patch-1 branch October 12, 2023 09:36
@strarsis strarsis restored the patch-1 branch October 12, 2023 09:39
@strarsis strarsis reopened this Oct 12, 2023
@strarsis strarsis mentioned this pull request Oct 12, 2023
@palicko
Copy link

palicko commented Dec 14, 2023

Hey guys, how does this PR looks like? You think it can be merged into Acorn core any time soon?
Thanks!

@strarsis
Copy link
Contributor Author

Hey guys, how does this PR looks like? You think it can be merged into Acorn core any time soon?
Thanks!

By the way, there is also another PR now that may handle hybrid theme situations better: #314

Independent from the particular acorn fix, you may find this Sage 10 FSE example theme base interesting:
https://github.com/strarsis/sage10-fse

🧑‍💻 Allow blade views to override FSE templates
@Log1x Log1x merged commit 129d64e into roots:main Mar 18, 2024
2 checks passed
@strarsis
Copy link
Contributor Author

Awesome! 👍

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.

8 participants