Added 'clearEvent' to eventBus to prevent multiple attach event on wysiwyg editor #25671

Nazar65 · 2019-11-20T15:20:55Z

Description (*)

Fixed Issues (if relevant)

Search Adobe Stock button is not active on Edit Product Page in Admin adobe-stock-integration#689

Manual testing scenarios (*)

set console.log('openfileBrowser') on 376 line in lib/web/mage/adminhtml/wysiwyg/tiny_mce/tinymce4Adapter.js
1.1 open browser dev tools

Open Catalog -> product -> Add new product
Content Tab -> on wisiwyg editor press image icon -> source click folder button

Expected result openfileBrowser appears one time on console dev tools when you open file bwroser

[Case2]
AbodeStock
Open Adobe Stock on product page in content tab
Expected result Adobe Stock button opens grid

Questions or comments

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 · 2019-11-20T15:20:58Z

Hi @Nazar65. 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.3-develop instance - deploy vanilla Magento instance

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

krzksz · 2019-11-21T17:45:26Z

lib/web/mage/adminhtml/wysiwyg/tiny_mce/tinymce4Adapter.js

@@ -373,7 +373,7 @@ define([
        /**
         * @param {Object} o
         */
-        openFileBrowser: function (o) {
+        openFileBrowser: _.debounce(function (o) {


Hey, did you find out what is the reason that openFileBrowser is called multiple times in the first place?

Hi @krzksz yes, this caused by this lines

magento2/lib/web/mage/adminhtml/wysiwyg/tiny_mce/tinymce4Adapter.js

Line 295 in d56adbb

varienGlobalEvents.fireEvent('open_browser_callback', payload);

magento2/lib/web/mage/adminhtml/wysiwyg/tiny_mce/tinymce4Adapter.js

Line 296 in d56adbb

this.eventBus.fireEvent('open_browser_callback', payload);

Where the same event fires two times with the same object, but as I observed this event needs both, but on some pages, these events triggered different result, like on product edit page this will fire twice, and on category page once as expected

krzksz · 2019-11-22T22:18:22Z

app/code/Magento/Tinymce3/view/base/web/tinymce3Adapter.js

@@ -78,6 +78,8 @@ define([
            }

            tinyMCE3.init(this.getSettings(mode));
+            varienGlobalEvents.clearEventHandlers("open_browser_callback");
+            varienGlobalEvents.attachEventHandler("open_browser_callback", tinyMceEditors.get(this.id).openFileBrowser);


Please take a look at static tests as they expect single apostrophes to be used.

krzksz · 2019-11-22T22:22:12Z

lib/internal/Magento/Framework/Data/Form/Element/Editor.php

@@ -310,7 +310,9 @@ protected function _getPluginButtonsHtml($visible = true)
                        if (isset($buttonOptions['style'])) {
                            $configStyle = $buttonOptions['style'];
                        }
+                        // phpcs:disable Magento2.Performance.ForeachArrayMerge
                        $buttonOptions = array_merge($buttonOptions, ['style' => 'display:none;' . $configStyle]);


This could be reworked into $buttonOptions['style'] = 'display:none;' . $configStyle; so we don't need to disable any checks, right?

…o ASI-689

magento-engcom-team · 2019-11-25T13:51:44Z

Hi @sivaschenko, thank you for the review.
ENGCOM-6329 has been created to process this Pull Request

engcom-Delta · 2019-11-26T13:15:26Z

✔️ QA passed

…h event on wysiwyg editor #25671

m2-assistant · 2019-11-27T16:46:50Z

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

Remove duplicated fireEcent, that prevents to duble initialization

dc1ab99

Nazar65 requested a review from DrewML as a code owner November 20, 2019 15:20

magento-engcom-team added Area: Lib/Frontend Release Line: 2.3 labels Nov 20, 2019

Nazar65 changed the title ~~Remove duplicated fireEcent, that prevents to duble initialization~~ Remove duplicate fireEvent, to prevent double initialization Nov 20, 2019

Revert event add checks that method not fires two times at the time

d56adbb

Nazar65 changed the title ~~Remove duplicate fireEvent, to prevent double initialization~~ Added debounce to 'open FIle Browser', to prevent double invocation at the same time Nov 21, 2019

Nazar65 mentioned this pull request Nov 21, 2019

Search Adobe Stock button is not active on Edit Product Page in Admin magento/adobe-stock-integration#748

Merged

krzksz suggested changes Nov 21, 2019

View reviewed changes

ghost assigned krzksz Nov 21, 2019

ghost added the Progress: needs update label Nov 21, 2019

Nazar65 requested a review from YevSent as a code owner November 22, 2019 12:39

Nazar65 added 2 commits November 22, 2019 14:41

remove duplicated events on wisiwyg initialization

a880a83

Static test fix

07fc246

Nazar65 changed the title ~~Added debounce to 'open FIle Browser', to prevent double invocation at the same time~~ Added 'clearEvent' to eventBus to prevent multiple attach event on wysiwyg editor Nov 22, 2019

Nazar65 added 2 commits November 22, 2019 15:46

Remove duplicated event

a6ba585

Fix for tinymce3 editor

1be4e03

krzksz suggested changes Nov 22, 2019

View reviewed changes

Merge branch '2.3-develop' of https://github.com/magento/magento2 int…

2c2748d

…o ASI-689

Nazar65 added the Auto-Tests: Covered All changes in Pull Request is covered by auto-tests label Nov 23, 2019

Cover with js unit test, static test fix

7b16d76

Nazar65 requested a review from krzksz November 25, 2019 10:07

sivaschenko requested review from omiroshnichenko and VladimirZaets November 25, 2019 13:41

sivaschenko approved these changes Nov 25, 2019

View reviewed changes

ghost assigned sivaschenko Nov 25, 2019

ghost added Progress: ready for testing and removed Progress: needs update labels Nov 25, 2019

magento-engcom-team added the Component: Data label Nov 25, 2019

magento-engcom-team added the Component: Tinymce label Nov 25, 2019

krzksz approved these changes Nov 25, 2019

View reviewed changes

engcom-Delta self-assigned this Nov 25, 2019

ghost added Progress: testing in progress and removed Progress: ready for testing labels Nov 25, 2019

ghost added Progress: extended testing and removed Progress: testing in progress labels Nov 26, 2019

engcom-Foxtrot self-assigned this Nov 26, 2019

ghost added Progress: accept and removed Progress: extended testing labels Nov 26, 2019

magento-engcom-team pushed a commit that referenced this pull request Nov 27, 2019

ENGCOM-6329: Added 'clearEvent' to eventBus to prevent multiple attac…

2d36ee0

…h event on wysiwyg editor #25671

magento-engcom-team merged commit 7b16d76 into magento:2.3-develop Nov 27, 2019

magento-engcom-team added this to the Release: 2.3.5 milestone Nov 27, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added 'clearEvent' to eventBus to prevent multiple attach event on wysiwyg editor #25671

Added 'clearEvent' to eventBus to prevent multiple attach event on wysiwyg editor #25671

Nazar65 commented Nov 20, 2019 •

edited

Loading

m2-assistant bot commented Nov 20, 2019

krzksz Nov 21, 2019

Nazar65 Nov 21, 2019 •

edited

Loading

krzksz Nov 22, 2019

krzksz Nov 22, 2019

magento-engcom-team commented Nov 25, 2019

engcom-Delta commented Nov 26, 2019

m2-assistant bot commented Nov 27, 2019

Added 'clearEvent' to eventBus to prevent multiple attach event on wysiwyg editor #25671

Added 'clearEvent' to eventBus to prevent multiple attach event on wysiwyg editor #25671

Conversation

Nazar65 commented Nov 20, 2019 • edited Loading

Description (*)

Fixed Issues (if relevant)

Manual testing scenarios (*)

Questions or comments

Contribution checklist (*)

m2-assistant bot commented Nov 20, 2019

krzksz Nov 21, 2019

Choose a reason for hiding this comment

Nazar65 Nov 21, 2019 • edited Loading

Choose a reason for hiding this comment

krzksz Nov 22, 2019

Choose a reason for hiding this comment

krzksz Nov 22, 2019

Choose a reason for hiding this comment

magento-engcom-team commented Nov 25, 2019

engcom-Delta commented Nov 26, 2019

m2-assistant bot commented Nov 27, 2019

Nazar65 commented Nov 20, 2019 •

edited

Loading

Nazar65 Nov 21, 2019 •

edited

Loading