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

Load all child blocks in Adminhtml Head block #3222

Merged
merged 1 commit into from
May 9, 2023

Conversation

elidrissidev
Copy link
Member

@elidrissidev elidrissidev commented Apr 27, 2023

Description (*)

First roadblock I encountered when creating the extension mentioned in #3219, I can't add a custom child block to admin head 😢, and since the content is dynamically created through calling PHP code I can't use addJs either.

I don't know why this change wasn't done before, maybe it caused some issue? but it seems to work fine for me.

Manual testing scenarios (*)

  1. Go to layout xml of any module and try to add a child block to adminhtml head, in my case:
...
<default>
    <reference name="head">
        <block type="elidrissidev_debugbar/head" name="debugbar_head" />
    </reference>
</default>
...

Should be loaded after this change.

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All automated tests passed successfully (all builds are green)
  • Add yourself to contributors list

@github-actions github-actions bot added Component: Adminhtml Relates to Mage_Adminhtml Component: Page Relates to Mage_Page Template : admin Relates to admin template labels Apr 27, 2023
Copy link
Contributor

@fballiano fballiano left a comment

Choose a reason for hiding this comment

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

since the getchildhtml outputs all of the children if no name is passed, it seems good to me.

did a brief test and worked fine

@fballiano
Copy link
Contributor

I wonder if there may be other blocks that were not printed before and that now get printed and that could cause problems.

@elidrissidev
Copy link
Member Author

elidrissidev commented Apr 27, 2023

The only block I see is directory/js/optional_zip_countries.phtml which is being added to the head in multiple places. Just checked, there doesn't seem to be anything unusual, only optional_zip_countries and head.calendar are loaded inside the head.

@elidrissidev
Copy link
Member Author

Do you think we should backport this to v19 for the next release?

@fballiano
Copy link
Contributor

it wouldn't cause any harm, absolutely, but I think we shouldn't anyway :-)

@elidrissidev
Copy link
Member Author

I found a workaround by observing core_block_abstract_to_html_after and appending the html when it's the Head block:

    public function addDebugbarHeadToAdminhtml(Varien_Event_Observer $observer)
    {
        $block = $observer->getEvent()->getBlock();

        if (!$block instanceof Mage_Page_Block_Html_Head) {
            return;
        }

        /** @var Varien_Object $transport */
        $transport = $observer->getEvent()->getTransport();
        $renderer = Mage::getSingleton('elidrissidev_debugbar/debugbar')
            ->getDebugbar()
            ->getJavascriptRenderer();

        $transport->setHtml($transport->getHtml() . $renderer->renderHead());
    }

Although not as clean, at least it doesn't require to rewrite the block.

@fballiano
Copy link
Contributor

sure, but I think this PR still makes a lot of sense :-)

@fballiano fballiano requested review from kiatng and addison74 May 9, 2023 12:29
@fballiano fballiano merged commit 73d4f48 into OpenMage:main May 9, 2023
@elidrissidev elidrissidev deleted the feat/adminhtml-head-childs branch May 25, 2023 10:13
@luigifab
Copy link
Contributor

Is there a general issue now?

do:

<?php echo $this->getChildHtml('debugbar_head') ?>
<?php echo $this->getChildHtml() ?>

and the debugbar is displayed 2x

do:

<?php echo $this->getChildHtml('debugbar_head') ?>
<?php echo $this->getChildHtml() ?>
<?php echo $this->getChildHtml() ?>

and the debugbar is displayed 3x

Or it's me? and it's already the time to travel again!

@elidrissidev
Copy link
Member Author

@luigifab I think that's the intended behaviour, $this->getChildHtml() will return HTML for all children, whereas $this->getChildHtml('debugbar_head') will return HTML of debugbar_head block only.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Adminhtml Relates to Mage_Adminhtml Component: Page Relates to Mage_Page new feature Template : admin Relates to admin template
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants