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.

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

remove changes of MAGETWO-69567 #13268

Closed
wants to merge 1 commit into from
Closed

remove changes of MAGETWO-69567 #13268

wants to merge 1 commit into from

Conversation

torhoehn
Copy link
Contributor

@torhoehn torhoehn commented Jan 18, 2018

Undo changes of MAGETWO-69567

Description

Previous changes in the fotorama gallery plugin causes problems on iOS devices.

Fixed Issues (if relevant)

  1. Can't close full screen gallery on iOS devices #13226: Can't close full screen gallery on iOS devices

Manual testing scenarios

  1. Start Apple Simulator (https://developer.apple.com/library/content/documentation/IDEs/Conceptual/iOS_Simulator_Guide/Introduction/Introduction.html)
  2. Run Safari and open any product detail page
  3. Tap on the image to open the product image in fullscreen mode
  4. Tap on the close button in the upper right corner
  5. Gallery closes and you can see the product detail page again

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 on Travis CI are green)

Copy link
Contributor

@iivashchenko iivashchenko left a comment

Choose a reason for hiding this comment

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

Removing this changes will provoke and additional click triggered and, as a result, multi-cart or search-box will be exposed. There is a resolved MAGETWO-85708 ticket where a special case for IOS devices was added.

@torhoehn
Copy link
Contributor Author

Maybe it would be a good idea to make your internal tickets or the associated pull request public.

@iivashchenko
Copy link
Contributor

@torhoehn Sorry, I can't make an influence on that, but I can inform you when mentioned ticket will be added to the public pull request or finally merged.

@miguelbalparda
Copy link
Contributor

miguelbalparda commented Jan 24, 2018

There are some commits with that internal ticket number. Can you review them and confirm we can close this PR?

@iivashchenko
Copy link
Contributor

@miguelbalparda yes, this PR can be closed.

@torhoehn torhoehn closed this Jan 29, 2018
@torhoehn torhoehn deleted the ios-gallery-fix branch January 29, 2018 16:26
@anastaciyagavrilova
Copy link

anastaciyagavrilova commented Mar 9, 2018

Hi,

The fix is still not working on Magento ver. 2.2.3 in /html/lib/web/fotorama/fotorama.js file

if (o_allowFullScreen) {
$fullscreenIcon.prependTo($stage);
o_nativeFullScreen = FULLSCREEN && o_allowFullScreen === 'native';

            // Due 300ms click delay on mobile devices
            // we stub touchend and fallback to click.
            // MAGETWO-69567

            stubEvent($fullscreenIcon, 'touchend');
        } else {
            $fullscreenIcon.detach();
            o_nativeFullScreen = false;
        }

IN iphones Cross Button is not working and gallery is not closing.
Any solution:--

@iivashchenko
Copy link
Contributor

iivashchenko commented Mar 9, 2018

@anastaciyagavrilova Thank you for the report. It seems the fix is not ported to 2.2.3. As a solution, you can create and apply patch from StubEvent function of Magento 2.2.2. Internal ticket MAGETWO-88973 to port the fix was created.

@h2ojunkie
Copy link

The link posted for StubEvent function of Magento 2.2.2. is 404'ing. Is there another place to get this fix?

@iivashchenko
Copy link
Contributor

@h2ojunkie sorry for the broken link. Right address is fotorama.js

@PatelHemali
Copy link

@iivashchenko ...............Thanks a lot.........

@Brengineer
Copy link

This issue applies to all touch devices, not just iOS. For example, Chrome on Android has the same issue. (The Fotorama gallery does not close when tapping the button.)

@Brengineer
Copy link

Brengineer commented May 4, 2018

@iivashchenko - I have not been able to reproduce the issue that MAGETWO-69567 claims to fix, and I can't find the original issue, so I can't see what the reproducible steps are.
What I do know is that the changes it included are causing the issue described here, and it is not limited to iOS, so commit d50c512 (for MAGETWO-85708) is not sufficient.
I have a patch that I have used on multiple projects that directly reverts commit dd9d57f, and I have not encountered any problems, so as far as I can tell, the best solution is to do exactly as the title of this PR says and remove changes of MAGETWO-69567.
If you think it's still an issue, please inform us of the reproducible steps for MAGETWO-69567 on the current version of Magento, so we can find a solution that will successfully close the image gallery on all touch devices without ever activating the minicart or search box.

@iivashchenko
Copy link
Contributor

@Brengineer Hi, and thank you for your feedback. We didn't get gallery issues connected with mobile devices, excluding IOS, yet. If you have such issue - please create the corresponding ticket and provide accurate information about your device and Magento version. Also, we have no reported issues with gallery closing since dd9d57f. Just FYI, the original steps to reproduce MAGETWO-69567 are:

  1. Open developer tools > emulate a mobile phone device
  2. Go to site front end > any product
  3. Click on the main image to enlarge
  4. Notice image window opens
  5. Click the X button to close the window
  6. Notice depending on where the button is clicked either the search bar or the mini cart drop down opens.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants