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

Confusing Site API related to meta() and option() #2881

Closed
gregrobson opened this issue Jan 11, 2024 · 3 comments · Fixed by #2978
Closed

Confusing Site API related to meta() and option() #2881

gregrobson opened this issue Jan 11, 2024 · 3 comments · Fixed by #2978
Assignees
Labels

Comments

@gregrobson
Copy link

Is your feature request related to a problem? Please describe.

I spent a couple of hours diagnosing what appears to be some confusing API details.

The documentation for custom fields (which I mistook for meaning advanced custom fields) led to me trying to do:

{{ site.option("my_field_name") }}

to try and get the contents for an options field called my_field_name. The documentation indicated that site.meta() was deprecated and I expected that it would function like post.meta() as it had taken on it's behaviour.

Sidenote: I've now seen the documentation about how to fetch options data and other examples of how to add options data to the context.

Issue: This adds an extra step to add options to context in PHP.
Issue: site.meta() seems like a really good way to get individual ACF field data from the site.

❗I realise the solution was documented, but I didn't see it for a couple of hours, and this seems like an extra step where one shouldn't be required and there's the potential to save users from having to implement a filter when they only need to fetch a site option as a one-off.

Describe the solution you’d like

Consolidate post.meta() and site.meta() to have the same behaviour?

Option 1

  1. Undeprecate(?) Site.php's meta() function?
  2. Make the function auto-prefix 'options_' on the variable it fetches so that:
{{ site.meta("foobar") }}
# made equivalent to 
{{ site.option("options_foobar") }}

I realise this would be a breaking change

Describe alternatives you’ve considered

  1. Provide a filter so that the user has a mechanism to amend the name of the variable fetched through site.option(). Then I could provide a list of all ACF option fields and if the variable name matched, prepend 'options_' to the variable name.
  2. Add some documentation to point people to the current solution.

Additional context

Happy to help with a pull request for the docs. Not so certain that I'm comfortable enough to make any amends with the codebase.

Sorry if these feels pedantic, but I think this might catch out other developers and this might improve the developer expereince. I ❤️ Timber.

@Levdbas Levdbas added the docs label Jan 15, 2024
@Levdbas
Copy link
Member

Levdbas commented Jan 22, 2024

Hi @gregrobson ,

I see where you getting at.

What a solution might be:

  • only throw deprecation notice of the meta method when the ACF integration of Timber is not loaded (ACF is not active)
  • use the meta method in the Site class for getting the acf options field value when the ACF integration is loaded by prefixing the field name as you suggests.

I think this will be a possible breaking change for people running ACF and are still using the meta() method to get regular WordPress site options.

@gregrobson
Copy link
Author

That seems reasonable. I don't think there's an easier way around it.

@gchtr
Copy link
Member

gchtr commented Jan 25, 2024

I’m sorry it took you so long to understand how everything works and is connected.

One of the problems with restoring {{ site.meta() }} is that when you use WordPress as a multisite, there’s a database table called wp_sitemeta, that stores information about, see https://deliciousbrains.com/tour-wordpress-database/#wp_sitemeta for a short explanation.

I guess we can’t reserve the name "site meta" for getting settings saved by ACF. This could also lead to a lot of confusion. Because people might think it refers to getting values from the wp_sitemeta database table.

I still think the way to go is setting the values in context or adding a custom function that gets the ACF options in a way you prefer.

If hope that we can fix this through better documentation and adding hints and links to other documentation pages in the right places.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants