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

Navis Slideshow JS is broken from recent changes in Gutenberg; styles are misapplied #1664

Closed
benlk opened this issue Apr 15, 2019 · 2 comments
Assignees
Labels
category: gutenberg Relating to general Gutenberg compatibility category: styles affects lots of styles, requiring visual testing status: in progress type: bug
Milestone

Comments

@benlk
Copy link
Collaborator

benlk commented Apr 15, 2019

Using https://secure.helpscout.net/conversation/802967329/3453/?folderId=1211654 as an example, if:

  • an image is positioned in a story using Gutenberg
  • and aligned right or left,
  • and imag elinking is set to "none" or "media file",

then if that image is clicked, it pops up into a strange position, and can't be dismissed.

Tyler wrote:

So did some debugging on this:

  1. The navis-slideshows code comes with Largo.
  2. That code makes it such that if you click on an image in a post, that image gets blown up in a black-background letterbox type thing.
  3. The way that works is that Navis's JS file inserts certain classes onto the
    that's the parent of the .
  4. PROBLEM: I think this starts with Gutenberg, but now that is contained within a , which in turn is contained by the
    -- that means when the image gets clicked, those extra classes go onto the instead which causes the screwed up view.
  5. POSSIBLE SOLUTION? If that intermediate doesn't have a display:table attached to it, I think everything displays normally?

Tyler's solution of removing the display: table works, but I don't understand why.

@benlk benlk added type: bug category: styles affects lots of styles, requiring visual testing category: gutenberg Relating to general Gutenberg compatibility labels Apr 15, 2019
@benlk benlk added this to the 0.6.2 milestone Apr 15, 2019
@joshdarby joshdarby self-assigned this Apr 22, 2019
joshdarby pushed a commit that referenced this issue Apr 22, 2019
…vent 'display: table; from causing issues when image is viewed at full size, per #1664
@benlk benlk closed this as completed Apr 22, 2019
@benlk benlk reopened this Apr 24, 2019
@benlk benlk modified the milestones: 0.6.2, 0.6.3 Apr 24, 2019
@benlk
Copy link
Collaborator Author

benlk commented Apr 24, 2019

#1675 didn't go far enough.

The .navis-slideshow.navis-full class needs transform: translateX(-50%); to counter the effects of a .alignfull class, when a full image block is output:

<figure class="wp-block-image alignfull" style="max-width: 100%;">
<img src="http://mstoday.test/wp-content/uploads/2018/02/24496652921_1108fff3b7_h-1170x780.jpg" alt="" class="wp-image-129823" srcset="http://mstoday.test/wp-content/uploads/2018/02/24496652921_1108fff3b7_h-1170x780.jpg 1170w, http://mstoday.test/wp-content/uploads/2018/02/24496652921_1108fff3b7_h-336x224.jpg 336w, http://mstoday.test/wp-content/uploads/2018/02/24496652921_1108fff3b7_h-768x512.jpg 768w, http://mstoday.test/wp-content/uploads/2018/02/24496652921_1108fff3b7_h-771x514.jpg 771w, http://mstoday.test/wp-content/uploads/2018/02/24496652921_1108fff3b7_h.jpg 1600w" sizes="(max-width: 1170px) 100vw, 1170px">
<figcaption>This is a full-width image. It should be preceded by a single paragraph tag.</figcaption
</figure>

A similar bug:

Screen Shot 2019-04-24 at 16 45 22

We must also must take care of the caption positioning changes in #1671 which have caused the caption to reappear on the slideshow:

.navis-slideshow.navis-full {
 &.aligncenter,
  &.alignleft,
  &.alignright,
  &.is-resized {
    display: block;
    > figcaption {
      display: none;
    }
  }
}

Also note in that image that the global nav is drawing over top of the image. That can be fixed by removing this z-index, added in 5a56ce5 :

https://github.com/INN/largo/blob/395412d158001de0e7332e714aff8d3c5edccb0b/less/inc/header.less#L5-L10

@benlk
Copy link
Collaborator Author

benlk commented Apr 24, 2019

A fix for this should be tested against both C and MST.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: gutenberg Relating to general Gutenberg compatibility category: styles affects lots of styles, requiring visual testing status: in progress type: bug
Projects
None yet
Development

No branches or pull requests

2 participants