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

Use Timber (as a composer library) #14

Open
mallorydxw opened this issue Aug 17, 2020 · 10 comments
Open

Use Timber (as a composer library) #14

mallorydxw opened this issue Aug 17, 2020 · 10 comments

Comments

@mallorydxw
Copy link
Contributor

Advantage: clean separation of templates and other code

Disadvantage: new syntax to learn

https://timber.github.io/docs/

@mallorydxw
Copy link
Contributor Author

mallorydxw commented Aug 27, 2020

  • Figure out how to put twig templates in a directory that isn't templates/views
  • Figure out how to load SVG images inline - {% import '../../assets/images/foo.svg' %} doesn't work because it doesn't allow directory traversal - a function may work
  • Set $autoescape to 'html'

@RobjS
Copy link
Contributor

RobjS commented Sep 28, 2020

Our experience of Timber on Wilton Park has left me uncertain as to whether or not it's a good thing. In theory, it still delivers real benefits, but the way it's structured seems particularly unfriendly to testing. Particular issues I encountered were:

It might be that we can deal with the first couple of issues via Kahlan once we're a bit more familiar with it, and the third could potentially be remedied via team convention/code review. But I feel like we need more experience of it before we can really know if it's the right tool. In particular, I think we need to be sure it doesn't hinder our ability to write testable code.

@RobjS
Copy link
Contributor

RobjS commented Dec 10, 2020

If we do end up using Timber, we should use a linter for the twig templates, e.g. https://github.com/sserbin/twig-linter

@RobjS
Copy link
Contributor

RobjS commented Dec 10, 2020

I think the release of Timber 2.0 might help with getting the logic into testable classes. e.g. there's a timber/calling_php_file action that we could potentially use to hook templates to classes of the same name, and automatically run all the logic there: https://timber.github.io/docs/v2/hooks/actions/#timber/calling_php_file

Any of the classes that are being hooked in like that could extend a parent class that automates that hooking in process maybe? We could make that parent class part of Iguana if needs be.

@serena-piccioni
Copy link

You can set your own locations for your twig files with:
Timber::$locations = '/Users/jared/Sandbox/templates';
Use the full file path to make sure Timber knows what you're trying to draw from.

@serena-piccioni
Copy link

serena-piccioni commented Jan 18, 2021

For the svg inline, there is https://github.com/manuelodelain/svg-twig-extension

OR

if your SVG lies in a folder called assets/icons/ in the main folder of your theme, you add that folder to the list:

functions.php

Timber::$dirname = array( 'views', 'assets' );
Be sure to also include the name of the folder for your Twig templates in the array. Otherwise these won’t work anymore.

You can then do

{% include 'icons/my.svg' ignore missing %}
To display the SVG.

If you have only one file to include, it’s probably overkill to make Timber look in another directory. You could also use file_get_contents() and get_template_directory().

{{ fn('file_get_contents', fn('get_template_directory') ~ '/assets/icons/my.svg') }}

OR

simply rename the .svg as .twig (it's text right?) and include it.

@serena-piccioni
Copy link

serena-piccioni commented Jan 18, 2021

I'm not sure what was the problem, because you can actually do that:

if ( class_exists('Timber') ) { Timber::$autoescape = 'html';}

But honestly I would prefer escaping one by one e.g.

{{ post.get_field('custom_link')|e('esc_url') }}

considering that Timber has

wp_kses_post
esc_url
esc_html
esc_js

@serena-piccioni
Copy link

serena-piccioni commented Jan 18, 2021

  1. its frequent use of static methods

could you please make an example of this?

  1. the requirement to new up instances of classes on demand (e.g. when handling menus or pagination)

Half true: you can declare the menus on the global context,
and you have to use a query (and its object) for the pagination only if it's a very specific one, otherwise usually you can use an easy get_pagination.

  1. the approach of expecting .twig templates to have an accompanying .php file containing the logic

Half true again: you can declare twig files separated from a specific php,
and the best way to set up the logic is not what we did into Wilton Park,
but the general idea of Timber is to extend the basic classes to move the logic away from the .php theme files.

See: https://timber.github.io/docs/guides/extending-timber/#an-example-that-extends-timberpost

These can get pretty complex. And that's the beauty. The complexity lives inside the context of the object, but very simple when it comes to your templates.

In that case, I think we could easily use Kahlan too

@RobjS
Copy link
Contributor

RobjS commented Jan 25, 2021

  1. its frequent use of static methods

could you please make an example of this?

e.g. Timber::get_posts(), Timber::query_post(), Timber::fetch(), Timber::get_terms() etc. Generally, these are harder to mock (and therefore test) than instance methods, where you can mock and inject an instance of the class.

  1. the requirement to new up instances of classes on demand (e.g. when handling menus or pagination)

Half true: you can declare the menus on the global context,
and you have to use a query (and its object) for the pagination only if it's a very specific one, otherwise usually you can use an easy get_pagination.

Looking at the documentation for declaring menus on the global context (https://timber.github.io/docs/guides/menus/#setting-up-a-menu-globally), it looks like this still requires a new \Timber\Menu() call.

  1. the approach of expecting .twig templates to have an accompanying .php file containing the logic

Half true again: you can declare twig files separated from a specific php,
and the best way to set up the logic is not what we did into Wilton Park,
but the general idea of Timber is to extend the basic classes to move the logic away from the .php theme files.

See: https://timber.github.io/docs/guides/extending-timber/#an-example-that-extends-timberpost

These can get pretty complex. And that's the beauty. The complexity lives inside the context of the object, but very simple when it comes to your templates.

In that case, I think we could easily use Kahlan too

I do like the look of the way you can extend the post object. That's something I've wanted to do with standard WP_Post objects in the past, and always been frustrated when you can't!

I think my main concern on this point was where there are e.g. parameters in the query string that should be used to determine what post query is run. The Timber docs don't really offer any pointers as how you should deal with a situation like that, so the obvious solution is to put all that logic into the .php file that lives with the .twig template (which as a result won't be testable, and possibly hard to maintain).

I guess maybe a couple of alternatives in a case like that might be to:

  1. Write a class that includes a method that can return a Timber\PostQuery() set up based on the query string params, and just call that instance method in the .php file that lives with the template. That means all the logic would still be a in a testable class.
  2. Write a class that hooks into pre_get_posts in the appropriate contexts, and sets the query up that way, e.g. like https://git.govpress.com/ukri-website/app/-/blob/develop/wp-content/themes/ukri/app/PostTypes/Opportunity/Filters.php. In which case, just calling Timber::get_posts() should retrieve the correct set of posts.

@serena-piccioni
Copy link

  1. Write a class that includes a method that can return a Timber\PostQuery() set up based on the query string params, and just call that instance method in the .php file that lives with the template. That means all the logic would still be a in a testable class.

I think we should consider to try and set up a new starter theme with this logic, and see how it works.
Of course I would not spend too much time on that.

I don't like the second alternative because hooking on pre_get_posts can result in unexpected behaviours, even with a set of tests.

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

No branches or pull requests

3 participants