Make sure the depends definition works for custom widgets. Also conve…

[hostep]

…rted code from PrototypeJS to jQuery.

Description (*)

This fixes #6868 and basically implements the solution proposed on stackexchange.

The solution from stackexchange is a bit strange though, since they use jQuery while the original code used Prototype and this caused them to have to swap the following if around, since the headElement could either be a jQuery or Prototype element and it would crash if it was a jQuery object:

 if (headElement.hasClassName('open') && target) {

vs

 if (target && headElement.hasClassName('open')) {

So instead of taking over the proposed code, I've converted all code touching headElement to now use jQuery so we no longer have this strange hybrid solution.

Related Pull Requests

None

Fixed Issues (if relevant)

1. Fixes Widget parameter depends does not work on specified block #6868: Widget parameter depends does not work on specified block
2. Fixes Widget node depends with block parameter #7252: Widget node depends with block parameter
3. Fixes Widget field depends with type block KO #13316: Widget field depends with type block KO
4. MAGETWO-82054

Manual testing scenarios (*)

1. See Widget parameter depends does not work on specified block #6868 (didn't really test it with that, but with a custom widget from our own collection)

Questions or comments

Before somebody asks, I'm not interested in writing tests for this, so if this is required, feel free to pick this up yourselves 🙂

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)

[m2-assistant]

Hi @hostep. 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:

1. Database Compare
2. Functional Tests CE
3. Functional Tests EE,
4. Functional Tests B2B
5. Integration Tests
6. Magento Health Index
7. Sample Data Tests CE
8. Sample Data Tests EE
9. Sample Data Tests B2B
10. Static Tests
11. Unit Tests
12. 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.

⚠️ According to the Magento Contribution requirements, all Pull Requests must go through the Community Contributions Triage process. Community Contributions Triage is a public meeting.

🕙 You can find the schedule on the Magento Community Calendar page.

📞 The triage of Pull Requests happens in the queue order. If you want to speed up the delivery of your contribution, please join the Community Contributions Triage session to discuss the appropriate ticket.

🎥 You can find the recording of the previous Community Contributions Triage on the Magento Youtube Channel

✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel

[hostep]

@magento run all tests

[ihor-sviziev]

Seems like this could be covered only with the js unit test, but that would be too hard. Adding label "auto tests not required".

Let's wait for test results.

[ihor-sviziev]

@magento run Functional Tests B2B, Functional Tests CE

[ghost]

This pull request cannot be added to non-default project automatically. Please add it manually if needed.

[ghost]

This pull request cannot be added to non-default project automatically. Please add it manually if needed.

[ghost]

This pull request cannot be added to non-default project automatically. Please add it manually if needed.

[ghost]

This pull request cannot be added to non-default project automatically. Please add it manually if needed.

[magento-engcom-team]

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

[ghost]

This pull request cannot be added to non-default project automatically. Please add it manually if needed.

[ghost]

This pull request cannot be added to non-default project automatically. Please add it manually if needed.

[ghost]

This pull request cannot be added to non-default project automatically. Please add it manually if needed.

[ihor-sviziev]

@sidolov @gabrieldagama what does this message from m2-backlog means?

[engcom-Alfa]

✔️ QA Passed

Manual testing scenario:

1. Create a custom widget with widget.xml file by following devdocs
2. Create a select option chooser

<parameter name="test_select" xsi:type="select" visible="true" required="true" sort_order="10">
    <label translate="true">Select</label>
    <options>
        <option name="val_1" value="val_1" selected="true">
            <label translate="true">Value 1</label>
        </option>
        <option name="val_2" value="val_2">
            <label translate="true">Value 2</label>
        </option>
    </options>
</parameter>

3. Create a CMS block Chooser that needs the second option selected

<parameter name="block_2_id" xsi:type="block" visible="true" required="true" sort_order="30">
    <label translate="true">Block 2</label>
    <depends>
        <parameter name="test_select" value="val_2" />
    </depends>
    <block class="Magento\Cms\Block\Adminhtml\Block\Widget\Chooser">
        <data>
            <item name="button" xsi:type="array">
                <item name="open" xsi:type="string" translate="true">Select Block...</item>
            </item>
        </data>
    </block>
</parameter>

4. Go to Admin -> Content -> Widgets -> Add Widget
5. Choose created before type of widget and click on the Continue button

6. Go to the Widget Options tab

Before:

✖️ When we select the Value 1, CMS Chooser is visible
✖️ When we select the Value 2, CMS Chooser is visible

After:

✔️ When I select the Value 1, CMS Chooser is not visible
✔️ When I select the Value 2, CMS Chooser is visible

[sidolov]

Hi @hostep , @ihor-sviziev there were some changes in the backlog app a few hours ago that caused such issue, it's resolved for now. Thank you for pointing out!

[m2-assistant]

Hi @hostep, 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.

[DmitryFurs]

Hi @hostep
When we use type="block" other than chooser, the field is hidden incorrectly, only the input itself is hidden, and the header remains displayed

<parameter name="reviews_to_display" xsi:type="block" visible="true" required="true" sort_order="20">
    <label translate="true">Reviews to Display</label>
    <depends>
        <parameter name="type" value="category" />
    </depends>
    <block class="Vendor\Module\Block\Adminhtml\Widget\Parameter\Number" />
</parameter>

<?php

declare(strict_types=1);

namespace Vendor\Module\Block\Adminhtml\Widget\Parameter;

use Magento\Backend\Block\Template;
use Magento\Backend\Block\Template\Context;
use Magento\Framework\Data\Form\Element\AbstractElement;
use Magento\Framework\Data\Form\Element\Factory as ElementFactory;

class Number extends Template
{
    /**
     * @var ElementFactory
     */
    private $elementFactory;

    /**
     * @param Context $context
     * @param ElementFactory $elementFactory
     * @param array $data
     */
    public function __construct(
        Context $context,
        ElementFactory $elementFactory,
        array $data = []
    ) {
        $this->elementFactory = $elementFactory;
        parent::__construct($context, $data);
    }

    /**
     * @param AbstractElement $element
     *
     * @return AbstractElement
     */
    public function prepareElementHtml(AbstractElement $element)
    {
        $input = $this->elementFactory->create('text', ['data' => $element->getData()]);
        $input->setId($element->getId());
        $input->setForm($element->getForm());
        $input->setData('class', 'widget-option input-text admin__control-text');
        $input->addClass('validate-number')->addClass('validate-greater-than-zero');
        if ($element->getData('required')) {
            $input->addClass('required-entry');
        }

        $element->setData('after_element_html', $input->getElementHtml());
        $element->setData('value', null);

        return $element;
    }
}

[hostep]

@DmitryFurs: thanks for that!

I was a bit afraid this ticket got accepted too quickly and it could potentially have some regressions.
@engcom-Alfa: can we re-test this showing/hiding thing with other blocks than the chooser thingy?

Sorry, I have very little experience with this backend form stuff, I only looked at the issue mentioned on #6868 and hoped QA would check the other hiding/showing stuff.

[DmitryFurs]

@hostep and one more thing, when we use category chooser with required="true", the check is triggered even if the dependent field is hidden

<parameter name="category_id" xsi:type="block" visible="true" required="true" sort_order="50">
    <label translate="true">Category</label>
    <depends>
        <parameter name="type" value="category" />
    </depends>
    <block class="Magento\Catalog\Block\Adminhtml\Category\Widget\Chooser">
        <data>
            <item name="button" xsi:type="array">
                <item name="open" xsi:type="string" translate="true">Select Category...</item>
            </item>
        </data>
    </block>
</parameter>

[ihor-sviziev]

@DmitryFurs As I understood - we need to revert changes from this PR? and later on prepare new PR that will cover all these cases? If so - could you please create PR for reverting?

[DmitryFurs]

@ihor-sviziev I think this fix won't exactly make things worse, just does not cover all possible cases. If this fix is reverted, what is the probability that this problem will ever be fixed?

[ihor-sviziev]

Ah, ok, so in this case - could you report separate issue with additional cases? Thank you!

[DmitryFurs]

@ihor-sviziev are these cases likely to be fixed in the next release?

[ihor-sviziev]

@DmitryFurs it doesn't seems like quite critical issue, so probably if someone from community will take it (maybe @hostep ?) - it will be fixed.

[hostep]

Ah, good to hear this isn't a regression, phew! 🙂

As long as we don't run into these issues on projects of our own, I'm probably not going to spend more time on this, sorry! I hope someone else will be interested to look at this.