Problem/Motivation

Some image style effects plugins (ie. Focal point) need the image dimensions to work.
As \Drupal\styleguide\Plugin\Styleguide\ImageStyleguide does not provide them, the styleguide page throws a fatal error when one of the image styles needs them.

Steps to reproduce

  1. Enable Styleguide
  2. Enable Focal point
  3. Configure one image style that use focal point
  4. Try to load the styleguide page

Proposed resolution

Provide widht and height values.

Issue fork styleguide-3258025

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

DuaelFr created an issue. See original summary.

DuaelFr opened merge request !4

duaelfr’s picture

duaelfr
he/him
Primary language French
Location Montpellier, France
commented
Status: Active » Needs review

Dave Reid made their first commit to this issue’s fork.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented

I'm not able to replicate the issue, I have a site using Focal Point and I'm not seeing any fatal errors on the current 2.x branch.

duaelfr’s picture

duaelfr
he/him
Primary language French
Location Montpellier, France
commented

(I'll try to find time to reproduce that tomorrow. In the meantime I rerolled the MR)

duaelfr’s picture

duaelfr
he/him
Primary language French
Location Montpellier, France
commented

@Dave Reid Thank you for trying to reproduce. I found out that this has been fixed in the Focal point module by "chance" (D10 compatibility issue)
https://git.drupalcode.org/project/focal_point/-/commit/d6f8823#035115c4...

I don't know if other image style extension modules have the same kind of issue or not but it seem safer to me to assume it could.
After digging a bit I now see two options:

  1. Merge this MR as it is because it's kind of doing the same that Core does in template_preprocess_image_style_preview()
  2. Use the '#theme' => 'image_style_preview' in your module instead of creating a custom preview so you don't have to worry about it anymore

What do you think?

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented

I'm torn about this because I think not providing the height and width is generally the default behavior for theme_image_style. There are several usages in core itself that do not do this:

\Drupal\media\MediaListBuilder::buildRow()

        $row['thumbnail']['data'] = [
          '#theme' => 'image_style',
          '#style_name' => 'thumbnail',
          '#uri' => $thumbnail_uri,
          '#height' => 50,
          // Note no #width attribute provided.
        ];

\Drupal\media_library\Form\AddFormBase::buildEntityFormElement()

      $element['preview']['thumbnail'] = [
        '#theme' => 'image_style',
        '#style_name' => 'media_library',
        '#uri' => $thumbnail_uri,
      ];

I also find many uses of this in contrib as well. I am tempted to say this was a bug in Focal Point that was resolved and not here, as this seems to be a fairly common use without providing them.

duaelfr’s picture

duaelfr
he/him
Primary language French
Location Montpellier, France
commented
Status: Needs review » Closed (works as designed)

No need to keep that open :)
Thanks Dave!