Skip to content

2.x Update Timber Object Getter function names in Twig#2178

Merged
acobster merged 16 commits into2.x-posts-apifrom
2.x-twig-get-functions-2
Sep 26, 2020
Merged

2.x Update Timber Object Getter function names in Twig#2178
acobster merged 16 commits into2.x-posts-apifrom
2.x-twig-get-functions-2

Conversation

@gchtr
Copy link
Copy Markdown
Member

@gchtr gchtr commented Jan 18, 2020

I didn‘t manage to get a hold of the mess that is #2170 now, after the base branch update to 2.x-factories 😅. So here it is again, with the same commits cherry-picked against 2.x-factories.

Ticket: #2144

Issue

See #2144.

Solution

  • Added new Twig functions for the relevant Timber::get_* functions.
  • Updated the deprecation warnings. There’s now always two possibilities. Instead of Post(), it can either be get_post() or get_posts(), depending on what’s at hand. The same goes for Term and User.
  • I deliberately removed the {{ Attachment() }} function as well as deprecated the {{ Image() }} function without replacement, because we can use {{ get_post() }} and {{ get_posts() }}. The Class Maps will handle returning either a Timber\Image or Timber\Attachment.

Impact

A more unified way of interacting with Timber’s API throughout PHP and Twig.

Usage Changes

New function names. Pretty much the same functionality, except for handling singular and multiple objects.

Considerations

  • Should we add Twig functions for the special-case Timber::get_* functions, like Timber::get_user_by() or Timber::get_post_by_slug()? I think, currently we don’t have to do that.
  • Should we add Twig functions for getting Attachment and Image posts: {{ get_attachment }} and {{ get_image() }}? If we have them in Twig, we should also have them in PHP, as Timber::get_* functions.

Testing

Tests are currently breaking. We first need to resolve the Considerations about adding a get_attachment() and a get_image() functions, so it’s easier to update the tests. If we just switch to get_post() now and then want to add get_attachment() or get_image() later, it will be harder to find the test methods that need these functions. Now, we can still search for {{ Attachment() }} and {{ Image() }}.

Todo

  • Add Class Map for Attachment and Image class.
  • Add testing for comment functions in Twig.
  • Resolve todos in code.

@jarednova
Copy link
Copy Markdown
Member

Sorry for the slow response on this @gchtr — didn't realize you had some open Qs blocking you ...

Should we add Twig functions for the special-case Timber::get_* functions, like Timber::get_user_by() or Timber::get_post_by_slug()? I think, currently we don’t have to do that.

I don't think so. Feels like something that could be done in the future, but not something we need to add for now.

Should we add Twig functions for getting Attachment and Image posts: {{ get_attachment }} and {{ get_image() }}? If we have them in Twig, we should also have them in PHP, as Timber::get_* functions.
On this one, while I realize that get_post might do functionally the same thing as get_attachment or get_image, it would be confusing syntax for someone to read. A frequent use case might be something like ...

<img src="{{ get_post( my_image_id ).src }}" />

which is just, kinda misleading to read. It'd be very easy for someone to read that and not really understand what's going on. Even if these function(s) extended get_post and did nothing else ...

<img src="{{ get_image( my_image_id ).src }}" />

makes total sense for someone to read-through (get_attachment splits the difference — def. more understandable than get_post but not as clear as it could be)

@gchtr gchtr self-assigned this Feb 16, 2020
@gchtr gchtr added wip Work in Progress blocked labels Aug 14, 2020
@gchtr gchtr requested a review from jarednova as a code owner August 17, 2020 21:33
@acobster acobster force-pushed the 2.x-factories branch 2 times, most recently from 23c3421 to b0d086a Compare August 17, 2020 22:13
Copy link
Copy Markdown
Member

@jarednova jarednova left a comment

Choose a reason for hiding this comment

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

@gchtr just found a few spots in the docs/messages to address. Please check these out to make sure I have these right and then 👍 away we go!


In Twig, you could use functions that were called the same as classes to convert objects or IDs of objects into Timber object. To have the same names as in Timber’s public API, we’ve added the following functions.

- `{{ get_post() }}`
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
- `{{ get_post() }}`
- `{{ get_attachment() }}`
- `{{ get_attachments() }}`
- `{{ get_image() }}`
- `{{ get_images() }}`
- `{{ get_post() }}`

- The `Timber\Image` class now extends the `Timber\Attachment` class. All your code should already be compatible with this change. But in the future, you might want to use the new `Timber\Attachment` class if you work with an attachment that is not an image.
- We’ve added new methods for `Timber\Attachment`. See the section below (@todo: Add anchor link)
- We’ve added a new Twig function `Attachment()`. (@todo: Add link to documentation)
- To get attachments from attachment IDs in Twig, you can use `{{ get_post(attachment_id) }}`. (@todo: Add link to documentation)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
- To get attachments from attachment IDs in Twig, you can use `{{ get_post(attachment_id) }}`. (@todo: Add link to documentation)
- To get attachments from attachment IDs in Twig, you can use `{{ get_attachment(attachment_id) }}`. (@todo: Add link to documentation)

@gchtr gchtr mentioned this pull request Aug 29, 2020
15 tasks
@acobster
Copy link
Copy Markdown
Collaborator

@gchtr

I deliberately removed the {{ Attachment() }} function as well as deprecated the {{ Image() }} function without replacement, because we can use {{ get_post() }} and {{ get_posts() }}. The Class Maps will handle returning either a Timber\Image or Timber\Attachment.

the URL/file path use-case brings up an interesting question here. Can we accomplish everything we need with just {{ get_post() }}? If passed a non-numeric string, PostFactory will always return false. So as-is, {{ get_post("/path/to/file.jpg") }} will not work. So that might be a good argument for also having a Timber::get_attachment() and corresponding Twig helper.

@acobster acobster mentioned this pull request Sep 9, 2020
8 tasks
@acobster acobster changed the base branch from 2.x-factories to 2.x-posts-api September 26, 2020 17:16
Coby Tamayo added 3 commits September 26, 2020 10:30
Resolved conflicts manually, favoring changes from #2178.

~ Conflicts:
~	lib/Twig.php
~	tests/test-timber-image.php
~	tests/test-timber-twig-objects.php
@acobster acobster merged commit adf9fc6 into 2.x-posts-api Sep 26, 2020
@jarednova jarednova deleted the 2.x-twig-get-functions-2 branch September 30, 2020 00:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2.0 blocked wip Work in Progress

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants