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

Make sure the gallery doesn't have a navis-before child on open #1743

Merged

Conversation

joshdarby
Copy link
Collaborator

@joshdarby joshdarby commented Jul 11, 2019

If it does have that child, the bad settings won't be applied.

Changes

This pull request makes the following changes:

  • Added another conditional on the navis image click trigger to first check if the gallery has a .navis-before child before firing functions to modify styles and add the X button.

Why

For #1700

Testing/Questions

Features that this PR affects:

  • Navis slideshow images

Questions that need to be answered before merging:

  • Does it work as expected now?

Steps to test this PR:

Given a single image in an image block that is aligned left or right, take the following steps to reproduce this bug:

  1. Click/tap on the image to open the image in a Navis modal.
    1. the modal opens
    2. The image width is set to 100%: https://github.com/INN/largo/blob/e2ac830239452658c285860b330d521c6a0f300e/lib/navis-slideshows/js/navis-slideshows.js#L271
  2. Click/tap on the image in the Navis modal.
    1. Nothing happens that the user can see, but the function addCloseX is called again, adding a duplicate X in the modal (which is invisible to the user because it's stacked atop the current X, using the same CSS rules for positioning)
    2. The 100% image width is saved as the pre-click image attribute https://github.com/INN/largo/blob/e2ac830239452658c285860b330d521c6a0f300e/lib/navis-slideshows/js/navis-slideshows.js#L263-L268 and is now relayed to the event handler that calls closeSingle https://github.com/INN/largo/blob/e2ac830239452658c285860b330d521c6a0f300e/lib/navis-slideshows/js/navis-slideshows.js#L274-L276
  3. Click on the X to exit the modal.
    1. The 100% from the second click is used as the pre-click image width by closeSingle when the image is closed: https://github.com/INN/largo/blob/e2ac830239452658c285860b330d521c6a0f300e/lib/navis-slideshows/js/navis-slideshows.js#L177
  4. The image should still be its normal size. Previously it would be 100% of the column width rather than its previous appearance.

If it does have that child, the bad settings won't be applied.
@joshdarby joshdarby requested a review from benlk July 11, 2019 21:10
@joshdarby joshdarby self-assigned this Jul 11, 2019
@joshdarby joshdarby added this to the 0.6.4 milestone Jul 11, 2019
@joshdarby joshdarby added category: images Issues relating to images priority: normal Must be completed before release of this version of plugin. type: bug labels Jul 11, 2019
@benlk
Copy link
Collaborator

benlk commented Jul 12, 2019

This works.

@joshdarby joshdarby merged commit 39a0fc4 into 0.5-dev Jul 12, 2019
@benlk benlk deleted the 1700-clicking-on-image-in-slideshow-breaks-image-in-content branch July 19, 2019 22:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: images Issues relating to images Estimate: < 6 Hours priority: normal Must be completed before release of this version of plugin. type: bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants