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

Add a global Twig variable with Contao state #6436

Merged
merged 11 commits into from Jan 29, 2024

Conversation

aschempp
Copy link
Member

@aschempp aschempp commented Oct 12, 2023

I think we controversely discussed this before. However, I had to copy-paste this to multiple of my projects recently, and I think it would make sense in general. With this, you can do the following in any Twig template:

any property from the page object (from current request = usually the global)

{{ contao.page.urlPrefix }}

check if preview mode is enabled

{{ contao.tokenChecker.isPreviewMode }}

@aschempp aschempp added this to the 5.3 milestone Oct 12, 2023
@aschempp aschempp requested a review from a team October 12, 2023 15:39
@aschempp aschempp self-assigned this Oct 12, 2023
@ausi
Copy link
Member

ausi commented Oct 13, 2023

I think the downside of something like this is, that the controller rendering the template no longer knows what data the template accesses, making the response of that controller theoretically uncacheable.

However, that problem already exists because of the global app variable.

Maybe we could at least mention that in the documentation of this global twig variable then?

@m-vo
Copy link
Member

m-vo commented Oct 13, 2023

We also should discuss if this would better be a runtime that provides dedicated functions instead of a 'global'. I'm not sure yet but I'm leaning a bit towards the latter because then we could provide some more convenience instead of outputting raw values that the template has to deal with.

@aschempp Out of interest: where did you need those things? I assume in content elements where you did not have control over the controllers? If so, we could also just add these things to AbstractContentElementController#addDefaultDataToTemplate().

@aschempp
Copy link
Member Author

I mostly used them in fe_page.html.twig actually. Only output the analytics scripts if no backend user is logged in (and not in preview mode). And I think I added some custom fields to tl_page and added CSS classes based on that to the body.

In my cases, neither of these would affect the cache (since anything in regards to user being logged in means a session, and a session means no cache). And any page property wouldn't change on a per-user basis.

@aschempp aschempp added the up for discussion Issues and PRs which will be discussed in our monthly Mumble calls. label Oct 27, 2023
@fritzmg
Copy link
Contributor

fritzmg commented Nov 21, 2023

At least the contao.tokenChecker would be required if you'd want to implement Analytics templates via Twig - since you'd want to use the hasBackendUser method of the token checker.

@aschempp
Copy link
Member Author

aschempp commented Dec 6, 2023

At least the contao.tokenChecker would be required if you'd want to implement Analytics templates via Twig - since you'd want to use the hasBackendUser method of the token checker.

It would probably be better to add a contao.hasBackendUser method directly and use the token checker there. Then Twig users would not rely on knowing there is a TokenChecker and what methods it contains.
But I don't know if it is possible (and how to) maybe have contao.security.hasBackendUser without populating the contao.security array with properties that are possibly never used.

@andyKempf
Copy link

andyKempf commented Jan 25, 2024

Servus & Moin!
Are there plans to include this change in the new LTS version? That would be nice, since you can't access tl_page DCA customizations with php variable $[GLOBALS] in Twig.

Thanks for your great work!

Greetings from Munich,
Andi

@fritzmg
Copy link
Contributor

fritzmg commented Jan 26, 2024

I agree that we should add this (or any other global variables that are useful) to the new LTS. Could also be done as a bugfix.

# Conflicts:
#	core-bundle/config/services.yaml
#	core-bundle/src/Twig/Extension/ContaoExtension.php
@aschempp aschempp marked this pull request as ready for review January 26, 2024 17:56
@aschempp
Copy link
Member Author

PR updated. I also think we should "group" more useful global properties into the contao. variables. That includes the request token to me. @contao/developers let me know if you agree, maybe I'll need to update all usages then.

I checked the TemplateTrait and a few other places but couldn't find anything else to add. But I'm sure we'll come up with more … once we start using it! I intentionally did not add TL_MODE because the scope should not be relevant to the templates (and already is available for fragments etc.).

@aschempp aschempp changed the base branch from 5.x to 5.3 January 26, 2024 18:00
@aschempp aschempp removed the up for discussion Issues and PRs which will be discussed in our monthly Mumble calls. label Jan 26, 2024
@aschempp
Copy link
Member Author

Also, the SecurityVariable is necessary to get autocomplete support in the IDE. I tried using an anonymous class first, but then PHPStorm would not autocomplete the properties.

core-bundle/config/services.yaml Outdated Show resolved Hide resolved
core-bundle/config/services.yaml Outdated Show resolved Hide resolved
@aschempp
Copy link
Member Author

The properties are now

{{ contao.has_backend_user }}
{{ contao.is_preview_mode }}
{{ contao.request_token }}
{{ contao.page.* }}

@aschempp aschempp requested review from ausi and fritzmg January 27, 2024 22:00
ausi
ausi previously approved these changes Jan 29, 2024
Copy link
Member

@ausi ausi left a comment

Choose a reason for hiding this comment

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

LGTM! Apart from #6436 (comment)

fritzmg
fritzmg previously approved these changes Jan 29, 2024
leofeyer
leofeyer previously approved these changes Jan 29, 2024
@leofeyer leofeyer merged commit 4df98c0 into contao:5.3 Jan 29, 2024
17 checks passed
@leofeyer
Copy link
Member

Thank you @aschempp.

@aschempp aschempp deleted the feature/contao-variable branch February 2, 2024 07:29
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 this pull request may close these issues.

None yet

6 participants