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.

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

Jump to bottom

Media gallery loaded twice #23621

Merged
merged 1 commit into from
Aug 27, 2019
Merged

Media gallery loaded twice #23621

merged 1 commit into from
Aug 27, 2019

Conversation

sidolov
Copy link
Contributor

@sidolov sidolov commented Jul 8, 2019 •
edited by sivaschenko
Loading

Description (*)

  1. Media gallery browser will send 2 requests for content after button click.
  2. Browser loading content every time when you open the tab.

Fixed Issues (if relevant)

magento/adobe-stock-integration#84

Manual testing scenarios (*)

  1. Log in to the admin panel
  2. Navigate to Content -> Pages
  3. Push New page button
  4. Expand content tab
  5. Push Insert\Edit image button, located on the editor
  6. Push Browse button

Expected result:
Content loaded. (in the browser dev tools we can see one request)

Actual result:
Content loaded. (in the browser dev tools we can see two requests)

  1. Close the tab.
  2. Push Browse button one more time

Expected result:
Content loaded without any request because it was loaded before.

Actual result:
Content loaded. (in the browser dev tools we can see two requests)

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
Copy link

m2-assistant bot commented Jul 8, 2019

Hi @sidolov. 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 Assistant documentation

@sidolov sidolov mentioned this pull request Jul 8, 2019
Closed
@ghost ghost assigned orlangur Jul 8, 2019
@magento-engcom-team
Copy link
Contributor

Hi @orlangur, thank you for the review.
ENGCOM-5418 has been created to process this Pull Request
✳️ @orlangur, could you please add one of the following labels to the Pull Request?

Label Description
Auto-Tests: Covered All changes in Pull Request is covered by auto-tests
Auto-Tests: Not Covered Changes in Pull Request requires coverage by auto-tests
Auto-Tests: Not Required Changes in Pull Request does not require coverage by auto-tests

@sidolov sidolov mentioned this pull request Jul 8, 2019
Merged
@engcom-Alfa engcom-Alfa self-assigned this Jul 9, 2019
@engcom-Alfa
Copy link
Contributor

Hi @sidolov !

During testing, i faced an issue.

Problem: after re-opening "Insert/Edit image" tab the page darkens and is blocked(GIF attached).

Manual testing scenario:

  1. Log in to the admin panel
  2. Navigate to Content -> Pages
  3. Push "New page" button
  4. Expand content tab
  5. Push "Insert\Edit image" button, located on the editor
  6. Push "Browse" button
  7. Close the tab.
  8. Push Browse button one more time
  9. Close the tab.
  10. Close "Insert/Edit image" tab
  11. Push "Insert\Edit image" button again
  12. Push "Browse" button
  13. Close the tab.

Actual Result:
"Insert/Edit image" tab hidden.
The page darkens and is blocked.

GIF:
23621

Could you take a look?

@sidolov
Copy link
Contributor Author

sidolov commented Jul 9, 2019

@magento give me 2.3-develop instance

@magento-engcom-team
Copy link
Contributor

Hi @sidolov. Thank you for your request. I'm working on Magento 2.3-develop instance for you

@magento-engcom-team
Copy link
Contributor

Hi @sidolov, here is your Magento instance.
Admin access: https://i-23621-2-3-develop.instances.magento-community.engineering/admin
Login: admin Password: 123123q
Instance will be terminated in up to 3 hours.

@sidolov
Copy link
Contributor Author

sidolov commented Jul 9, 2019

@engcom-Alfa , I found the cause of the issue, it should be fixed now

orlangur
orlangur requested changes Jul 9, 2019
View reviewed changes
Copy link
Contributor

@orlangur orlangur left a comment

Choose a reason for hiding this comment

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

LGTM but please squash changes

@ghost ghost assigned sivaschenko Jul 10, 2019
@magento-engcom-team
Copy link
Contributor

Hi @sivaschenko, thank you for the review.
ENGCOM-5418 has been created to process this Pull Request
✳️ @sivaschenko, could you please add one of the following labels to the Pull Request?

Label Description
Auto-Tests: Covered All changes in Pull Request is covered by auto-tests
Auto-Tests: Not Covered Changes in Pull Request requires coverage by auto-tests
Auto-Tests: Not Required Changes in Pull Request does not require coverage by auto-tests

@sivaschenko sivaschenko added the Auto-Tests: Not Covered Changes in Pull Request requires coverage by auto-tests label Jul 10, 2019
orlangur
orlangur requested changes Jul 10, 2019
View reviewed changes
Copy link
Contributor

@orlangur orlangur left a comment

Choose a reason for hiding this comment

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

@sivaschenko @sidolov can we get this squashed without retesting please?

@sidolov sidolov force-pushed the media_gallery_loaded_twice branch from 51cdbec to 76619e1 Compare July 10, 2019 14:37
sivaschenko
sivaschenko requested changes Jul 14, 2019
View reviewed changes
Copy link
Member

@sivaschenko sivaschenko left a comment

Choose a reason for hiding this comment

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

Magento\FunctionalTestingFramework.functional\PageBuilderWithWYSIWYG.SlideItemDeleteImage is failing on this PR

@engcom-Foxtrot engcom-Foxtrot self-assigned this Jul 15, 2019
@sivaschenko sivaschenko mentioned this pull request Jul 16, 2019
Closed
@engcom-Foxtrot engcom-Foxtrot mentioned this pull request Jul 17, 2019
Merged
davemacaulay added a commit to magento/magento2-page-builder that referenced this pull request Jul 22, 2019
@davemacaulay
Merge pull request #266 from engcom-Foxtrot/FIX-23621
d3a2d42 
Fix for magento/magento2#23621.
@sivaschenko
Copy link
Member

@magento run all tests

@konarshankar07 konarshankar07 mentioned this pull request Aug 4, 2019
Closed
@engcom-Golf engcom-Golf mentioned this pull request Aug 16, 2019
Closed
magento-engcom-team pushed a commit that referenced this pull request Aug 27, 2019
@sidolov
ENGCOM-5418: Media gallery loaded twice #23621
a8e62b0
@magento-engcom-team magento-engcom-team merged commit 76619e1 into magento:2.3-develop Aug 27, 2019
@m2-assistant
Copy link

m2-assistant bot commented Aug 27, 2019

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

@magento-engcom-team magento-engcom-team added this to the Release: 2.3.4 milestone Aug 27, 2019
@sivaschenko sivaschenko mentioned this pull request Aug 27, 2019
Merged
@engcom-Golf engcom-Golf mentioned this pull request Apr 10, 2020
Closed
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@orlangur orlangur orlangur requested changes

@sivaschenko sivaschenko sivaschenko requested changes

Assignees

@orlangur orlangur

@sivaschenko sivaschenko

@engcom-Alfa engcom-Alfa

@engcom-Foxtrot engcom-Foxtrot

Labels
Area: Lib/Frontend Auto-Tests: Not Covered Changes in Pull Request requires coverage by auto-tests Progress: accept Release Line: 2.3
Projects
None yet
Milestone
Release: 2.3.4
Development

Successfully merging this pull request may close these issues.
6 participants
@sidolov @magento-engcom-team @engcom-Alfa @sivaschenko @orlangur @engcom-Foxtrot