So this could be a "works as designed" situation, but I think solving this would be an improvement to the way that DS Extras finds field templates/functions. I think there this is the same issue mentioned at #1265920-24: The theme's field.tpl.php should be used unless field display settings exist when "Default" option is selected.
I'm using DS Extras to specify a default field template and I'm choosing "Minimal". So all my Display Suite fields are going to use the Minimal field template. For one specific field, field_event_dates, I choose "Drupal Default" as the field template in the Manage Display UI.
When ds_extras_preprocess_field() gets called, I would think that the theme_hook_suggestions should end up like:
field__date
field__field_event_dates
field__event
field__field_event_dates__event
These are the "Drupal default" suggestions before ds_extras_preprocess_field() is called.
Instead, this field ends up switching to the minimal field templates, as specified in the ft-default variable (via the DS Extras configuration UI).
I think the correct way that ds_extras_preprocess_field() should handle this situation is to just leave the theme_hook_suggestions alone since I am specifying "Drupal default" as the field template rather than specifying "The configured DS Extras default field template". To put it another way, if you select a default field template (Minimal, Full Reset), there is no other way of getting back to the Drupal default field template for a specific field.
Comment | File | Size | Author |
---|---|---|---|
#1 | ds-wrong-hook_theme_suggestions-1885480-1.patch | 1.2 KB | JonMcL |
Comments
Comment #1
JonMcL CreditAttribution: JonMcL commentedA rather simple patch attached that possibly solves this issue. Feedback needed and welcome.
Comment #2
swentel CreditAttribution: swentel commentedHmm this makes very much sense indeed, setting to needs review so I can let it test next week.
Comment #3
jyve CreditAttribution: jyve commentedPatch tested, and makes perfect sense!
Comment #4
swentel CreditAttribution: swentel commentedcommitted and pushed on 7.x-2.x and 7.x-1.x, thanks!
Moving to 8.x-2.x.
Comment #5
swentel CreditAttribution: swentel commentedAnd committed and pushed to 8.x-2.x branch as well.
Comment #6
donquixote CreditAttribution: donquixote commentedIt is enough to just check
if (empty($config['func']))
.empty()
implies!isset()
, and will not break on undefined variables.Comment #7
donquixote CreditAttribution: donquixote commentedAlso, if we test
empty()
there, we should also testempty()
in the first condition, like so:If
$config['func']
has any value that PHP considers empty, we don't want to build functions with it. Right?Or we only need
isset()
in both cases.Furthermore:
The existence of the first if () tells us that the only relevant case to exclude down in this
elseif ()
block is where$config['func'] === 'theme_field'
.Comment #8
donquixote CreditAttribution: donquixote commentedAnd a different problem:
In the second block with
$field_instance_info['ds_extras_field_template']
, we also don't want to overwrite$config['func'] === 'theme_field'
, do we? So the applied patch / commit is incomplete.Fixing all this also allows to simplify the code a lot.
Maybe I will open a new issue for this.
Comment #9
donquixote CreditAttribution: donquixote commentedAnd in the second block (condition broken to multiple lines):
In the case of
$field_instance_info['ds_extras_field_template'] === 'theme_field'
, it should not fall through to the next condition, but rather just stop altogether.Comment #10
donquixote CreditAttribution: donquixote commentedFollow-up: #2767615: Refactor and fix ds_extras_preprocess_field()