ghengeveld / cot-admin-bootstrap

[unmaintained] Bootstrap admin theme for Cotonti
14 stars 3 forks source link

Custom config pages for extension #11

Open macik opened 11 years ago

macik commented 11 years ago

Not working custom configuration pages in Extension configs. As example see «News» extension. Look at this page with standard and bootstrap admin theme. Sample link: example.com/admin/config?n=edit&o=plug&p=news

macik commented 11 years ago

Also we need to add {FOOTER_RC} on footer.tpl and {HEADER_HEAD} in the head.

ghengeveld commented 11 years ago

Could you provide more info about the issue with news extension? I'm not sure what you mean. I don't see a difference in functionality between default and bootstrap theme. Perhaps you can provide a screenshot of what it should be like.

macik commented 11 years ago

See screenshots: standard skin - http://static.galaxyhost.org/cotonti/news_std_2012-10-18_16-48_Administration.png priori skin - http://static.galaxyhost.org/cotonti/news_priori_2012-10-18_16-49_Administration.png bootstrap skin - http://static.galaxyhost.org/cotonti/news_bsa_2012-10-18_16-46_Administration.png

In bootstrap «we loose» important configuration block.

ghengeveld commented 11 years ago

I must've missed this feature, because I haven't seen that custom config section before. I'll upgrade my local Cotonti copy and fix it. Or is it a plugin?

ghengeveld commented 11 years ago

After some investigation I've concluded that News is an exception. It uses a hack (with javascript) in order to replace part of it's config form with a custom template. We're going to modify the core config code to allow custom templates for the config screen, in order to allow extensions to override the default config template. This way the news plugin won't have to use javascript to modify its config screen. After this is done, I'll create a custom template for news config so it will fit in with the bootstrap theme.

macik commented 11 years ago

It's not a hack, it's just a common way to use sub template implode and clientside javascript to improve UI of config pages. I this plugin not an exception. (Only one exception that this plugin comes with «the box».)
I use the same methods in several of my public and private plugins.

Look to one of them — social_share: https://github.com/macik/cot-social_share

It's working fine with AdminBootstrap - look at conf page.

So my opinion - making core modifications is unnecessary. And it really too much - mod core, than mod news plagin, than making custom tamplate. It's overhead.

I think we only need to trace a bug and fixed that part in news extension.

ghengeveld commented 11 years ago

Fixing it is easy, it's simply a matter of adding id="saveconfig" to the form in admin.config.tpl. Nevertheless, allowing plugins to provide a custom template for their config page is a feature which a lot of extensions could benefit from, so it makes sense to include support for this in Cotonti itself. Assuming plugin developers will go so far as to write javascript to modify the config page is far-fetched and unrealistic. You and esclkm may do it, but others may not have the skills to do that.

It's a hack because javascript is being used for a purpose it's not supposed to be used for. Furthermore, it will break functionality when javascript is disabled, because the config section is hidden by default (it's moved and showed by javascript instead).

I'm not saying I won't provide support for the way the news plugin modifies its config page, I'm just saying that we're going to provide a cleaner solution (which doesn't require javascript) for this as well.

macik commented 11 years ago

If speaks about JS switched off - this case it's not important as there are two scenarios:

  1. we render simple extended form (already visible), by hook admin.config.edit.loop . in this case we dont need JS at all.
  2. we renders extended dynamic form (already visible) by hook admin.config.edit.loop as sicial_share do. In this case we can not manage form without JS at all. Regardless it render by admin.config.edit.loop or own admin.config.tpl used.

If we speak about develop complexity I'm agree that custom admin.config.tpl should be simplier adoption for a wider audience. But I'm not sure we can easy provide support of this custom templates with different admin themes that user wants to switch. As an end point developer of extension must support possible misfits of extension and themes. Theme maker can not foresee all range of possible plugins. It's easier to maintain and support on programing (php) side then in TPL files.


by the way, some weeks ago I post idea (in russian part of forum) to implement custom user functions for extension config vars (like we have callback type). The main idea to call 2 functions for specified config var: one for renders custom UI element for that var, second one calls for import data right before saving data in DB.

plug_var=01:user_def:my_new_ui():def_value:Simple user defined UI input

Then 2 functions called: my_new_ui_create() - for renders it and my_new_ui_import() for import and filtering values.

It's a flexible way:

What do you think?

ghengeveld commented 11 years ago

I think the current way configuration settings are defined can be greatly improved. Currently it relies on parsing a specific format to define the config form fields. Personally I'd like to get rid of this custom string format and instead use a plain PHP array. This would be similar to the way columns are defined in a CotORM Model: https://github.com/GHengeveld/cot-factory/wiki/ORM#wiki-models

When the config definition is done in PHP, it will be easy to add support for custom functions (I'd call them callbacks). For example:

$R['my_custom_config_row_resource'] = '<tr><th>{$caption}</th><td>{$field}</td></tr>';

$configfields = array(
    'plug_var' => array(
        'order' => 1,
        'type' => 'callback',
        'callback' => 'my_new_ui',
        'default_value' => 'def_value',
        'caption' => 'Simple user defined UI input'
    ),
    'another_var' => array(
       'order' => 2,
       'type' => 'string',
       'resource' => 'my_custom_config_row_resource',
       'default' => 'def_value',
       'caption' => 'Simple user defined UI input'
    ),
    'some_radio_field' => array(
        'type' => 'radio',
        'options' => array(
            'option1' => 'Option 1',
            'option2' => 'Option 2',
            'option3' => 'Option 3',
        ),
        'default' => 'option2',
        'caption' => 'Three radiobuttons'
    )
);

function my_new_ui() { ... }

In this example you see a type 'callback', which can be any function in global scope or a static method of a class (MyClass::my_callback). This callback would be able to overwrite the entire config row, use a custom tpl file or whatever you want. In the second field definition there's a 'resource' property which is a link to a key in the $R array, which is already being used for a lot of other html stuff. The good thing about $R is that you can easily override it's values from within an extension or theme. Basically it would provide an easy way to allow custom HTML to be used.

The great thing about this is that it's a lot easier to read than the existing config strings. Furthermore it provides increased flexibility because it will allow the usage of variables, functions and other PHP stuff in the config field definition. As you see, many field properties are optional (unlike in the current string format), which both makes it easier to write and more flexible to use.

macik commented 11 years ago

Seems pretty enough. But it takes some work to convert existing plugins for it. And applicable with increasing major verion. And may be needs some kind compatibility module to support old style plugs for a while (like with Genoa plugs). And don;t forget custom import/filter functions.

ghengeveld commented 11 years ago

The old format can still be supported as a fallback for existing extensions.

trustmaster commented 11 years ago

Current config format won't be replaced with plain PHP. We had a poll about it before upgrading the configs for Siena and the current format took a certain lead because for most cases it is by far more concise.

But have a look at Configuration API one day. It already has something for you.

ghengeveld commented 11 years ago

If I understand correctly, cot_config_add() can be used in the setup file instead of the regular config block to create config fields upon installation. Worth investigating.

trustmaster commented 11 years ago

Right. And it is actually used to add them normally after parsing the setup-file meta information.