|
[
Permalink
| « Hide
]
Jamie Pratt added a comment - 01/May/08 08:38 PM
Added Tim, Petr and Martin D here as I want your opinions on whether this seems a sensible way of allowing activities to add more setting pages to the site admin menu.
// add a top level category for us to add stuff to
$ADMIN->add('root', new admin_category('racp', get_string('racp','local'))); $ADMIN->add('racp', new admin_externalpage('curriculum', get_string('editcurriculum', 'local'), $ADMIN->add('racp', new admin_externalpage('modsettinglplan', get_string('adminnotifications', 'lplan'), $ADMIN->add('racp', new admin_externalpage('authsettingdb', get_string('configauthdb', 'local'), I understand how to add categories to the admin tree. The problem is the code to construct the tree is only looking in settings.php for form fields to add to the module settings page. See the code fragments below :
in the class file for admin_root : /**
and in admin/settings/plugins.php we have : if ($ADMIN->fulltree) { include($CFG->dirroot.'/mod/'.$modulename.'/settings.php'); } which means that we can only use settings.php to add stuff to one settings page for each module. Also I should add that the quiz module is not using settings.php but config.html. The settings form for the quiz module could not be laid out in a user friendly way using the admin menu methods to construct a form.
So I can see that it might be suggested that each module uses only one form and plugins could add settings to that form. But this would only be possible if the plug-in was using settings.php to add form fields to the form. Having to use settings.php instead of config.html would be a big set back in terms of usability of the form. Um, perhaps I'm missing something but why don't you just put some code in mod/quiz/settings.php to cycle through the reports and include settings.php files that might be found there?
The problem is the if condition I mention above :
if ($ADMIN->fulltree) { include($CFG->dirroot.'/mod/'.$modulename.'/settings.php'); } It precludes having anything in settings.php or included from settings.php but form fields within one page. an alternative is to move the :
if ($ADMIN->fulltree) { condition into settings.php I suppose. But this might break some custom modules. Better not too change api but to add to it?? Also moving the : if ($ADMIN->fulltree) { into settings.php would not work for the quiz module that is still using config.html as I mention above. OK, I understand the problem better now. I think the approach you took is basically sound but I'll leave it to Petr for a final review.
Sorry I just thought we can still use an admin_externalpage from settings.php and use config.html. So the quiz module could use settings.php if we were to decide to move the if ($ADMIN->fulltree) { into settings.php.
Look forward to Petr's comments. hmm, I personally did not like what I saw in quiz/config.htm - I hoped somebody will convert it to settings.php before 1.9 goes gold and I still think that it should be done.
I propose: I asked Petr for clarification of what he meant in his last comment and he said :
"let's have both settings.php and settingstree.php - and legacy option in 1.9 - but always only one of them" He is in favour of removing config.html altogether before 2.0. some more clarification - I like the screenshot!
and I do not like config.html Just a note that this reminded me of.. last time I checked we hadn't produced any documentation on (or even a mention of) the settings.php structure for module authors.
Jamie and Petr seem to have reached a consensus. And I think it is a good one.
The only quesiton is who has time to change quiz/config.html to settingstree.php. That seems to be Fixed in HEAD and 1.9 branch. I added a TODO comment to delete the if branch that allows legacy config.html which will be deleted as soon as someone migrates quiz/config.html
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Improvement
Closed
Minor
allow activity modules to add sub menus of settings pages to admin menu
 |
|
