[1.0.x] LightGallery Formatter

Thank you for applying!

Please read Review process for security advisory coverage: What to expect for more details and Security advisory coverage application checklist to understand what reviewers look for. Tips for ensuring a smooth review gives some hints for a smoother review.

The important notes are the following.

• New releases are not necessary for these applications, which could require changes that are not backward-compatible. Not creating new releases avoids any possible issue.
• Please do not change the branch to review once reviews started, except in the case the used branch needs to be deleted.
• If you have not done it yet, enable GitLab CI for the project, and fix what reported from the phpcs job. This help to fix most of what reviewers would report.
• For the time this application is open, only your commits are allowed. No other people, including other maintainers/co-maintainers can make commits.
• The purpose of this application is giving you a new drupal.org role that allows you to opt projects into security advisory coverage, either projects you already created, or projects you will create. The project status won't be changed by this application.
• Nobody else will get the permission to opt projects into security advisory policy. If there are other maintainers/co-maintainers who will to get that permission, they need to apply with a different module.
• We only accept an application per user. If you change your mind about the project to use for this application, or it is necessary to use a different project for the application, please update the issue summary with the link to the correct project and the issue title with the project name and the branch to review.

To the reviewers

Please read How to review security advisory coverage applications, Application workflow, What to cover in an application review, and Tools to use for reviews.

The important notes are the following.

• It is preferable to wait for a Code Review Administrator before commenting on newly created applications. Code Review Administrators will do some preliminary checks that are necessary before any change on the project files is suggested.
• Reviewers should show the output of a CLI tool only once per application. The configuration used for these tools needs to be the same configuration used by GitLab CI, stored in the GitLab Templates repository.
• It may be best to have the applicant fix things before further review.

For new reviewers, I would also suggest to first read In which way the issue queue for coverage applications is different from other project queues.

1. FILE: lightgallery_formatter.libraries.yml and modules/lightgallery_formatter_preview/lightgallery_formatter_preview.libraries.yml

version: VERSION

VERSION is only used by Drupal core modules. Contributed modules should use a literal string that does not change with the Drupal core version a site is using.

2. FILE: src/LightgalleryPluginBase.php

  /**
   * The entity type manager.
   *
   * @var \Drupal\Core\Entity\EntityTypeManagerInterface
   */
  protected EntityTypeManagerInterface $entityTypeManager;

  /**
   * Constructs a LightgalleryPluginBase object.
   *
   * @param array $configuration
   *   A configuration array containing information about the plugin instance.
   * @param string $plugin_id
   *   The plugin_id for the plugin instance.
   * @param mixed $plugin_definition
   *   The plugin implementation definition.
   * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager
   *   The entity type manager.
   */
  public function __construct(
    array $configuration,
    $plugin_id,
    $plugin_definition,
    EntityTypeManagerInterface $entity_type_manager,
  ) {
    parent::__construct($configuration, $plugin_id, $plugin_definition);
    $this->entityTypeManager = $entity_type_manager;
    $this->setConfiguration($configuration);
  }

FILE: src/Form/LightgalleryProfileForm.php

  /**
   * The entity type manager.
   *
   * @var \Drupal\Core\Entity\EntityTypeManagerInterface
   */
  protected $entityTypeManager;

  /**
   * The extension path resolver.
   *
   * @var \Drupal\Core\Extension\ExtensionPathResolver
   */
  protected $extensionPathResolver;

  /**
   * The module extension list.
   *
   * @var \Drupal\Core\Extension\ModuleExtensionList
   */
  protected $moduleExtensionList;

  /**
   * The app root path.
   *
   * @var string
   */
  protected $appRoot;

  /**
   * The module handler.
   *
   * @var \Drupal\Core\Extension\ModuleHandlerInterface
   */
  protected $moduleHandler;

  /**
   * The preview builder service (if available).
   *
   * @var \Drupal\lightgallery_formatter_preview\PreviewBuilder|null
   */
  protected $previewBuilder;

  /**
   * The LightGallery plugin manager.
   *
   * @var \Drupal\lightgallery_formatter\LightgalleryPluginManager
   */
  protected LightgalleryPluginManager $pluginManager;

  /**
   * Constructs a new LightgalleryProfileForm object.
   *
   * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager
   *   The entity type manager.
   * @param \Drupal\Core\Extension\ExtensionPathResolver $extension_path_resolver
   *   The extension path resolver.
   * @param \Drupal\Core\Extension\ModuleExtensionList $module_extension_list
   *   The module extension list.
   * @param string $app_root
   *   The app root path.
   * @param \Drupal\Core\Extension\ModuleHandlerInterface $module_handler
   *   The module handler.
   * @param \Drupal\lightgallery_formatter\LightgalleryPluginManager $plugin_manager
   *   The LightGallery plugin manager.
   */
  public function __construct(
    EntityTypeManagerInterface $entity_type_manager,
    ExtensionPathResolver $extension_path_resolver,
    ModuleExtensionList $module_extension_list,
    string $app_root,
    ModuleHandlerInterface $module_handler,
    LightgalleryPluginManager $plugin_manager,
  ) {
    $this->entityTypeManager = $entity_type_manager;
    $this->extensionPathResolver = $extension_path_resolver;
    $this->moduleExtensionList = $module_extension_list;
    $this->appRoot = $app_root;
    $this->moduleHandler = $module_handler;
    $this->pluginManager = $plugin_manager;
  }

FILE: modules/lightgallery_formatter_preview/src/PreviewBuilder.php

  /**
   * The entity type manager.
   *
   * @var \Drupal\Core\Entity\EntityTypeManagerInterface
   */
  protected $entityTypeManager;

  /**
   * The state service.
   *
   * @var \Drupal\Core\State\StateInterface
   */
  protected $state;

  /**
   * The file URL generator.
   *
   * @var \Drupal\Core\File\FileUrlGeneratorInterface
   */
  protected $fileUrlGenerator;

  /**
   * Constructs a PreviewBuilder object.
   *
   * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager
   *   The entity type manager.
   * @param \Drupal\Core\State\StateInterface $state
   *   The state service.
   * @param \Drupal\Core\File\FileUrlGeneratorInterface $file_url_generator
   *   The file URL generator.
   */
  public function __construct(
    EntityTypeManagerInterface $entity_type_manager,
    StateInterface $state,
    FileUrlGeneratorInterface $file_url_generator,
  ) {
    $this->entityTypeManager = $entity_type_manager;
    $this->state = $state;
    $this->fileUrlGenerator = $file_url_generator;
  }

New modules, which are compatible with Drupal 10 and higher versions are expected to include type declarations in property definitions, and use constructor property promotion.

Rest seems fine to me.

Please wait for other reviewers and Project Moderator to take a look and if everything goes fine, you will get the role.

hi @man-1982,

Overall the code looks solid, but I have 2 comments that require attention:

1. I wouldn't recommend having type: ignore in the lightgallery_formatter.schema.yml for the plugin settings, it's too permissive. Instead I'd define a schema for the plugin settings.

2. While the approach of Html::escape() + |raw is safe, I'd rather not use the twig raw filter, and instead take a more maintainable approach like a render array or similar:

'subHtml' => [
  '#type' => 'html_tag',
  '#tag' => 'h4',
  '#value' => $media->label(),
],

This way, twig doesn't need the raw filter anymore, and the autoescape is handled by the render array.

Hi @man-1982,

You don't need to define a full mapping for the plugin_settings in the lightgallery_formatter.profile.*, you can point dynamically to the plugin schema by using a wildcard, especially since each plugin can have its own definition which could defer from others. Here's an example in core (web/core/config/schema/core.entity.schema.yml):

field_formatter:
  type: mapping
  label: 'Field formatter'
  mapping:
    type:
      type: string
      label: 'Format type machine name'
      constraints:
        PluginExists:
          manager: plugin.manager.field.formatter
          interface: 'Drupal\Core\Field\FormatterInterface'
    label:
      type: string
      label: 'Label setting machine name'
    settings:
      type: field.formatter.settings.[%parent.type]
      label: 'Settings'
    third_party_settings:
      # Third party settings are always optional: they're an optional extension
      # point.
      requiredKey: false
      type: sequence
      label: 'Third party settings'
      sequence:
        type: field.formatter.third_party.[%key]

@man-1982 there are many issues open, at this point it's just waiting & patience.

+1 RTBC

A few suggestions to further improve the README, based on the drupal.org README template. These are merely suggestions for the benefit of users of the module.

1. Move developer documentation to a separate file: The "For Developers" section (DDEV setup, architecture, React build process, plugin development) makes up roughly half the README. Moving it to a CONTRIBUTING.md or docs/DEVELOPMENT.md would keep the README focused on what end users need: what the module does, how to install it, and how to configure it.
2. Also move documentation that is aimed at AI agents to a AGENTS.md file.
3. Add a table of contents: Given the length of the README, a table of contents near the top (like in the Views GeoJSON README) would help users navigate to the section they need.
4. The installation section can be as simple as explained in https://www.drupal.org/docs/develop/managing-a-drupalorg-theme-module-or...

What's already good: All four required sections (Introduction, Requirements, Installation, Configuration) are present and well-written. The Configuration section with numbered steps is particularly clear. The AI disclosure is commendable and follows Drupal's guidelines.

Great application and a very promising module! As a Drupal developer, I’d like to highlight a few strong architectural decisions in the LightGallery Formatter:

Smart use of the Plugin API: Splitting the functionality (Thumbnail, Zoom, Fullscreen, Share, etc.) into 11 independent plugins is the true "Drupal way". This approach makes the codebase clean, modular, and easily extensible for other developers.
Configuration Profiles: Moving the gallery settings into reusable Configuration Entities instead of overloading the Field Formatter settings is a brilliant idea. It prevents configuration duplication across different View Modes and integrates perfectly with the Configuration Management Initiative (CMI).

Modern stack: The use of React for rendering, combined with progressive enhancement and mobile gesture support, is exactly what’s often missing in many contrib media solutions.

The feedback from reviewers in the issue queue (regarding the hardcoded version: VERSION in libraries.yml and utilizing Constructor Property Promotion for PHP 8.1+) are absolutely standard and easy-to-fix minor details when aligning with Drupal 10/11 standards. The architectural foundation of the module itself looks incredibly solid.

I am changing priority as per Issue priorities.

Adjusting the issue status to reflect that it has been reviewed 3 times and appears to now be passing those reviews.

Thank you for your contribution and for your patience with the review process!

I am going to update your account so you can opt into security advisory coverage any project you create, including the projects you already created.

These are some recommended readings to help you with maintainership:

• Dries' post on Responsible maintainers
• Maintainership
• Git version control system
• Issue procedures and etiquette
• Maintaining and responding to issues for a project
• Release naming conventions.

You can find more contributors chatting on Slack or IRC in #drupal-contribute. So, come hang out and stay involved!
Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

I thank also all the reviewers for helping with these applications.