Problem/Motivation

The themes #image_formatter and #image_style are both taking the image style name as argument. Then the image style object is loaded in the theme layer, in preprocess. That's a bad design because puts the loading and validating objects logic in the theme layer. Theme layer should not be in business of loading and validating. These are backend operations. The backend should be responsible of loading, validating and preparing the correct image style to be used by the theme.

Proposed resolution

  1. Deprecate the usage of #image_style theme variable 'style_name' in favour of a new 'style' that will hold the entire image style object. If the deprecated variable 'style_name' will get a non-empty value, that will take precedence in theme preprocess to assure a full BC.
  2. Deprecate the usage of #image_formatter theme variable 'image_style' in favour of a new 'style' that will hold the entire image style object. If the deprecated 'image_style' will get a non-empty value, that will take precedence in preprocess to assure a full BC.
  3. Make core use the new variables.
  4. In 9.0.x drop the deprecated variables. Open a followup for that.

Remaining tasks

Open a 9.0.x followup for removing deprecated variables.

User interface changes

None.

API changes

  • New variable 'style' available to 'image_style' theme.
  • New variable 'style' available to 'image_formatter' theme.

Data model changes

None.

CommentFileSizeAuthor
#25 reroll_8-25.txt8.68 KBpradhumanjain2311
#25 2589847-25.patch5.36 KBpradhumanjain2311
#8 2589847-8.patch6.88 KBmondrake

Issue fork drupal-2589847

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
    Support from Acquia helps fund testing for Drupal Acquia logo

    Comments

    claudiu.cristea created an issue. See original summary.

    claudiu.cristea’s picture

    claudiu.cristea
    Primary language Romanian
    Location Arad 🇷🇴
    CreditAttribution: claudiu.cristea at Webikon commented
    Issue summary: View changes
    claudiu.cristea’s picture

    claudiu.cristea
    Primary language Romanian
    Location Arad 🇷🇴
    CreditAttribution: claudiu.cristea at Webikon commented
    Issue summary: View changes
    mondrake’s picture

    mondrake
    Location 🇮🇹
    CreditAttribution: mondrake as a volunteer commented
    Related issues: +#2479487: ImageStyles can be deleted while having dependent configuration.

    Yes! That's in line with what discussed in #2479487-85: ImageStyles can be deleted while having dependent configuration. and below. I suppose it's 8.1.x stuff, though?

    claudiu.cristea’s picture

    claudiu.cristea
    Primary language Romanian
    Location Arad 🇷🇴
    CreditAttribution: claudiu.cristea at Webikon commented

    I would like to have some input from the theme team.

    claudiu.cristea’s picture

    claudiu.cristea
    Primary language Romanian
    Location Arad 🇷🇴
    CreditAttribution: claudiu.cristea at Webikon commented
    Issue summary: View changes
    star-szr’s picture

    star-szr
    he/him
    Primary language English
    CreditAttribution: star-szr as a volunteer commented
    Related issues: +#2361299: Twig image style filter/function

    Architecturally it does seem cleaner but maybe there's a use case here. I might have a clearer opinion once I see a patch, or even some before/after pseudo code.

    Would the style name still be available to the theme layer if one wanted to add a theme suggestion based on the style name, for example? I guess you could always call ->getName() on the ImageStyle object?

    Adding a somewhat related issue that wants to load image styles (by name) via Twig template to draw a parallel there.

    mondrake’s picture

    mondrake
    Location 🇮🇹
    CreditAttribution: mondrake as a volunteer commented
    Version: 8.0.x-dev » 8.1.x-dev
    Status: Active » Needs review
    FileSize
    2589847-8.patch6.88 KB

    This is the (my) idea.

    @Cottser

    I guess you could always call ->getName() on the ImageStyle object?

    Exactly, see the example in template_preprocess_image_style for passing the style name to the 'image' theme.

    claudiu.cristea’s picture

    claudiu.cristea
    Primary language Romanian
    Location Arad 🇷🇴
    CreditAttribution: claudiu.cristea at Webikon commented
    Status: Needs review » Needs work

    Great! :)

    1. +++ b/core/modules/image/image.field.inc
@@ -44,14 +45,26 @@ function template_preprocess_image_widget(&$variables) {
+ * @todo 'image_style' variable is deprecated as of Drupal 8.1.x, will be
+ *   removed in Drupal 9.0.x. Use the 'style' variable to pass in an
+ *   ImageStyleInterface object.

      Nice. I would add at the end of @todo a new phrase: "The image style name can be extracted from the 'style' variable $variables['style']->getName()". Not very sure about the grammar. 

    2. +++ b/core/modules/image/image.field.inc
@@ -44,14 +45,26 @@ function template_preprocess_image_widget(&$variables) {
-      '#style_name' => $variables['image_style'],
+      '#style' => ImageStyle::load($variables['image_style']),
...
+      '#style' => $variables['style'],

+++ b/core/modules/image/templates/image-formatter.html.twig
@@ -5,7 +5,8 @@
+ * - style: An optional image style object.

      I'm confused now about the scope of the entire issue. So, we pass an object to Twig template? How to use an object in a Twig template? Maybe it's possible (I'm ignorant with theming) but isn't this more complicated for a themer than before? I think a themer should receive only scalars and in some special circumstances sequences or associative arrays. I think we need somehow to extract everything that is usable in a template from the object and inject them as scalars into the Twig template. I see that new 'style_name' variable in image-style.html.twig. I like that and maybe we can do the same in image-formatter? Maybe along with all other valuable info embedded in the image style object? 

    3. +++ b/core/modules/image/image.module
@@ -107,6 +107,8 @@ function image_theme() {
+        // @todo remove deprecated 'style_name' variable in #@todo

@@ -145,7 +147,14 @@ function image_theme() {
+        // @todo remove deprecated 'image_style' variable in #@todo

@@ -268,9 +278,19 @@ function image_style_options($include_empty = TRUE) {
+  // @todo BC: remove 'style_name' variable in #@todo

      A little bit confused abut this comment :)

    star-szr’s picture

    star-szr
    he/him
    Primary language English
    CreditAttribution: star-szr as a volunteer commented

    I think the #2 change is so that you can still get the image style name for example. Because the new way would not pass the image style name. In a way this could maybe be documented stronger so it's not just optional, but that's just based on that snippet, been a while since I looked at the full patch (and on my phone now).

    Version: 8.1.x-dev » 8.2.x-dev

    Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

    Version: 8.2.x-dev » 8.3.x-dev

    Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

    Version: 8.3.x-dev » 8.4.x-dev

    Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

    Version: 8.4.x-dev » 8.5.x-dev

    Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

    Version: 8.5.x-dev » 8.6.x-dev

    Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

    Version: 8.6.x-dev » 8.7.x-dev

    Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

    Version: 8.7.x-dev » 8.8.x-dev

    Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

    Version: 8.8.x-dev » 8.9.x-dev

    Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

    Version: 8.9.x-dev » 9.1.x-dev

    Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

    Version: 9.1.x-dev » 9.2.x-dev

    Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

    Version: 9.2.x-dev » 9.3.x-dev

    Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

    Version: 9.3.x-dev » 9.4.x-dev

    Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

    Version: 9.4.x-dev » 9.5.x-dev

    Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

    kim.pepper’s picture

    kim.pepper
    Primary language English
    Location 🏄‍♂️🇦🇺Sydney, Australia
    CreditAttribution: kim.pepper at PreviousNext commented
    Category: Bug report » Task
    Issue tags: +Bug Smash Initiative

    Reviewed this as part of Bug Smash Initiative. As this is more about changing existing functionality, I'm reclassifying as a Task.

    pradhumanjain2311’s picture

    pradhumanjain2311 CreditAttribution: pradhumanjain2311 at OpenSense Labs for DrupalFit commented
    FileSize
    2589847-25.patch5.36 KB
    reroll_8-25.txt8.68 KB

    Patch reroll for version 9.5.x. Needs review.
    Still needs work for comment #9

    Version: 9.5.x-dev » 10.1.x-dev

    Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

    Version: 10.1.x-dev » 11.x-dev

    Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.