Why is this theme-file present? Default there are no image-field added to the article bundle, so why style something that's not there. Kind of confusing too, and irritating to undo in a subtheme since I cant/shouldnt remove it from the parent theme.

CommentFileSizeAuthor
#11 remove_field--node--field-image--article.html_.twig-2834878.patch2.88 KBsahilsharma011
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joos created an issue. See original summary.

markhalliwell’s picture

markhalliwell
Primary language English
CreditAttribution: markhalliwell at Tag1 Consulting commented
Related issues: +#2633742: Align default image in article content type

Default there are no image-field added to the article bundle

Um, when did that change? Because I don't think that's actually true of a vanilla install.

I'm half/half on this. One one hand, it's good that, if a user uploads a pic, it's aligned. On the other, yes, it's a pain to override when needed.

joos’s picture

joos CreditAttribution: joos commented

Maybe I'm wrong about that, and it is the "Basic page" that don't have an image by default.

However I don't think its a good idea to overstyle a base-theme. It's better to ship the image-template-override in the child-theme-folder and let the themer decide if he/she wants to keep it or not.

The current solution also render a col-sm-4 (the image container) directly inside a col-sm-xx (the article container) without a row in between, which result in a double padding on the left side of the image (or both sides if its 100% wide).

I can understand why its hard to remove it now, hovever, since users may depend on this behaivour.

markhalliwell’s picture

markhalliwell
Primary language English
CreditAttribution: markhalliwell at Tag1 Consulting commented

However I don't think its a good idea to overstyle a base-theme.

I tend to agree with this.

I'd be ok removing this template entirely, provided that there's a change record attached with this issue explaining why it was removed and how to add it back in a sub-theme.

joos’s picture

joos CreditAttribution: joos commented

Ah, nice. Thanks. We could move the file to the starterkit/.../templates folder?

An explanation could be something like:
"
The bootstrap base theme is not responsible for the layout of field instances that may be altered in the cck. This should be done in the childtheme implementation, where we now ship this template instead. Copy the file from the starerkits template folder to you themes template folder in order to keep this functionality.
"

markhalliwell’s picture

markhalliwell
Primary language English
CreditAttribution: markhalliwell at Tag1 Consulting commented

I don't think there's any reason to keep it. It was an attempt to make it "look good out-of-the-box". I think a change record will suffice if anyone really wants to put it back in their sub-theme.

hass’s picture

hass CreditAttribution: hass commented

My case #2641380: image-widget.html.twig references a non-existing classy/image-widget library sounds related. That means 8.3.0+

markhalliwell’s picture

markhalliwell
Primary language English
CreditAttribution: markhalliwell at Tag1 Consulting commented

No.......... not related at all.

Christopher Riley’s picture

Christopher Riley CreditAttribution: Christopher Riley commented

This creates an issue when you are trying to use things such as Display Suite. I am in the process of going through the templates using both DS as well as Panels and coming up with some really FUNKY defaults.

sahilsharma011’s picture

sahilsharma011 CreditAttribution: sahilsharma011 at Google Summer of Code commented
Assigned: Unassigned » sahilsharma011

I'd like to provide a patch. My current understanding of this issue is that field--node--field-image--article.html.twig file should be removed because it'll not be a a good idea to

overstyle a base-theme.
sahilsharma011’s picture

sahilsharma011 CreditAttribution: sahilsharma011 at Google Summer of Code commented
Status: Active » Needs review
FileSize
remove_field--node--field-image--article.html_.twig-2834878.patch2.88 KB

Removed file field--node--field-image--article.html.twig.

Christopher Riley’s picture

Christopher Riley CreditAttribution: Christopher Riley commented

Instead of having to use the patch you can of course just replace the field specific file with another template within your sub-theme.

markhalliwell’s picture

markhalliwell
Primary language English
CreditAttribution: markhalliwell at Tag1 Consulting commented
Status: Needs review » Fixed

Change record: https://www.drupal.org/node/2868469

markhalliwell’s picture

markhalliwell
Primary language English
CreditAttribution: markhalliwell at Tag1 Consulting commented
Assigned: sahilsharma011 » Unassigned
joos’s picture

joos CreditAttribution: joos commented

Thanks!

Status: Fixed » Closed (fixed)

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