Problem/Motivation

Since the latest update, all none responsive images are broken:

Images are printed like:

<img class="img-fluid">

This is also the case for the responsive images fallback image.

(Reported by @thomas.frobieter)

UPDATE: The fix is to copy the image.html.twig template from the radix starterkit to your sub-theme to receive the change made in 6.0.x-rc7.

Comments

anybody created an issue. See original summary.

anybody’s picture

Priority: Major » Critical
danchadwick’s picture

Category: Bug report » Support request
Priority: Critical » Normal
Status: Active » Fixed
Related issues: +#3482516: Inconsistent `attributes` handling

Thanks for the report. I'm 92.6% sure this is related to the changes in the template and component template made in in #3482516. Please adjust your theme's templates to correspond to the new radix template (or replace both your your subtheme's templates).

I did note in the rc7 release notes that there are template changes. The dropdown and dropdown-menu both changes, for example.

If this isn't this issue, please do re-open with more information.

The only other change I see is the removal of the trailing closing slash in the image tag, and a major change in the template for the user_picture in the comment template, either of which seem relevant here.

anybody’s picture

Hi @danchadwick thanks - i forwarded that to @thomas.frobieter - we'll check that tomorrow.
Thank you so much for your impressive work on radix!

Once radix has a stable release, such breaking changes will be made in major releases, I guess?

danchadwick’s picture

Issue summary: View changes

@anybody - Re breaking changes: I'm not the primary maintainer. That's a question for @doxigo. Certainly, such changes to a stable version should be mentioned in the release notes. Due to the nature of how the sub-theme has copies of templates (and theme functions and CSS and JS), it's becomes very hard to make improvements to the base theme without some manual work needed upon upgrade. Personally, I'd like to see minor-level version change for such things (e.g. 6.8.3 -> 6.9.0). That way the major version tracks the "bootstrap 5, SDC-aware" version.

And I do agree that a patch-level version should be backward-compatible.

anybody’s picture

@danchadwick thanks!
I think Drupal (and modules /themes) stick to semantic versioning https://semver.org typically:

The three types of versions are:

  • Major versions contain breaking changes.
  • Minor versions add new features or deprecate existing features without breaking changes.
  • Patch versions fix defects or optimize existing features without breaking changes.

Version numbers are presented Major.Minor.Patch, for example, 3.2.1.

I'd vote to do it like that, once a stable version has been released, because I think stability is more important than new features or even bugfixes. Release notes are important, but still risky.

But that's just my 2 cents and experience. (Not meant to criticise anyone! You're doing great things here!) :)

tstermitz’s picture

I can verify that copying the new, RC7 starterkit template file "image.html.twig" into my sub-theme fixes the missing image problem

I'm sure I have to go through all my sub-theme templates and update files to catch or avoid other problems. I've been waiting to do that until we get past the RC phase. I have been monitoring the issue queue and noticing that the core team has been doing some major reorganization.

I understand and accept that Radix has gone through some changes, and I appreciate all the work that has gone into it. The hazard of living on the bleeding edge: sometimes you have to redo some stuff.

Thanks to all...

doxigo’s picture

to add my two cents:
If we make a change that has breaks things for existing websites, we should do a revert for now. if it's a major change that we can't live without, we can do a new major release even and move forward with that. Otherwise not all people will find this thread.

danchadwick’s picture

I think the ship has sailed on reversing the change. It was pushed on November 21, 2024. There are about 500 reported users of 6.0.x. Certainly some of them (like me) will have started with this change and changing back would break my site.

I just now updated the release notes to specifically call out the change to the image template.

This issue highlights that it is somewhat brittle to have the templates (and associated x.theme files with preprocess functions) copied from the base theme to the sub-theme, and them have them use the base theme's components.

Some opttions:
- Do nothing and rely on the release notes. If you update without reading release notes, particularly for a pre-stable version, you are taking a risk, IMO.
- Change the version to 6.1.0-rc1.
- Change the version to 7.0.0-rc1
- Change the image.twig file to contain:
{% set image_attributes = image_attributes ?? attributes ?? create_attribute() %}
and them immediately release 6.0.0-rc8. (I need to confirm the that associativity of the ?? operator correct :) )

I don't love the last option because a) it may trigger an issue with the schema and we don't want to change the schema to include it, lest we encourage people to use it and b) it adds cruft to the first stable release of this branch. This is a decision for @doxigo.

danchadwick’s picture

Version: 6.0.0-rc7 » 6.0.x-dev
Status: Fixed » Active
tstermitz’s picture

I vote to bite the bullet and keep these changes. Perhaps with a major version number change: 6.1.x I guess.

Radix has made some big changes around components to make them more logical and modern (I trust). I always assumed by working with Radix 6.x from dev to RC, that upon release I would need to do a full survey of my templates and sub-theme files. I've already done that occasionally. It would be nice to do that ONCE, rather than frequently.

It is not a huge burden to me as my site is not very complex, and I don't have an intense work schedule.

doxigo’s picture

Okay we will definitely will have unhappy users, but what's done is done. I suppose we can keep the 6.0.x since in Drupal, 6.1 and 6.0 is not that different. Also we are still on RC, so we have an excuse :)

anybody’s picture

Thanks @doxigo - I totally agree and I think it should primarily be a learning for the future to use the semantic release rules to let people know about the risk of updates / upgrades (once stable). Thank you so much once again, you're doing a great job and we're really thankful!

I think this issue could be kept open for 2w for information or so and then be set fixed?

danchadwick’s picture

Agree on 2 weeks, although I'm hoping for a stable within that time. I was thinking a week of RC7 with no significant issues before stable.

danchadwick’s picture

Title: Images are broken since 6.0.0-rc7 (only none responsive images) » Images don't display since 6.0.0-rc7
Issue summary: View changes

  • doxigo committed 958010a5 on 6.0.x
    Issue #3498545: Add backward compatibility for image loading issue
    
doxigo’s picture

Status: Active » Fixed

Okay I pushed a backward compatibility fix for this since one of my projects also got hit with this. marking this as fixed for now, and taggign a stable release 6.0.0 🎉

Status: Fixed » Closed (fixed)

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