[#3793] - Support adding a priority queue for JLayout paths
- Closed
- 18 Jun 2014
- Medium
- Build: staging
- # 3793
- Diff
- wilsonge:patch-6
- Success The Travis CI build passed Details
User tests: Successful: Unsuccessful:
@phproberto see what you think
Testing Instructions
- Test current JLayouts continue to work as expected with no new issues
- Test creating a JLayout in a weird file path and then add using a priority queue. berto see what you think
Testing Instructions
Test current JLayouts continue to work as expected with no new issues
Test using a priority queue works
e.g. Move the layout in /root/layouts/joomla/content/icons.php to a new location (not something that is currently recognized). Then edit the following https://github.com/joomla/joomla-cms/blob/staging/components/com_content/views/article/tmpl/default.php#L67
$layout = new JLayout('joomla.content.icons');
$paths = new SplPriorityQueue;
$paths->insert('path/to/new/location');
$layout->addIncludePaths($paths);
$layout->render(array('params' => $params, 'item' => $this->item, 'print' => false));
And the Jlayout should load from the location path/to/new/location/joomla/content/icons.php
If you want as a further test you can do the same thing except change $paths from being a SplPriorityQueue to being an array by changing the previous code to
$layout = new JLayout('joomla.content.icons');
$paths = array('path/to/new/location');
$layout->addIncludePaths($paths);
$layout->render(array('params' => $params, 'item' => $this->item, 'print' => false));
and checking this 'old' way of doing things also works as expected
FAQ
The reason I made the private function is because in the future a setIncludePaths() function will likely be added that will need to utilise the same code and therefore I'm abstracting it away in a method in anticipation of this.
| Description | ⇒ | <p><a href="https://github.com/phproberto" class="user-mention">@phproberto</a> see what you think</p> | |
| Labels |
Added:
?
?
|
||
Well actually this should be the default use case in my opinion. Because this include array is literally a set of prioritised paths. We have to use a host of functions here to shove stuff to the top of the array and on the correct order. But really that's exactly what the priority queue is for.
That's also why it's used in the new MVC as a set of default paths https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/view/html.php#L43 which is going to be my use case
I agree that the way to go is SplPriorityQueue that should be there from the begining of JLayout.
Using it would have saved us code to deal with paths. But now we have everything working with arrays so now not sure what would be better.
The ideal solution would be the reverse way with SplPriorityQueue as the base of the system and maybe array support. But that would break BC for classes that extend JLayoutFile now.
That's why I think that we maybe shouldn't change this now. We talked about creating a rendering system based on JLayout for the framework. That would be the best place as it won't break BC and we can start using it everywhere.
Does that make sense for you?
I'm happy doing that :) I just thought it would make more sense to have this code in the main class. But it's not a problem dumping the function into the renderer class
| Status | New | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2014-06-18 00:26:36 |
What is the use case where you would use a queue instead of just setting the include paths directly?
Sounds a bit overly complicate to create queues only to convert it back to an array. I understand queues more as a way to queue tasks, not as a storage of paths.
But maybe I miss something ;)