I tried to override the Library image style created by the scald_dnd_library.module, but Drupal throws the error "Please only use lowercase alphanumeric characters, underscores (_), and hyphens (-) for style names.", preventing override.

I've attached a patch with corrections to the hook_image_default_styles() to allow override. I would like larger thumbnails in the library.

CommentFileSizeAuthor
#22 library-image-style-name-fix-2571203-22.patch2.14 KBgifad
#20 library-image-style-name-fix-2571203-20.patch2.13 KBgifad
#18 library-image-style-name-fix-2571203-18.patch2.07 KBgifad
#16 library-image-style-name-fix-2571203-16.patch2.1 KBgifad
#11 library-image-style-name-fix-2571203-11.patch1.54 KBnagy.balint
#8 library-image-style-name-fix-2571203-8.patch924 bytesnagy.balint
#4 scald-dnd-image-style-2571203.patch750 bytesanchovie91471
#2 scald-dnd-image-style-2571203.patch767 bytesanchovie91471
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

anchovie91471 created an issue. See original summary.

anchovie91471’s picture

anchovie91471 CreditAttribution: anchovie91471 commented
FileSize
scald-dnd-image-style-2571203.patch767 bytes

Status: Needs review » Needs work

The last submitted patch, 2: scald-dnd-image-style-2571203.patch, failed testing.

anchovie91471’s picture

anchovie91471 CreditAttribution: anchovie91471 commented
FileSize
scald-dnd-image-style-2571203.patch750 bytes
anchovie91471’s picture

anchovie91471 CreditAttribution: anchovie91471 commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 4: scald-dnd-image-style-2571203.patch, failed testing.

anchovie91471’s picture

anchovie91471 CreditAttribution: anchovie91471 commented

OK. My patches suck. Instead of spamming the issues any more, I'll just put the code here and perhaps someone will be willing to create the patch from it.

Line 169 of scald_dnd_library.module

  if (!empty($atom->rendered->thumbnail_transcoded_url)) {
    $path = $atom->rendered->thumbnail_transcoded_url;
  }
  else {
    $path = image_style_url('library', $atom->thumbnail_source);
  }

Line 347 of scald_dnd_library.module

/**
 * Implements hook_image_default_styles().
 */
function scald_dnd_library_image_default_styles() {
  $presets = array();
  $presets['library'] = array(
    'label' => 'Library',
    'effects' => array(
      array(
        'name' => 'image_scale',
        'data' => array(
          'width' => '48',
          'height' => '',
          'upscale' => 0,
        ),
        'weight' => '0',
      ),
    ),
  );
  return $presets;
}
nagy.balint’s picture

nagy.balint CreditAttribution: nagy.balint commented
Status: Needs work » Needs review
FileSize
library-image-style-name-fix-2571203-8.patch924 bytes

Indeed that image style machine name should have only contained lowercase letters. As described in https://www.drupal.org/node/1577800

Attached the patch.

However we have to consider backward incompatibility implications. Its a built in style, so in theory it should not have been used by 3rd party. But the possibility is there to be used.

nagy.balint’s picture

nagy.balint CreditAttribution: nagy.balint commented
Version: 7.x-1.5 » 7.x-1.x-dev

Status: Needs review » Needs work

The last submitted patch, 8: library-image-style-name-fix-2571203-8.patch, failed testing.

nagy.balint’s picture

nagy.balint CreditAttribution: nagy.balint commented
Status: Needs work » Needs review
FileSize
library-image-style-name-fix-2571203-11.patch1.54 KB

One more place to modify.

gifad’s picture

gifad CreditAttribution: gifad commented
Status: Needs review » Needs work

Found an issue with scald_gallery module :
scald_gallery.install contains, hard-coded :

/**
 * Implements hook_install().
 */
function scald_gallery_install() {
  ScaldAtomController::addType('gallery', 'Gallery', 'Gallery');

  // Associate the gallery atom type to the "library" image style
  // in the library context.
  $context_config = scald_context_config_load('sdl_library_item');
  $context_config->transcoder['gallery']['*'] = 'style-Library';
  scald_context_config_save($context_config);

This makes ScaldAtomController::save() fail at line 304...

Any better solution than a hack in scald_context_config_load() ?

nagy.balint’s picture

nagy.balint CreditAttribution: nagy.balint commented

yes, we need to do something with existing data :(

nagy.balint’s picture

nagy.balint CreditAttribution: nagy.balint commented

I suppose an update hook could work for both modules, and the install can be fixed as well for new installations, but then it can only be fixed in the install once we put a requirement on the scald version after this commit...

Not so simple to do this then.

nagy.balint’s picture

nagy.balint CreditAttribution: nagy.balint commented

Actually i dont think we even need those 3 lines:

  $context_config = scald_context_config_load('sdl_library_item');
  $context_config->transcoder['image']['*'] = 'style-Library';
  scald_context_config_save($context_config);

Because the render of the representation will not care about that anyways.

If we can simply remove it, then new installs will not have a problem, although the update hook might still be needed to fix old configuration when updating scald.

gifad’s picture

gifad CreditAttribution: gifad commented
Status: Needs work » Needs review
FileSize
library-image-style-name-fix-2571203-16.patch2.1 KB

Added a workaround for existing 'style-Library'

Status: Needs review » Needs work

The last submitted patch, 16: library-image-style-name-fix-2571203-16.patch, failed testing.

gifad’s picture

gifad CreditAttribution: gifad commented
Status: Needs work » Needs review
FileSize
library-image-style-name-fix-2571203-18.patch2.07 KB

#16 got corrupted, reroll...

Status: Needs review » Needs work

The last submitted patch, 18: library-image-style-name-fix-2571203-18.patch, failed testing.

gifad’s picture

gifad CreditAttribution: gifad commented
Status: Needs work » Needs review
FileSize
library-image-style-name-fix-2571203-20.patch2.13 KB

Handled redundancies in scald_contexts() structure

Status: Needs review » Needs work

The last submitted patch, 20: library-image-style-name-fix-2571203-20.patch, failed testing.

gifad’s picture

gifad CreditAttribution: gifad commented
Status: Needs work » Needs review
FileSize
library-image-style-name-fix-2571203-22.patch2.14 KB

Stupid typo...

  • nagy.balint committed 39d16fd on 7.x-1.x authored by gifad 
    Issue #2571203 by gifad, nagy.balint, anchovie91471: Corrected image...
nagy.balint’s picture

nagy.balint CreditAttribution: nagy.balint commented
Status: Needs review » Fixed

Thanks, Committed.

Status: Fixed » Closed (fixed)

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

DuneBL’s picture

DuneBL
Primary language French
CreditAttribution: DuneBL commented

With this patch, we can change the library style... this is great.
But, it could be nice to have this change reflected into the dnd library as well.
For now, the width of the image is coming from the css:
/modules/library/dnd/css/dnd-library.css b/modules/library/dnd/css/dnd-library.css

.dnd-library-wrapper .editor-item .image img {
    width: 48px;
}

But of course, the width of the dnd library should be changed as well...

nagy.balint’s picture

nagy.balint CreditAttribution: nagy.balint commented

Please open a new issue with this. Thanks!

DuneBL’s picture

DuneBL
Primary language French
CreditAttribution: DuneBL commented

done!
https://www.drupal.org/node/2698349