New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Change params variable to reflect merged state #16971
Conversation
I have tested this item ✅ successfully on 8d0e5f5 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16971. |
1 similar comment
I have tested this item ✅ successfully on 8d0e5f5 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16971. |
It seems drone failed for reasons not related to this PR (some phpcs script missing). |
I saw the bug after updating. Gave it a try by exchanging my override with this new version in three different websites. Works fine now :-) |
rtc This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16971. |
I think we need same edit also in the template beez override |
hm, if all template overrides have to be updated, isn't it a b/c break then? in other words: something was changed and now breaks template overrides. This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16971. |
Not according to the B/C promise:
|
ah, ok. thanks for the info! |
@lunalars actually the mark up with this PR remains the same so this is just a bug fix and it shouldn't affect you unless you already have created an override with the latest version |
@dgt41 I am affected, but I'm not sure why. Maybe I'm doing somthing wrong in a plugin I created, which adds some fields to the contact edit form in backend. These fields are saved with the params of the contact. Now I'm confused :-) But maybe I should just add my fields in a different way. Edit: And yes, my problem is not related to this PR |
I'm switching back to pending based on a special request by @matrikular This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16971. |
Thanks @zero-24. I'll post an update soon (this evening). |
First of all, I feel responsible for this and do want to make it right. You can be assured, that it is not my intention to hide behind some "yet unwritten", "debatable", "between the lines" interpretation of the stated b/c promise regarding rendered markup. That said, personally - I would expect every site owner / admin / webmaster to thoroughly check their overrides after each update and that a layout is hardly to keep free from "breaking" changes. A simple change in the data layer or parameter handling e.g. can very well affect the output. So much for cheap excuses,... I've reverted the changes back to the original state for the address layout and assigned the merged parameters to $this->params in the view. That way, there should be no need to change an already existing override. Well, at least not in this context (hopefully). Some more tests are highly appreciated, thanks. |
I have tested this item ✅ successfully on 597a503 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16971. |
I have tested this item ✅ successfully on 597a503 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16971. |
RTC after two successful tests. |
I have tested this item ✅ successfully on 597a503 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16971. |
I have tested this item ✅ successfully on 597a503 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16971. |
Pull Request for Issue #16968.
Summary of Changes
Change params variable to reflect merged state (in address sub-layout).
Testing Instructions
See #16968 (comment)
Expected result
The mentioned field values and icons are shown to the user.
Actual result
They are not.