Add a head.additional block to adminhtml layout #28389

gsomoza · 2020-05-27T13:25:17Z

Module developers are currently able to add external JS files to the admin area's <head> section, but adding inline scripts is not as straightforward as it is in the Frontend area because the admin layout doesn't define the head.additional block. In fact, I couldn't find a way to add inline scripts to the head in a way that's not "hackish" and remains compatible with other modules. The mechanism to do this easily is simply missing.

This PR addresses the mechanism issue: once this block is added \Magento\Framework\View\Result\Page will make sure all head.additional child blocks are rendered, mirroring the functionality in the Frontend area.

Description (*)

This PR simply adds a head.additional block of type \Magento\Framework\View\Element\Text\ListText to the Adminhtml's default layout (via magento2/module-base).

Related Pull Requests

None

Fixed Issues (if relevant)

I couldn't find any.

Manual testing scenarios (*)

Create a module that adds the following layout update in the adminhtml area:

<?xml version="1.0"?>
<page xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
      xsi:noNamespaceSchemaLocation="urn:magento:framework:View/Layout/etc/page_configuration.xsd">
    <referenceBlock name="head.additional">
        <block class="Magento\Framework\View\Element\Template" name="test"
               template="Magento_Theme::html/copyright.phtml"/>
    </referenceBlock>
</page>

You should see the copyright text rendered within any admin page's <head> area.

(which is invalid HTML but should work as a quick test)

Other / Reasoning

Why a `ListText` block?

Because a container renders its children within a tag, and we don't want that inside head.
Because in the Frontend area this is done with a Template block with a container.phtml template is used, but that template doesn't exist in the adminhtml area.
Because ListText simply gets the job done exactly as it's supposed to and without any side-effects.

Backwards Compatibility

This change adds a new block to an existing layout and therefore shouldn't break backwards compatibility with any existing code.

Automated tests?

I'd be happy to create them if you really think it's necessary but I'll need a bit of guidance. However, I also doubt this is the kind of things we're testing, because existing tests for \Magento\Framework\View\Result\Page and \Magento\Framework\View\Result\Page would essentially cover most (if not all) requirements for this functionality.

Contribution checklist (*)

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/integration tests (if applicable)
All automated tests passed successfully (all builds are green)

Resolved issues:

resolves [Issue] Add a head.additional block to adminhtml layout #29165: Add a head.additional block to adminhtml layout

m2-assistant · 2020-05-27T13:25:19Z

Hi @gsomoza. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

@magento give me test instance - deploy test instance based on PR changes
@magento give me 2.4-develop instance - deploy vanilla Magento instance

❗ Automated tests can be triggered manually with an appropriate comment:

@magento run all tests - run or re-run all required tests against the PR changes
@magento run <test-build(s)> - run or re-run specific test build(s)
For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names. Allowed build names are:

Database Compare
Functional Tests CE
Functional Tests EE,
Functional Tests B2B
Integration Tests
Magento Health Index
Sample Data Tests CE
Sample Data Tests EE
Sample Data Tests B2B
Static Tests
Unit Tests
WebAPI Tests

You can find more information about the builds here

ℹ️ Please run only needed test builds instead of all when developing. Please run all test builds before sending your PR for review.

For more details, please, review the Magento Contributor Guide documentation.

gsomoza · 2020-05-27T13:27:24Z

Note1: this probably shouldn't have the Area: Frontend label.
Note2: assuming you're interested in this PR let me know if you'd like a backport for 2.3.x (although as you can probably see it's quite trivial).

ihor-sviziev · 2020-06-19T07:17:05Z

@magento run all tests

magento-engcom-team · 2020-06-22T06:17:46Z

Hi @ihor-sviziev, thank you for the review.
ENGCOM-7718 has been created to process this Pull Request

magento-engcom-team · 2020-06-22T06:17:48Z

@gsomoza thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository.

gsomoza · 2020-06-22T07:28:51Z

Done!

app/code/Magento/Backend/view/adminhtml/layout/default.xml

engcom-Alfa · 2020-07-16T08:29:16Z

@magento create issue

engcom-Alfa · 2020-07-16T08:38:11Z

✔️ QA Passed

Manual testing scenario:

Create a module that adds the following layout update in the adminhtml area:

<?xml version="1.0"?>
<page xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
      xsi:noNamespaceSchemaLocation="urn:magento:framework:View/Layout/etc/page_configuration.xsd">
    <referenceBlock name="head.additional">
        <block class="Magento\Framework\View\Element\Template" name="test"
               template="Magento_Backend::page/copyright.phtml"/>
    </referenceBlock>
</page>

Run: php bin/magento cache:clean;
Go to Admin and view page source;

Before: ✖️ no copyright text in the <head>

After: ✔️ The copyright text rendered within any admin page's area

ihor-sviziev · 2020-09-14T06:22:21Z

@sidolov could you explain why this PR was moved to "pending review" without any comment? Were the additional tests executed? What their result?Can I approve it?

sidolov · 2020-09-14T15:55:05Z

@ihor-sviziev I don't see any reason to hold it, I agree with @naydav, additional cases should be verified but we can do that on the QA stage.

magento-engcom-team · 2020-09-14T15:57:36Z

Hi @ihor-sviziev, thank you for the review.
ENGCOM-7718 has been created to process this Pull Request

engcom-Alfa · 2020-10-05T09:43:13Z

Dev experience is required for testing this PR. Please note that Manual testing has not been performed.

engcom-Foxtrot · 2020-10-05T13:42:39Z

@magento run all tests

magento-engcom-team · 2020-10-06T13:29:32Z

Hi @ihor-sviziev, thank you for the review.
ENGCOM-7718 has been created to process this Pull Request

m2-assistant · 2020-10-07T20:18:34Z

Hi @gsomoza, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

Add a head.additional block to adminhtml layout

336b463

magento-engcom-team added Area: Frontend Component: Backend Release Line: 2.4 labels May 27, 2020

ghost added the Progress: pending review label May 27, 2020

ihor-sviziev self-assigned this Jun 19, 2020

ihor-sviziev added the Severity: S4 Affects aesthetics, professional look and feel, “quality” or “usability”. label Jun 19, 2020

ghost added Progress: review and removed Progress: pending review labels Jun 19, 2020

ihor-sviziev added Auto-Tests: Not Required Changes in Pull Request does not require coverage by auto-tests improvement labels Jun 22, 2020

ihor-sviziev approved these changes Jun 22, 2020

View reviewed changes

ghost added Progress: ready for testing and removed Progress: review labels Jun 22, 2020

engcom-Alfa self-assigned this Jul 9, 2020

ghost added Progress: testing in progress and removed Progress: ready for testing labels Jul 15, 2020

Nazar65 suggested changes Jul 15, 2020

View reviewed changes

app/code/Magento/Backend/view/adminhtml/layout/default.xml Outdated Show resolved Hide resolved

m2-assistant bot mentioned this pull request Jul 16, 2020

[Issue] Add a head.additional block to adminhtml layout #29165

Closed

4 tasks

ghost added Progress: extended testing and removed Progress: testing in progress labels Jul 16, 2020

engcom-Echo self-assigned this Jul 16, 2020

ihor-sviziev approved these changes Sep 14, 2020

View reviewed changes

ghost added Progress: ready for testing and removed Progress: pending review labels Sep 14, 2020

engcom-Echo removed their assignment Oct 2, 2020

engcom-Alfa added the Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it label Oct 2, 2020

engcom-Alfa removed their assignment Oct 5, 2020

ghost added Progress: extended testing and removed Progress: ready for testing labels Oct 5, 2020

engcom-Foxtrot self-assigned this Oct 5, 2020

engcom-Foxtrot and others added 2 commits October 5, 2020 16:40

magento#29165: Add a head.additional block to adminhtml layout.

75cc0b4

Merge branch '2.4-develop' into add-head-additional-to-adminhtml-layout

01bc9cf

engcom-Foxtrot requested a review from ihor-sviziev October 6, 2020 08:07

ghost added Progress: pending review and removed Progress: extended testing labels Oct 6, 2020

ihor-sviziev approved these changes Oct 6, 2020

View reviewed changes

ghost added Progress: ready for testing Progress: extended testing and removed Progress: pending review Progress: ready for testing labels Oct 6, 2020

ghost added Progress: accept and removed Progress: extended testing labels Oct 7, 2020

magento-engcom-team pushed a commit that referenced this pull request Oct 7, 2020

ENGCOM-7718: Add a head.additional block to adminhtml layout #28389

5d7686c

magento-engcom-team merged commit 0333675 into magento:2.4-develop Oct 7, 2020

Add a head.additional block to adminhtml layout #28389

Add a head.additional block to adminhtml layout #28389

Uh oh!

Conversation

gsomoza commented May 27, 2020 • edited by m2-assistant bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description (*)

Related Pull Requests

Fixed Issues (if relevant)

Manual testing scenarios (*)

Other / Reasoning

Why a ListText block?

Backwards Compatibility

Automated tests?

Contribution checklist (*)

Resolved issues:

Uh oh!

m2-assistant bot commented May 27, 2020

Uh oh!

gsomoza commented May 27, 2020

Uh oh!

ihor-sviziev commented Jun 19, 2020

Uh oh!

magento-engcom-team commented Jun 22, 2020

Uh oh!

magento-engcom-team commented Jun 22, 2020

Uh oh!

gsomoza commented Jun 22, 2020

Uh oh!

Uh oh!

engcom-Alfa commented Jul 16, 2020

Uh oh!

engcom-Alfa commented Jul 16, 2020

Uh oh!

ihor-sviziev commented Sep 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sidolov commented Sep 14, 2020

Uh oh!

magento-engcom-team commented Sep 14, 2020

Uh oh!

engcom-Alfa commented Oct 5, 2020

Uh oh!

engcom-Foxtrot commented Oct 5, 2020

Uh oh!

magento-engcom-team commented Oct 6, 2020

Uh oh!

m2-assistant bot commented Oct 7, 2020

Uh oh!

Uh oh!

gsomoza commented May 27, 2020 •

edited by m2-assistant bot

Loading

Why a `ListText` block?

ihor-sviziev commented Sep 14, 2020 •

edited

Loading