PrestaShop / docs

PrestaShop technical documentation
https://devdocs.prestashop-project.org/
Other
120 stars 477 forks source link

A second fieldset with tabs shows broken on a module configuration page #557

Open bpato opened 4 years ago

bpato commented 4 years ago

Describe the bug

If you add a second fieldset with tabs on a module's configuration page, the first its ok but the second ones its broken.

this are the ids for the panels:

<div class="panel" id="fieldset_0">
...
<div class="panel" id="fieldset_1_1">
...

https://github.com/PrestaShop/PrestaShop/blob/12177c5efaad9abaf8fca5c669f4c0026ee4114a/admin-dev/themes/default/template/helpers/form/form.tpl#L48

the javascript that makes the tabs in the second case is searching for a wrong id:

$('#'+unique_field_id+'fieldset_'+index+' .form-wrapper')

https://github.com/PrestaShop/PrestaShop/blob/12177c5efaad9abaf8fca5c669f4c0026ee4114a/js/admin.js#L684-L687

prestashop-issue-bot[bot] commented 4 years ago

Thanks for opening this issue! We will help you to keep its state consistent

Matt75 commented 4 years ago

Well I always be against usage of Module::getContent() because since PrestaShop 1.5 module can create a ModuleAdminController where Helpers introduced in 1.5 are designed for.

Usage of helpers on Module::getContent() is a trick but not a good practice, it’s not initially designed for so it’s not working well...

Personally I use only Module::getContent to put a redirection to my main ModuleAdminController.

You can find example with HelperOption here :

I have to publish some example with HelperForm and HelperList but you can see example in legacy AdminController of PrestaShop 1.6 Core For example : https://github.com/PrestaShop/PrestaShop-1.6/blob/master/controllers/admin/AdminGendersController.php

MatShir commented 4 years ago

Hi @Matt75, Is there still an issue here?

bpato commented 4 years ago

little example, most part from https://devdocs.prestashop.com/1.7/modules/creation/adding-configuration-page/

<?php
// /modules/test/test.php

if (!defined('_PS_VERSION_')) {
    exit;
}

class Test extends Module
{
    public function __construct()
    {
        $this->name = 'test';
        $this->tab = 'front_office_features';
        $this->version = '1.0.0';
        $this->author = 'Firstname Lastname';
        $this->need_instance = 0;
        $this->ps_versions_compliancy = [
            'min' => '1.6',
            'max' => _PS_VERSION_
        ];
        $this->bootstrap = true;

        parent::__construct();

        $this->displayName = $this->l('My module Test');
        $this->description = $this->l('Description of my module.');

        $this->confirmUninstall = $this->l('Are you sure you want to uninstall?');

        if (!Configuration::get('MYMODULE_NAME')) {
            $this->warning = $this->l('No name provided');
        }
    }

    public function install()
    {

        return (parent::install());
    }

    public function uninstall()
    {
        return parent::uninstall();
    }

    public function getContent()
    {
        $output = null;

        if (Tools::isSubmit('submit'.$this->name)) {
            $myModuleName = strval(Tools::getValue('MYMODULE_NAME'));

            if (
                !$myModuleName ||
                empty($myModuleName) ||
                !Validate::isGenericName($myModuleName)
            ) {
                $output .= $this->displayError($this->l('Invalid Configuration value'));
            } else {
                Configuration::updateValue('MYMODULE_NAME', $myModuleName);
                $output .= $this->displayConfirmation($this->l('Settings updated'));
            }
        }

        return $output.$this->displayForm();
    }

    public function displayForm()
    {
        // Get default language
        $defaultLang = (int)Configuration::get('PS_LANG_DEFAULT');

        // Init Fields form array
        $fieldsForm[0]['form'] = [
            'legend' => [
                'title' => $this->l('Settings'),
            ],
            'tabs' => [
                'testtab0A' => 'testtab0A',
                'testtab0B' => 'testtab0B',
            ],
            'input' => [
                [
                    'type' => 'text',
                    'label' => $this->l('Configuration value testtab0A'),
                    'name' => 'MYMODULE_NAME',
                    'size' => 20,
                    'required' => true,
                    'tab' => 'testtab0A'
                ],
                [
                    'type' => 'text',
                    'label' => $this->l('Configuration value testtab0B'),
                    'name' => 'MYMODULE_NAME',
                    'size' => 20,
                    'required' => true,
                    'tab' => 'testtab0B'
                ]
            ],
            'submit' => [
                'title' => $this->l('Save'),
                'class' => 'btn btn-default pull-right'
            ]
        ];

        $fieldsForm[1]['form'] = [
            'legend' => [
                'title' => $this->l('Settings'),
            ],
            'tabs' => [
                'testtab1A' => 'testtab1A',
                'testtab1B' => 'testtab1B',
            ],
            'input' => [
                [
                    'type' => 'text',
                    'label' => $this->l('Configuration value testtab1A'),
                    'name' => 'MYMODULE_NAME',
                    'size' => 20,
                    'required' => true,
                    'tab' => 'testtab1A'
                ],
                [
                    'type' => 'text',
                    'label' => $this->l('Configuration value testtab1B'),
                    'name' => 'MYMODULE_NAME',
                    'size' => 20,
                    'required' => true,
                    'tab' => 'testtab1B'
                ]
            ],
            'submit' => [
                'title' => $this->l('Save'),
                'class' => 'btn btn-default pull-right'
            ]
        ];

        $helper = new HelperForm();

        // Module, token and currentIndex
        $helper->module = $this;
        $helper->name_controller = $this->name;
        $helper->token = Tools::getAdminTokenLite('AdminModules');
        $helper->currentIndex = AdminController::$currentIndex.'&configure='.$this->name;

        // Language
        $helper->default_form_language = $defaultLang;
        $helper->allow_employee_form_lang = $defaultLang;

        // Title and toolbar
        $helper->title = $this->displayName;
        $helper->show_toolbar = true;        // false -> remove toolbar
        $helper->toolbar_scroll = true;      // yes - > Toolbar is always visible on the top of the screen.
        $helper->submit_action = 'submit'.$this->name;
        $helper->toolbar_btn = [
            'save' => [
                'desc' => $this->l('Save'),
                'href' => AdminController::$currentIndex.'&configure='.$this->name.'&save'.$this->name.
                '&token='.Tools::getAdminTokenLite('AdminModules'),
            ],
            'back' => [
                'href' => AdminController::$currentIndex.'&token='.Tools::getAdminTokenLite('AdminModules'),
                'desc' => $this->l('Back to list')
            ]
        ];

        // Load current value
        $helper->fields_value['MYMODULE_NAME'] = Tools::getValue('MYMODULE_NAME', Configuration::get('MYMODULE_NAME'));

        return $helper->generateForm($fieldsForm);
    }
}

CapturaError

using a ModuleAdminController it shows correctly.

Matt75 commented 4 years ago

As use Helpers on Module::getContent() is a bad pratice, I thinks this is not an issue. Helpers were designed to works with AdminController Usage in Module::getContent() is a trick and don't works well.

bpato commented 4 years ago

perhaps the best thing to do is update the documentation and make clear that usig this method is a bad practice.

Matt75 commented 4 years ago

I will make improvement on modules docs when PaymentExample will be completed.

ftardio commented 4 years ago

Sure, it's better using AdminController. But most modules (even Prestashop's ones) are using helpers at getContent(). Also modules created using module generator from Prestashop's website use this method. And there is a bug, as bpato indicates. It's very simple: when loading views, helper is naming first panel as "fieldset_0", and second is named as "fieldset_1_1", while Javascript are supposing it name is "fieldset_1" (the logical name, following the sequence). So it seems to be simply a bug or issue.