Problem/Motivation

The Media Gallery module uses a custom Media Photoswipe view mode to display the image field (`field_images`) on gallery entities. This view mode:
- Is not easily customizable by site builders from the UI
- Is not using the existing configurable field formatter provided by [Photoswipe field formatter](https://www.drupal.org/project/photoswipe).

Steps to reproduce

1. Install and enable the Media Gallery module.
2. Create a media gallery entity and add media items with images.
3. Go to the Structure -> Media Gallery -> Manage View Display
4. Observe that the "Rendered Entity" Format and "Media Photoswipe View Mode" is used to render `field_images` but this does not expose any configurability for Photoswipe to the user.
5. Switch the Format to the the [Photoswipe field formatter](https://www.drupal.org/project/photoswipe) module.
6. Save the formatting
7. Go to a gallery and observe that the photoswipe gallery is not rendered correctly.

Proposed resolution

- Update the default configuration for `field_images` to use the `photoswipe_field` formatter plugin instead of the `Rendered Entity` formatter.

- Implement a `hook_post_update_NAME()` function that:
- Scans all `entity_view_display` configs for `media_gallery` entities.
- Replaces the formatter for `field_images` with `photoswipe_field` if it still uses the default `Media Photoswipe` view
mode and Rendered Entity formatter.
- Applies reasonable default settings for the Photoswipe formatter (e.g., image style).
- If present and unused, delete the `Media Photoswipe" view mode and its associated display config to avoid conflicts.

Remaining tasks

- [x] Open this issue.
- [ ] Submit merge request/patch
- [ ] Code Review

User interface changes

No user-facing UI changes. Admins may notice the Photoswipe formatter used by default on the `field_images` field when editing view displays.

API changes

None.

Data model changes

No changes to field storage or entity schema. The update modifies field display configuration and removes a view mode config entity (`core.entity_view_display.media.image.media_photoswipe`, `core.entity_view_display.media.media_photoswipe`)

Issue fork media_gallery-3537414

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

circuitcipher created an issue. See original summary.

circuitcipher opened merge request !9

circuitcipher’s picture

circuitcipher commented
Status: Active » Needs review
circuitcipher’s picture

circuitcipher commented
Status: Needs review » Active
circuitcipher’s picture

circuitcipher commented

I am currently working on fixing PHPCS errors and hope to push them today.

circuitcipher’s picture

circuitcipher commented
Status: Active » Needs review

These changes should be complete at this point. I am just requesting a code review/testing as this is making some sweeping changes and I'm relatively new to the Drupal development space.

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

ivnish’s picture

ivnish
he/him
commented
Status: Needs review » Needs work

I tested this patch manually.

1) The reverse order is broken with pager (reverse works only for current (first?) page)

2) The new pager shows 12 images instead of 9. This can break existing sites.

When you fixed this, I will test upgrade from existing sites

circuitcipher’s picture

circuitcipher commented

Thank you so much for your timely review! I will work on resolving these issues.

ivnish’s picture

ivnish
he/him
commented

This issue also has pager issues. If I understand correctly we need to commit another issue first

circuitcipher’s picture

circuitcipher commented
Status: Needs work » Needs review

1) The reverse order is broken with pager (reverse works only for current (first?) page)
2) The new pager shows 12 images instead of 9. This can break existing sites.

I have fixed the above issues.

This issue also has pager issues. If I understand correctly we need to commit another issue first

Yes, we should merge #3508977: Extra configuration options for the Pager first. After those changes are merged, we can implement the necessary items_per_page logic for the pagination logic in this issue.

ivnish’s picture

ivnish
he/him
commented
Status: Needs review » Needs work

I merged MR about pager improvements. Do you need to update this MR ?

circuitcipher’s picture

circuitcipher commented

I merged MR about pager improvements. Do you need to update this MR ?

Yes, I need to make updates to this MR so that it makes use of the items_per_page parameter. I will submit these changes today.

circuitcipher’s picture

circuitcipher commented
Status: Needs work » Needs review

I merged MR about pager improvements. Do you need to update this MR ?

I have made the changes to support the pager improvements. This is ready for your review.

ivnish’s picture

ivnish
he/him
commented
Status: Needs review » Needs work

I left a few comments in MR

circuitcipher’s picture

circuitcipher commented

Ok, I am working on the changes you have requested.

circuitcipher’s picture

circuitcipher commented
Status: Needs work » Needs review

I have completed the requested changes.

circuitcipher’s picture

circuitcipher commented

I wanted to add a quick explanation as to why I made the changes in commit "9b8e3d90 - Fix(entity): Correct target_bundles format for handler_settings"

I have been working locally on fixing Media Gallery compatibility with Layout Builder and found out that the core logic in Drupal for generating sample content based on bundle types expects that a field's "handler_settings" => "target_bundles" be in a specific format. This fix will be crucial to future enhancements I plan on doing to fix layout builder compatibility.

ivnish’s picture

ivnish
he/him
commented

Merged! Thanks!

ivnish’s picture

ivnish
he/him
commented
Assigned: circuitcipher » Unassigned
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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