Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to define module front controllers layout #7780

Merged
merged 2 commits into from
Jun 7, 2017
Merged

Allow to define module front controllers layout #7780

merged 2 commits into from
Jun 7, 2017

Conversation

tonyyb
Copy link

@tonyyb tonyyb commented Apr 13, 2017

Re-open : #7771 with good convention name.

Questions Answers
Branch 1.7.x
Description? Currently it is not possible to define the layout of a module front controller.
Type? improvement
Category? FO
BC breaks? no
Deprecations? no
Fixed ticket? -
How to test? Try to define the layout of a module from AdminThemes back office controller.

@prestonBot
Copy link
Collaborator

Hello tonyyb!

This is your first pull request on the PrestaShop project. Thank you, and welcome to this Open Source community!

@xBorderie
Copy link
Contributor

👍

@@ -1308,6 +1308,9 @@ protected function getThemeDir()
public function getLayout()
{
$entity = $this->php_self;
if (empty($entity) && $this->controller_type === 'modulefront' && !empty($this->page_name)) {
$entity = $this->page_name;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @tonyyb,

What do you think if we just use $this->getPageName()?

$entity = $this->php_self;
if (empty($entity)) {
    $entity = $this->getPageName();
}

It seems to handle even more cases:

    public function getPageName()
    {
        // Are we in a payment module
        $module_name = '';
        if (Validate::isModuleName(Tools::getValue('module'))) {
            $module_name = Tools::getValue('module');
        }

        if (!empty($this->page_name)) {
            $page_name = $this->page_name;
        } elseif (!empty($this->php_self)) {
            $page_name = $this->php_self;
        } elseif (Tools::getValue('fc') == 'module' && $module_name != '' && (Module::getInstanceByName($module_name) instanceof PaymentModule)) {
            $page_name = 'module-payment-submit';
        } elseif (preg_match('#^'.preg_quote($this->context->shop->physical_uri, '#').'modules/([a-zA-Z0-9_-]+?)/(.*)$#', $_SERVER['REQUEST_URI'], $m)) {
            // @retrocompatibility Are we in a module ?
            $page_name = 'module-'.$m[1].'-'.str_replace(array('.php', '/'), array('', '-'), $m[2]);
        } else {
            $page_name = Dispatcher::getInstance()->getController();
            $page_name = (preg_match('/^[0-9]/', $page_name) ? 'page_'.$page_name : $page_name);
        }

        return $page_name;
    }

@xBorderie
Copy link
Contributor

Hey @tonyyb ! What do you think of @Quetzacoalt91 's suggestion above?

@tonyyb
Copy link
Author

tonyyb commented Jun 7, 2017

Hi @xBorderie @Quetzacoalt91 : I think that's a good idea ! 👍

@Quetzacoalt91 Quetzacoalt91 changed the title FO: Allow to define module front controllers layout Allow to define module front controllers layout Jun 7, 2017
@xBorderie
Copy link
Contributor

Thank you!

Could you try and squash those commits into just one? Another explanation of squashing.

@Quetzacoalt91 Quetzacoalt91 merged commit 8014999 into PrestaShop:develop Jun 7, 2017
@Quetzacoalt91
Copy link
Member

This is okay to keep the history like this @xBorderie, that's why I merge it. ;)

Thanks @tonyyb.

@xBorderie
Copy link
Contributor

Fine by me :)

@xBorderie xBorderie added this to the 1.7.2.0 milestone Jun 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants