Jump to content

IPS, please specify the app in {template} tags


Colonel_mortis

Recommended Posts

In the {template} template tags, it would help massively if you could specify the app except where there is a specific reason not to, so that the templates can be reused in other apps. The one that's bothering me specifically at the moment is on line 2 of core/html/front/system/settings.phtml, where you include the pageHeader without specifying that it needs to be in core. This means that, without requiring everyone who will use my app to modify that template to add the app in there, there is no way to reuse that template in my own app. The reason I want this is that I am adding a new item to the settings page (which is a painful experience btw), but I need the URLs to be handled by my own app rather than all in a settings hook, because I need a whole controller for it, and I have some extra authentication checks in execute.

For now, can you please add that particular app specifier so that my app will work, and in future can you try to include them where possible. In future, it might make sense (though it would be a breaking change) to make the app default to app that the current template is from (so anything inside a template from core would be assumed to be in core), with maybe a special specifier like "@current" to use the current app that the user is viewing.

Link to comment
Share on other sites

+1 for this. While it is true that going and fixing all templates would be tedious, at least try to check/fix all templates you work on from now. Even just editing a template to add a button or something, if you notice a template tag then please add the app value if it is missing. That way slowly all calls will be fixed at some point.

Link to comment
Share on other sites

  • 1 month later...
On 8.12.2016 at 2:31 PM, bfarber said:

Going through every template call would be a bit tedious but we can most likely address the ones brought up individually if you run into problems. I've adjusted this particular template call for 4.1.18.

Well, If you're feeling in a good mood one day: (And tempted to do what Colonel_mortis suggests in his last paragraph)

ips41171dev tsp$ git grep '{template=' | grep -v 'app=' | grep -v 'location=\"admin\"'

http://pastebin.com/2he5zqj0

You're welcome :)

Link to comment
Share on other sites

  • 4 weeks later...
On 8.12.2016 at 2:31 PM, bfarber said:

Going through every template call would be a bit tedious but we can most likely address the ones brought up individually if you run into problems. I've adjusted this particular template call for 4.1.18.

Could you please adjust the forumDisplay-template to specify the app. 

I just encountered an issue using this, and I need to use that template. Please. :)

applications/forums/dev/html/front/forums/forumDisplay.phtml

Specifically: 

{template="forumButtons" group="forums" params="$forum"}

 

Link to comment
Share on other sites

@bfarber, can you please change the call to a template in IPS\core\extensions\core\MemberForm\BasicInformation.php ? I'm creating a hook that will display all admin profile tabs but I got the error:

Quote

ErrorException: template_store_missing (0)
#0 C:\wamp64\www\general\applications\core\extensions\core\MemberForm\BasicInformation.php(125): IPS\_Theme->getTemplate('members')

because:

		/* Password */
		$form->add( new \IPS\Helpers\Form\Custom( 'password', NULL, FALSE, array(
			'getHtml'	=> function( $element ) use ( $member )
			{
				return \IPS\Theme::i()->getTemplate('members')->changePassword( $element->name, $member->member_id );
			}
		) ) );

If you change it to:

		/* Password */
		$form->add( new \IPS\Helpers\Form\Custom( 'password', NULL, FALSE, array(
			'getHtml'	=> function( $element ) use ( $member )
			{
				return \IPS\Theme::i()->getTemplate( 'members', 'core', 'admin' )->changePassword( $element->name, $member->member_id );
			}
		) ) );

My hook works just fine.

Thank you.

Link to comment
Share on other sites

  • 2 weeks later...
On 14/02/2017 at 2:59 PM, Adriano Faria said:

can you please change the call to a template in IPS\core\extensions\core\MemberForm\BasicInformation.php ? I'm creating a hook that will display all admin profile tabs but I got the error:

Can you confirm this in 4.1.19, @bfarber? We wouldn't need to ask over and over again if you IPS at least replied here. :)

Link to comment
Share on other sites

Just now, bfarber said:

For now, I would recommend duplicating the template files.

That's not an option on my case. It throws an error, as expllained above. Come on, seriously, what kind of agreement would be required to change from:

return \IPS\Theme::i()->getTemplate('members')->changePassword

to:

return \IPS\Theme::i()->getTemplate( 'members', 'core', 'admin' )->changePassword

?

Link to comment
Share on other sites

Brandon is creating a changePassword template into your application so that line loads your template rather than the core one. There are several benefits to this approach:

  1. You'll probably want to change the template anyway so that you can link to a different location which will allow you to redirect the admin should they choose to change the password - otherwise, after changing it, they'll be taken to the edit member page, rather than remaining within your application.
  2. Templates are not designed to be reused by controllers in other applications. By reusing a template in the core, you run the risk of the page changing, or worse breaking, if we decide to change the template or add a parameter to it, which we may do in a minor version.
  3. It will be more efficient, as loading templates in your application is done automatically. Loading other templates creates additional queries.

If you are not concerned about any of those, you can just include it by doing something like this rather than copying/pasting the whole thing:

<ips:template parameters="$name, $memberId" />
{template="changePassword" app="core" group="members" params="$name, $memberId"}

Think of it sort of like how you do parent::method() in code hooks, but for templates :)

Link to comment
Share on other sites

On 24.2.2017 at 1:58 PM, bfarber said:

Unfortunately, this is not complete agreement towards making the suggested change, so I cannot confirm the requested changes will be made. For now, I would recommend duplicating the template files.

Sorry, but did that apply to my suggestion as well? The forumDisplay.phtml

Because I'm not seeing it in 4.1.19 at least. 

Link to comment
Share on other sites

Development cannot make arbitrary changes, and some of the development team are not in agreement with the suggested changes. Specifically, as Mark indicated above

Quote

Templates are not designed to be reused by controllers in other applications. By reusing a template in the core, you run the risk of the page changing, or worse breaking, if we decide to change the template or add a parameter to it, which we may do in a minor version.

Thus, while it's not "hard" for us to make the change, it is felt that we might be promoting a bad practice and potentially be creating new problems by doing so. I have linked to this topic with our internal discussion on the matter.

Link to comment
Share on other sites

  • 3 weeks later...
On 27.2.2017 at 8:57 PM, bfarber said:

Development cannot make arbitrary changes, and some of the development team are not in agreement with the suggested changes. Specifically, as Mark indicated above

Thus, while it's not "hard" for us to make the change, it is felt that we might be promoting a bad practice and potentially be creating new problems by doing so. I have linked to this topic with our internal discussion on the matter.

Sorry for once again asking this, but I feel like I lack a straight answer and that my question has been forgotten. Every time I've asked, there seems like you've only answered the request from Adriano.

In: applications/forums/dev/html/front/forums/forumDisplay.phtml

Specifically: 

{template="forumButtons" group="forums" params="$forum"}

Could you please add the app="forums" here

The reason is I've made an alternative front page (with the Pages-application), where only some of the forums are to be listed, but they should be presented the same way forums are on the regular forum frontpage. 

This is my pages PHP code: 

{{$jsFile = \IPS\Output::i()->js('front_forum.js', 'forums' );}}

<script type='text/javascript' src='{$jsFile[0]|raw}' data-ips></script>
{{$commonForums = \IPS\forums\Forum::load(5205);}}
{template="forumDisplay" group="forums" app="forums" params="$commonForums, ''"}

{{$otherForumsToBeListed = \IPS\forums\Forum::load(5375)->children();}}
{{foreach $otherForumsToBeListed as $forum}}
	{template="forumDisplay" group="forums" app="forums" params="$forum, ''"}
{{endforeach}}

This gives an error in the default theme though, because the call to forumButtons inside forumDisplay doesn't specify the app, so it looks for it in the pages application theme folder. 

As soon as I add app="forums" in that template in my custom theme, this page works perfectly as expected. 

I don't see how the earlier posts here gives any arguments as to why this specific request has not been responded to, and I don't feel the responses to Adriano is relevant for my request. 

Thanks.

Link to comment
Share on other sites

1 hour ago, bfarber said:

I have raised the suggestion internally, and there is not universal agreement as to making this change, so unfortunately I cannot unilaterally make the requested change at this time. That is, unfortunately, the best I can say right now.

I can completely understand not wanting to change the app that it defaults to when you omit the app tag, since that is potentially backwards incompatible. However, I can't figure out why there are objections to explicitly stating the app for that one template, where it shouldn't be able to break things except in the most extreme edge case. What is preventing other staff members from agreeing with the suggestion, so that we can understand what potential issues we might face by making the change manually, or find workarounds?

Link to comment
Share on other sites

  • 5 months later...
On 21.3.2017 at 4:05 PM, bfarber said:

I have raised the suggestion internally, and there is not universal agreement as to making this change, so unfortunately I cannot unilaterally make the requested change at this time. That is, unfortunately, the best I can say right now.

I'm still waiting for this. 

What are the opposing arguments? What did you decide. This is clearly a forums-only and forums-specific template, hence the name "forumDisplay", and the included template is named "forumButtons". I can't see why someone would argue against changing from: 

{template="forumButtons" group="forums" params="$forum"}

to 

{template="forumButtons" app="forums" group="forums" params="$forum"}

in the forumDisplay-template. 

Please give me your logical reasoning for not changing this one specific request regarding forumDisplay-template. 

Link to comment
Share on other sites

On 2/24/2017 at 8:14 AM, Mark said:

Brandon is creating a changePassword template into your application so that line loads your template rather than the core one. There are several benefits to this approach:

  1. You'll probably want to change the template anyway so that you can link to a different location which will allow you to redirect the admin should they choose to change the password - otherwise, after changing it, they'll be taken to the edit member page, rather than remaining within your application.
  2. Templates are not designed to be reused by controllers in other applications. By reusing a template in the core, you run the risk of the page changing, or worse breaking, if we decide to change the template or add a parameter to it, which we may do in a minor version.
  3. It will be more efficient, as loading templates in your application is done automatically. Loading other templates creates additional queries.

If you are not concerned about any of those, you can just include it by doing something like this rather than copying/pasting the whole thing:


<ips:template parameters="$name, $memberId" />
{template="changePassword" app="core" group="members" params="$name, $memberId"}

Think of it sort of like how you do parent::method() in code hooks, but for templates :)

 

@TSP please see Mark's previous reply. Nothing has changed since then and unfortunately I have nothing further to add at this time.

Link to comment
Share on other sites

Archived

This topic is now archived and is closed to further replies.

  • Recently Browsing   0 members

    • No registered users viewing this page.
×
×
  • Create New...