No Code Attached Yet
avatar Ruud68
Ruud68
19 Nov 2024

Steps to reproduce the issue

in components config.xml set the following fields:

		<field
			name="discount_recurringrenewal"
			type="radio"
			default="0"
			layout="joomla.form.field.radio.switcher"
			label="COM_COMPONENT_GENERIC_FIELD_DISCOUNTRECURRINGRENEWAL_LABEL"
			description="COM_COMPONENT_GENERIC_FIELD_DISCOUNTRECURRINGRENEWAL_DESC">
			<option value="0">JNO</option>
			<option value="1">JYES</option>
		</field>
		<field
			name="discount_recurringrenewal_percentage"
			type="number"
			default="10"
			min="1"
			max="100"
			step="1"
			label="COM_COMPONENT_GENERIC_FIELD_DISCOUNTPERCENTAGE_LABEL"
			description="COM_COMPONENT_GENERIC_FIELD_DISCOUNTRECURRINGRENEWALPERCENTAGE_DESC"
			showon="discount_recurringrenewal:1" />

in (in my case) product.xml set the following fields:

		<field
			name="discount_recurringrenewal"
			type="list"
			useglobal="true"
			class="form-select-color-state chzn-color-state custom-select-color-state"
			label="COM_COMPONENT_GENERIC_FIELD_DISCOUNTRECURRINGRENEWAL_LABEL"
			description="COM_COMPONENT_GENERIC_FIELD_DISCOUNTRECURRINGRENEWAL_DESC">
			<option value="0">JNO</option>
			<option value="1">JYES</option>
		</field>
		<field
			name="discount_recurringrenewal_percentage"
			type="number"
			default="10"
			min="1"
			max="100"
			step="1"
			label="COM_COMPONENT_GENERIC_FIELD_DISCOUNTPERCENTAGE_LABEL"
			description="COM_COMPONENT_GENERIC_FIELD_DISCOUNTRECURRINGRENEWALPERCENTAGE_DESC"
			showon="discount_recurringrenewal:1" />

Expected result

as it is in Joomla 5.1.4

in component options:
Image

in product view:
Image

Actual result

As it currently is in 5.2.1

in component options:
Image

in product view:
Image

System information (as much as possible)

Additional comments

No idea in what version / what changed triggered this

avatar Ruud68 Ruud68 - open - 19 Nov 2024
avatar Ruud68 Ruud68 - change - 19 Nov 2024
Labels Removed: ?
avatar joomla-cms-bot joomla-cms-bot - change - 19 Nov 2024
Labels Added: No Code Attached Yet
avatar joomla-cms-bot joomla-cms-bot - labeled - 19 Nov 2024
avatar Ruud68 Ruud68 - change - 19 Nov 2024
The description was changed
avatar Ruud68 Ruud68 - edited - 19 Nov 2024
avatar Ruud68 Ruud68 - change - 19 Nov 2024
The description was changed
avatar Ruud68 Ruud68 - edited - 19 Nov 2024
avatar brianteeman
brianteeman - comment - 19 Nov 2024

Try it without useglobal="true"

avatar Ruud68
Ruud68 - comment - 19 Nov 2024

@brianteeman when i remove the useglobal = true then I loose the global option inheritance: I then only have two options in the product: yes / no and not use global.

I need the global is that is where the main configuration is done which is then inherited by the product. per product I can then deviate when needed.

Issue here is in the showon="discount_recurringrenewal:1"

when use global is selected the value for recurringrenewal in J 5.1.4 was "" and is such the field would not show.
in J5.2.1 the value is stil "" but the showon behavior changed and acts not on the product value recurringrenewal (which is "") but on the global value recurringrenewal (which is 1)

This imo is a bc break.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44484.

avatar brianteeman
brianteeman - comment - 19 Nov 2024

My question was to determine if the problem was related to a change in showon or if it was related to useglobal. I understand that you want useglobal I just wanted to try and determine if that was (as I suspect) the root cause of the problem

avatar Ruud68
Ruud68 - comment - 20 Nov 2024

Ok, so the root cause of this B/C issue is due to this PR: #43842
it changes the behavior of showon to not take the value of the field as it should / is supposed to do but it instead takes the inherited value causing issues in the forms that use global fields
pinging @LadySolveig here as writer of this PR

avatar Ruud68
Ruud68 - comment - 20 Nov 2024

and just to note how 'bad' this is:

When you have in your component options a set of fields where the second part is a subform that can have multiple values and you group these together with the show on and fill in the form with e.g. 10 entries, then in the inheriting form that uses useglobal='true" will show Use Global (yes) (which is correct, but now it will display an empty form instead of the global configured form with 10 entries.

So this is really confusing as it is unclear what the configured values are that are used for the form: the empty values or the 10 configured in the component options....

So although I understand what this PR 'solves', I am now facing issues with what it breaks

avatar LadySolveig
LadySolveig - comment - 20 Nov 2024

Thank you for reporting, I will have a look and come back to you for this issue as soon as possible.

avatar LadySolveig
LadySolveig - comment - 20 Nov 2024

I just wanted to briefly summarize whether I have understood you correctly? @Ruud68

With the bug fix, the logic for useGlobal that we have all over in core was also applied to the showOn method.
i.e.

  • Configuration: true => selected field value in single entity: useGlobal (true) == true
  • Configuration: false ==> selected field value in single entitiy: **useGlobal (false)** == false

My understanding so far is, that you would like the logic to remain in place but to restore the bug and completely ignore the parameter set in the global configuration for the showOn logic and only trigger showOn if it is set directly in the single entitiy?

To take the example from above, this would then only be for showOn

  • Configuration: true => selected field value in single entity: useGlobal (true) == ``
  • Configuration: false ==> selected field value in single entitiy: useGlobal (false) == ``

For me, at first glance it is more of a structural misconception that was made possible by the bug.

If I want the option of a discount to be independent of the option of an individual discount per Entity, I would expect in the first case described that there is another switch for 'use individual discount' in the global configuration, for example, and that the discount is then linked to this via ShowOn.

This would ensure that the option for a discount on recurring renewals is entirely independent, which is what you actually want to achieve.

The same applies to the second example.
showOn itself has no logic to take over global values and is not intended for this purpose.
You either implement these yourself via your component or, if this is sufficient, use what the core makes available to you via useGlobal.
Again, I would expect a more stable implementation via an additional switch if I only need individual options in special cases..
This would avoid creating dependencies that are clearly not intended to depend.

avatar Ruud68
Ruud68 - comment - 20 Nov 2024

Hi @LadySolveig thanks for following up.
The issue here is that your PR didn't fix a bug, but changed existing functionality.
Before your PR things where working OK.

let me (try to) explain:
before the PR, an inherited list field (field with useglobal=true) has one added option: the "" (empty) option which indicates 'use global value'
In my use case the config list field has two options: yes (1) and no (0), and the field in the product.xml has after rendering with the useglobal=true three options: "" (empty = global), yes (1) and no (0)

The showon should act on the actual field value. in the inherited list field the possible values are empty, 1 and 0

So when the field value is empty (use global), the value for that field is NOT 1 but it is "" (empty)

This leads to the issue where the showon cannot be used anymore on the "use Global" setting (empty) as that is replaced by the yes / no setting from the 'parent' field.

So this actually broke the possibility to use showon based on the REAL value (which is not the value of the used global value) but the 'use global ' = empty value

So IMO your PR changed behavior without a replacement for the functionality required. There is no way with core functionality to get this functionality restored as it was.

avatar LadySolveig
LadySolveig - comment - 20 Nov 2024

I'll try it another way.

The logic in the entire core is currently structured in such a way that if the option is empty, check if there is a global option and than use this instead.

I'm not saying that's good or bad. I'm just saying that this is how it is currently mapped in the entire core.

Yes, you are right that there are three states in this case, but I have already explained above how the third state (empty) works everywhere else where there is a useGlobal in the field. It is only a "placeholder" for what is set in the global configuration.

My PR has removed this inconsistency so that the showOn now shows exactly the same behaviour.

The REAL value that you continue to work with in the rest of your code is in this case the one from the global config.

Even if it has now helped you personally and disguised the fact that dependencies have been built up here in your form, which de facto can no longer exist in the rest of your extension logic (unless you have just rewritten the core logic here), it is nonetheless still a fixed bug.

avatar Ruud68
Ruud68 - comment - 20 Nov 2024

@LadySolveig I understand how it works, I understand how and why it changed. That said: it is still a B/C issue (because it breaks functionality that developers build upon for years) and as such it should be documented so that developers who run into this know about it and get offered a workaround (or create one themselves).

The behavior you changed was working like that for as long as I can remember. I (developer) trust that is working as intended. If that is not the case it should be documented.

Like having a car where you tank gazoline and all of a sudden after years it stops working. The manufacturer tells you: yeay that was an error from the beginning as it was supposed to only be fileld with petrol, so we changed the engine to not accept gazoline anymore.

I hope my feedback helps.

avatar Ruud68
Ruud68 - comment - 21 Nov 2024

And just to add to the discussion:

the PR changed existing functionality, but only for 1 field (ListField), not for the other fields.

so it not only breaks how it used to work but also introduces ambiguity in the fact that the showon not works differently depending on which field you use it on.

e.g. when you have a field of type 'number' and set that to use global, then the showon will still work on the local set value and not on the global value as it now does with the list field....

This is not good especially as there is no way to configure it so that it works as it worked before (so showon local value as opposed to showon global value).

I will do a PR in a minute that will fix this and give developers the possibility with a minimal change to configure their list field to use the local value again.

So what this PR does is add an attribute to the field: showonlocal="true"

when this field is set, the showon will act on the actual value of the field and not on the global value of the field.

I hope that you (@LadySolveig ) can have a look at that PR and bring it to the attention on of the (dev) community.

avatar LadySolveig LadySolveig - change - 21 Nov 2024
Status New Closed
Closed_Date 0000-00-00 00:00:00 2024-11-21 14:18:34
Closed_By LadySolveig
avatar LadySolveig LadySolveig - close - 21 Nov 2024
avatar LadySolveig
LadySolveig - comment - 21 Nov 2024

Closed as there is a PR #44495

Add a Comment

Login with GitHub to post a comment