Skip to content

Add dispatching of view_block_abstract_to_html_after event #3779

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Sep 28, 2016
Merged

Add dispatching of view_block_abstract_to_html_after event #3779

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4afef63
Add view_block_abstract_to_html_after event to toHtml() method
aleron75 Mar 14, 2016
05b40d1
Remove unused variable
aleron75 Mar 14, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/code/Magento/Catalog/Test/Unit/Block/Adminhtml/Product/Options/AjaxTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public function testToHtml()
->disableOriginalConstructor()
->setMethods(['dispatch'])
->getMock();
$eventManager->expects($this->once())->method('dispatch')->will($this->returnValue(true));
$eventManager->expects($this->exactly(2))->method('dispatch')->will($this->returnValue(true));

$scopeConfig = $this->getMockBuilder('\Magento\Framework\App\Config')
->setMethods(['getValue'])
Expand Down
2 changes: 1 addition & 1 deletion app/code/Magento/Catalog/Test/Unit/Block/Product/Widget/NewWidgetTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ public function testGetProductsCount()

protected function generalGetProductCollection()
{
$this->eventManager->expects($this->once())->method('dispatch')
$this->eventManager->expects($this->exactly(2))->method('dispatch')
->will($this->returnValue(true));
$this->scopeConfig->expects($this->once())->method('getValue')->withAnyParameters()
->willReturn(false);
Expand Down
7 changes: 6 additions & 1 deletion app/code/Magento/Paypal/Test/Unit/Block/Billing/AgreementsTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,9 +227,14 @@ public function testGetWizardPaymentMethodOptions()
public function testToHtml()
{
$this->eventManager
->expects($this->once())
->expects($this->at(0))
->method('dispatch')
->with('view_block_abstract_to_html_before', ['block' => $this->block]);
$transport = new \Magento\Framework\DataObject(['html' => '']);
$this->eventManager
->expects($this->at(1))
->method('dispatch')
->with('view_block_abstract_to_html_after', ['block' => $this->block, 'transport' => $transport]);
$this->scopeConfig
->expects($this->once())
->method('getValue')
Expand Down
5 changes: 4 additions & 1 deletion app/code/Magento/Persistent/Test/Unit/Block/Header/AdditionalTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,9 +261,12 @@ public function testToHtml($customerId)
$this->additional->setData('cache_lifetime', 789);
$this->additional->setData('cache_key', 'cache-key');

$this->eventManagerMock->expects($this->once())
$this->eventManagerMock->expects($this->at(0))
->method('dispatch')
->with('view_block_abstract_to_html_before', ['block' => $this->additional]);
$this->eventManagerMock->expects($this->at(1))
->method('dispatch')
->with('view_block_abstract_to_html_after');
$this->scopeConfigMock->expects($this->once())
->method('getValue')
->with(
Expand Down
8 changes: 4 additions & 4 deletions ...e/Magento/Widget/Test/Unit/Block/Adminhtml/Widget/Instance/Edit/Chooser/ContainerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public function testToHtmlCatalogProductsListGroupedProduct()
. 'Main Content Area</option><option value="content.bottom" >Main Content Bottom</option>'
. '<option value="content.top" >Main Content Top</option></select>';

$this->eventManagerMock->expects($this->once())->method('dispatch')->willReturn(true);
$this->eventManagerMock->expects($this->exactly(2))->method('dispatch')->willReturn(true);
$this->scopeConfigMock->expects($this->once())->method('getValue')->willReturn(false);

$this->themeCollectionFactoryMock->expects($this->once())
Expand Down Expand Up @@ -153,7 +153,7 @@ public function testToHtmlCatalogCategoryLinkSimpleProduct()
. '<option value="sidebar.additional" >Sidebar Additional</option>'
. '<option value="sidebar.main" >Sidebar Main</option></select>';

$this->eventManagerMock->expects($this->once())->method('dispatch')->willReturn(true);
$this->eventManagerMock->expects($this->exactly(2))->method('dispatch')->willReturn(true);
$this->scopeConfigMock->expects($this->once())->method('getValue')->willReturn(false);

$this->themeCollectionFactoryMock->expects($this->once())
Expand Down Expand Up @@ -284,7 +284,7 @@ public function testToHtmlCmsStaticBlockAllProductTypes()
. '<option value="sidebar.additional" >Sidebar Additional</option>'
. '<option value="sidebar.main" >Sidebar Main</option></select>';

$this->eventManagerMock->expects($this->once())->method('dispatch')->willReturn(true);
$this->eventManagerMock->expects($this->exactly(2))->method('dispatch')->willReturn(true);
$this->scopeConfigMock->expects($this->once())->method('getValue')->willReturn(false);

$this->themeCollectionFactoryMock->expects($this->once())
Expand Down Expand Up @@ -402,7 +402,7 @@ public function testToHtmlOrderBySkuAllPages()
. '<option value="sidebar.additional" >Sidebar Additional</option><option value="sidebar.main" >'
. 'Sidebar Main</option></select>';

$this->eventManagerMock->expects($this->once())->method('dispatch')->willReturn(true);
$this->eventManagerMock->expects($this->exactly(2))->method('dispatch')->willReturn(true);
$this->scopeConfigMock->expects($this->once())->method('getValue')->willReturn(false);

$this->themeCollectionFactoryMock->expects($this->once())
Expand Down
9 changes: 9 additions & 0 deletions lib/internal/Magento/Framework/View/Element/AbstractBlock.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -665,6 +665,15 @@ public function toHtml()
}
$html = $this->_afterToHtml($html);

/** @var \Magento\Framework\DataObject */
$transportObject = new \Magento\Framework\DataObject(
Copy link
Contributor

Choose a reason for hiding this comment

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

There are at least 2 reasons not to trigger this event in AbstractBlock:

  1. Abstract block is overloaded with behavior
  2. If some block will just implement BlockInterface (not extend from AbstractBlock), this event will not be called.

Overall approach of Magento 2 is to use composition instead of inheritance to reuse behavior. In this case you need to reuse behavior common for all toHtml() calls. Please consider putting this behavior to \Magento\Framework\View\Layout::renderNonCachedElement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your comment.

I followed the example of view_block_abstract_to_html_before event which is dispatched at the beginning of the toHtml() method: in what does the approach of dispatching this event differ from mine?

Regarding your suggestion of putting the event in the \Magento\Framework\View\Layout::renderNonCachedElement do you suggest to add both a before and an after event?

Furthermore, would you suggest to differentiate the event based on the element (UI component, Block or Container) or should I keep the same event for all three?

Cheers,
Alessandro

Copy link
Contributor

Choose a reason for hiding this comment

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

I looked closer at implementation. Please disregard my comment.

While it would be more correct to put triggering of both before- and after- events in layout, Magenot is not ready for that, since in many places Block::toHtml() is triggered directly.

[
'html' => $html,
]
);
$this->_eventManager->dispatch('view_block_abstract_to_html_after', ['block' => $this, 'transport' => $transportObject]);
$html = $transportObject->getHtml();

return $html;
}

Expand Down