Colonel_mortis Posted December 8, 2016 Posted December 8, 2016 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.
bfarber Posted December 8, 2016 Posted December 8, 2016 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.
teraßyte Posted December 8, 2016 Posted December 8, 2016 +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.
TSP Posted January 15, 2017 Posted January 15, 2017 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
TSP Posted February 7, 2017 Posted February 7, 2017 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"}
Adriano Faria Posted February 14, 2017 Posted February 14, 2017 @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.
Adriano Faria Posted February 24, 2017 Posted February 24, 2017 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.
bfarber Posted February 24, 2017 Posted February 24, 2017 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.
Adriano Faria Posted February 24, 2017 Posted February 24, 2017 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 ?
Mark Posted February 24, 2017 Posted February 24, 2017 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: 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. 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. 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
Adriano Faria Posted February 24, 2017 Posted February 24, 2017 That would simply show the template twice: Nvm. I'll try to make an workaround by overloading process method.
TSP Posted February 27, 2017 Posted February 27, 2017 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.
teraßyte Posted February 27, 2017 Posted February 27, 2017 I guess I am confused, what kind of agreement do you need in order to add a string in a template call to specify the application/location?
Adriano Faria Posted February 27, 2017 Posted February 27, 2017 Just now, teraßyte said: what kind of agreement do you need in order to add a string in a template call to specify the application/location? Yeah, that's what I asked and am waiting a reply till then. What's the purpose of this forum/topic anyway ? Just asking to do something that should be there since 4.0.0!
bfarber Posted February 27, 2017 Posted February 27, 2017 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.
TSP Posted March 21, 2017 Posted March 21, 2017 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.
bfarber Posted March 21, 2017 Posted March 21, 2017 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.
Colonel_mortis Posted March 21, 2017 Author Posted March 21, 2017 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?
TSP Posted September 20, 2017 Posted September 20, 2017 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.
bfarber Posted September 20, 2017 Posted September 20, 2017 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: 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. 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. 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.
Recommended Posts
Archived
This topic is now archived and is closed to further replies.