Closed (fixed)
Project:
radix
Version:
6.0.x-dev
Component:
Code
Priority:
Normal
Category:
Support request
Assigned:
Unassigned
Reporter:
Created:
9 Jan 2025 at 16:19 UTC
Updated:
30 Jan 2025 at 12:24 UTC
Jump to comment: Most recent
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
Comment #2
anybodyComment #3
danchadwick commentedThanks 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.
Comment #4
anybodyHi @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?
Comment #5
danchadwick commented@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.
Comment #6
anybody@danchadwick thanks!
I think Drupal (and modules /themes) stick to semantic versioning https://semver.org typically:
The three types of versions are:
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!) :)
Comment #7
tstermitzI 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...
Comment #8
doxigo commentedto 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.
Comment #9
danchadwick commentedI 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.
Comment #10
danchadwick commentedComment #11
tstermitzI 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.
Comment #12
doxigo commentedOkay 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 :)
Comment #13
anybodyThanks @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?
Comment #14
danchadwick commentedAgree 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.
Comment #15
danchadwick commentedComment #17
doxigo commentedOkay 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 🎉