User tests: Successful: Unsuccessful:
Pull Request for Issue #17399 .
Replaces the 'Image Float ' field in com_contents with an 'Image Class' field (For reasons why see the original issue.. #17399)
Navigate to the Images and Links tab in article edit. Set an image and add a utility class to the Image Class field (eg. float-right). Check frontend and ensure class has been applied to the image (eg. image floated to right / inspect element).
Category | ⇒ | Administration com_content Language & Strings Layout |
Status | New | ⇒ | Pending |
The problem I see with this change is that
it means that we will end up with unused params in the database on upgraded web site with no way to remove them without editing the database directly
This is changing users data
This will require a site owner to review all their existing content and to add in the class manually. On my blog for example I would have to do this for thousands of articles
This will require every template to be updated
Just because we can break backwards compatibility does not me we should do it without carefully considering the consequences.
Off the top of my head I can't think of an example of where an option has been removed that effects user data like this. Probably because it hasn't been done for the reasons I outlined above.
This option as it is, should never have been added in the first place. Tying an option in core directly to any CSS property is wrong and will always cause a b/c break at some point. Changing this to Image Class means we never have to worry about a b/c break in instances like this again. It creates a layer that can be changed as CSS changes.
Fact is it's there now and it's our responsibility now to deal with how to go forward with it. Unfortunately we are approaching uncharted territory since we historically have avoided changes that impact user data and this change would need a migration tool to "fix" things the right way (granted it could be done with a single PHP file with a couple of prompts to convert a float to a CSS class, but it still couldn't be automated).
The only solution I can suggest within my capabilities is to use the old param names (float_intro & float_fulltext). Value saved for each of these is left
and right
so I can just define these as classes in the CSS to float left and right respectively...
.left.item-image {
float: left;
}
.right.item-image {
float: right;
}
Not the 'cleanest' solution but will resolve all issues mentioned by @brianteeman
Well aware, I will say though that solution will probably have to be a standalone tool because we should not try to blindly migrate user data unless we have 100% certainty our upgrade process can handle it without screwing existing user sites up.
And even then, I'd still do it standalone for the simple fact if we're talking about a site with a large quantity of content, we're going to be much more prone to running into resource issues if we're doing that change in the middle of the regular update routine.
I know that both you and George are aware of the problem, all I wanted to point out is that if we are going to fix the captions legacy somehow then we should consider this proposal as well. Looking ahead this could be very beneficial for all devs...
Category | Administration com_content Language & Strings Layout | ⇒ | Administration com_content Language & Strings Templates (admin) SQL Installation Postgresql Layout Front End Templates (site) Unit Tests |
Labels |
Added:
?
?
|
Amended the PR as I suggested here #17402 (comment)
Thanks. I cannot test ignore today but will do ASAP
I have tested this item
This didn't work on Intro-Image as it was in Joomla 3 too ("Use global (*)" didn't change alignment) which is not Scope of this PR.
@franz-wohlkoenig Thank you for the test
Article, Full Image using "Use global (left)": after PR Image stay left even "Image Class" is empty
Check your global config as this is set to 'left' by default with the current sample data.
Labels |
Added:
?
|
When I enter "right" for the full image I get a class "right item-image" via layouts/joomla/content/full_image.php
.
When I enter "right" for the intro image I get a class "pull-right item-image" via layouts/joomla/content/intro_image.php
.
following and will test/feedback tomorrow or late tonight since its class night.
for the test can you make a PR against this Repo https://github.com/joomla/test-integration files are in the 4.0-dev branch /stubs/database and remove the files from your PR here, thanks
Status | Pending | ⇒ | Information Required |
Category | Administration com_content Language & Strings Layout Templates (admin) SQL Installation Postgresql Front End Templates (site) Unit Tests | ⇒ | Administration com_content Front End Installation Layout Postgresql SQL Templates (admin) Templates (site) Unit Tests |
Error while applying patch..Wasn't able to test the issue
whats the matter with using the default bootstrap "pull-right/pull-left? which if I recall doesn't do much in grid
In general, since we now have flex, floating elements should be avoided considering the negative effect it can have surrounding elements.
Probably easier creating a new pr than bring this one up to date so gonna close.
Status | Information Required | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2019-10-25 08:48:11 |
Closed_By | ⇒ | ciar4n |
Category | Administration com_content Layout Templates (admin) SQL Installation Postgresql Front End Templates (site) Unit Tests | ⇒ | Administration com_content Language & Strings Templates (admin) SQL Installation Postgresql Layout Front End Templates (site) Unit Tests |
I have tested this item✅ successfully on 2be532b
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/17402.