User tests: Successful: Unsuccessful:
I'm not sure if I'm doing this right, but I've updated the JControllerLegacy class to make it easier to use polymorphic models without overriding existing functions.
The only change is to the display function and shouldn't present any backwards compatibility issues, because all of the changes were to local scope variables.
Please test and let me know what you think.
Labels |
Added:
?
|
How about using hook? It will be more safe for BC and easy to use.
public function display($cachable = false, $urlparams = array())
{
// ...
$view = $this->getView($viewName, $viewType, '', array('base_path' => $this->basePath, 'layout' => $viewLayout));
// Get/Create the model
if ($model = $this->getModel($viewName))
{
// Push the model into the view (as default)
$view->setModel($model, true);
}
// Assign model hook
$this->assignViewModel($view);
// ...
}
/**
* A hook to seet multiple models to view for display task.
*
* @param JViewLegacy $view The view object.
*
* @return void
*/
public function assignViewModel(JViewLegacy $view)
{
// Override this method and set other models here.
}
Yes, that is the basic idea. Although it is currently possible to achieve this by simply overriding the display function, you have to copy paste the cache code or give up the benefits of caching.
Sincerely,
Mathew Lenning
Babel-university.com
P.S. This message was sent via iPhone, so please forgive any errors
On Jan 12, 2014, at 11:38 PM, Simon Asika notifications@github.com wrote:
So you hope to do this?
public function display($cachable = false, $urlparams = array())
{
$this->view = $this->getView('view', 'type');
$model = $this->getModel('foo');$this->view->setModel($model); parent::display($cachable, $urlparams);
}
to support multiple models in view?\
Reply to this email directly or view it on GitHub.
The problem with your hook is that for the view to use the model it would have to know what model it is using or worse initiate the default and then overwrite the default.
Also it doesn't allow the developer to do the switch using the task system.
With the proposed change, I could have different tasks that load different models depending on the action without using switches or if else statements.
Sincerely,
Mathew Lenning
Babel-university.com
P.S. This message was sent via iPhone, so please forgive any errors
On Jan 12, 2014, at 11:47 PM, Simon Asika notifications@github.com wrote:
How about using hook? It will be more safe for BC and easy to use.
public function display($cachable = false, $urlparams = array())
{
// ...$view = $this->getView($viewName, $viewType, '', array('base_path' => $this->basePath, 'layout' => $viewLayout)); // Get/Create the model if ($model = $this->getModel($viewName)) { // Push the model into the view (as default) $view->setModel($model, true); } // Assign model hook $this->assignViewModel($view); // ...
}
/**
Just one more point before going to bed (12am in JP) there is no BC concerns, because the only change is making a local scope var into a class property. And since the conditional only sets the class property if it is empty, this won't even effect 3pd who implemented a $veiw property in their extended controllers
Sincerely,
Mathew Lenning
Babel-university.com
P.S. This message was sent via iPhone, so please forgive any errors
On Jan 12, 2014, at 11:47 PM, Simon Asika notifications@github.com wrote:
How about using hook? It will be more safe for BC and easy to use.
public function display($cachable = false, $urlparams = array())
{
// ...$view = $this->getView($viewName, $viewType, '', array('base_path' => $this->basePath, 'layout' => $viewLayout)); // Get/Create the model if ($model = $this->getModel($viewName)) { // Push the model into the view (as default) $view->setModel($model, true); } // Assign model hook $this->assignViewModel($view); // ...
}
/**
Actually, there can be issues with 3rd parties if they defined for example a public $view or used it for something else.
But it's not really a BC issue per definition.
I would actually prefer a hook variant like Asika suggested.
I do a multimodel controller myself in my extension like this: https://github.com/Bakual/SermonSpeaker/blob/master/com_sermonspeaker/site/controller.php#L48
I don't have to do the caching stuff myself, I just call the getView in my display function before I call parent::display. It seems to work quite well so far but of course I'd prefer a simpler way
Hook is a way but not the best. Actually, the real problem is that we just use only ONE base controller to dispatch all views, so we are hard to assign models to different view, we have to use if or switch statements.
However, the display()
method has blocked in JControllerAdmin
:
https://github.com/joomla/joomla-cms/blob/staging/libraries/legacy/controller/admin.php#L161
So we are not able to override it in sub-controllers. But I don't know what's the reason to block display() in this class?
So we are not able to override it in sub-controllers. But I don't know what's the reason to block display() in this class?
JControllerAdmin is used for forms I think. And there you don't want a display function. You always go over add/edit.
One of the benefits of this change is that using different views with one base controller would be easier. Just assign a different task for each view/model pair.
function taskOne()
{
$this->view = $this->getView('taskOne', $viewType, '', array('base_path' => $this->basePath, 'layout' => $viewLayout));
// Get/Create the model
if ($model = $this->getModel('taskOne'))
{
// Push the model into the view (as default)
$this->view->setModel($model, true);
}
parent::display($cachable, $urlparams);
}
function taskTwo()
{
$this->view = $this->getView('taskTwo', $viewType, '', array('base_path' => $this->basePath, 'layout' => $viewLayout));
// Get/Create the model
if ($model = $this->getModel('taskTwo'))
{
// Push the model into the view (as default)
$this->view->setModel($model, true);
}
parent::display($cachable, $urlparams);
}
Then you just need to use the controller.task syntax to display the various views using one controller.
Status | New | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2014-01-13 08:46:49 |
Labels |
Added:
?
|
Status | Closed | ⇒ | New |
O.k. So the close button isn't for the comment window. Learn something new everyday right.
Status | New | ⇒ | Closed |
Closed_Date | 2014-01-13 08:46:49 | ⇒ | 2014-04-24 23:12:20 |
So you hope to do this?
to support multiple models in view?