Problem/Motivation

When dynamic caption settings are changed, they have no effect when viewing a page.

Steps to reproduce

  1. Enable the BigPipe module (enabled by default in the standard profile)
  2. On /admin/config/media/photoswipe_dynamic_caption, change "Caption Position" to "Below".
  3. View a Photoswipe page.

Result: the caption still appears to the right of the image, not below it.

Proposed resolution

When a page is initially loaded, Drupal.behaviors.photoswipeCaption deletes the caption options from drupalSettings, so the options are empty during all subsequent executions of attach(). The caption options should be stored so they can be applied every time attach() is called, not just the first time.

CommentFileSizeAuthor
#3 photoswipe-caption-options.patch983 bytesfj23

Issue fork photoswipe-3517156

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

bgreco created an issue. See original summary.

bgreco opened merge request !130

fj23’s picture

fj23 commented
StatusFileSize
new photoswipe-caption-options.patch983 bytes

Patch attached.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Status: Active » Needs work

Thanks for pointing this out. I left a comment on the MR.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Assigned: Unassigned » grevil

@grevil: We're running into this in a client project, maybe you could have a look in the next days? See my comment in the MR.

bgreco’s picture

bgreco commented
Status: Needs work » Needs review

Makes sense, I assumed the options were being deleted for some other reason, but if they're not hurting anything that's even simpler.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Well I think, there might have been a reason for that line, which we need to keep, but we should not alter the original value. I'm not deeply into this though.

grevil made their first commit to this issue’s fork.

grevil opened merge request !131

grevil closed merge request !130

grevil’s picture

grevil commented
Assigned: grevil » Unassigned
Status: Needs review » Reviewed & tested by the community

I honestly have no idea why @segovia94 added this line in the original proof of concept in #3232070: [5.x] PhotoSwipe 5 Branch. Looks like I overlooked that back then.

Thanks for the fix! Works like a charm. :)

  • grevil committed c3053a29 on 5.x 
    Issue #3517156 by bgreco, grevil, fj23, anybody: Dynamic caption...
grevil’s picture

grevil commented
Status: Reviewed & tested by the community » Fixed
grevil’s picture

grevil commented

Release 5.0.4: https://www.drupal.org/project/photoswipe/releases/5.0.4

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.