Include _head.twig from post as well as base - #2676
Conversation
Sorry, something went wrong.
|
I have to admit I have to idea what this includeFrom means and what effect it has on anything else. Like I stated in our Slack-chat, I never see the throbber.. This leads me to this concern: a themed (header included) throbber might increase the idea that we're waiting for the SSP-site to load, while in fact we're waiting for the SP to show their index-page. Instead of including the header in the post-page, would it be an idea to create a colours-CSS that we can include in both the main-CSS and the post-CSS? |
Sorry, something went wrong.
|
@monkeyiq What is your expertise on this? |
Sorry, something went wrong.
It doesn't include the visible header bar, nor do I think that would be sensible (for the same reason that concerns you). The visible theme header bar comes from _header.twig, included in base.twig here: https://github.com/simplesamlphp/simplesamlphp/blob/v2.5.3.1/templates/base.twig#L17. I am not suggesting we include that in post.twig (I actually think that would be silly, for the same reason you do.) The _head.twig file is an extension of the preload block and is optionally included in the <head> element. It only makes sense for that to include things that would normally be in <head> (i.e. non-visible elements such as <style>, <link> or <meta>). By default _head.twig does not exist, meaning these changes do nothing on a stock install -- they only come into play if someone wants to add things into <head>, and my theming is just an example (albeit my use case). In base.twig, the _head.twig file is included here: https://github.com/simplesamlphp/simplesamlphp/blob/v2.5.3.1/templates/base.twig#L12. This just adds an equivalent placeholder in post.twig. |
Sorry, something went wrong.
The _head.twig file introduced in 2.0 by commit 28cf796 provides a really convenient and straightforward way to introduce new elements into the without altering the templates or overriding them in a theme.
Coupled with the proposed change to the CSS in simplesamlphp/simplesamlphp-assets-base#108, this also allows for a poor-man's theming option where colours can be overridden by simply adding some styling in _head.twig.
Unfortunately, while it is included by base.twig, it is not currently included by post.twig. That means it's not possible to apply this to the loader throbber introduced by 2.5 by #2516. While that might seem like a small issue, depending on the colour change, the current loader's default red can seem quite stark.
This change means _head.twig applies to both base HTML templates.
It introduces an additional context variable includedFrom so that people making changes in _head.twig can determine where it is being used. A grep of the source suggests I'm not changing or overloading an existing twig variable by doing this, so it's a more-or-less backwards compatible change. The only risk is that someone has an existing _head.twig that introduces extraneous elements into post.twig (but they can fix that by adding a context check against includedFrom).