? Pending

User tests: Successful: Unsuccessful:

avatar bembelimen
bembelimen
24 Mar 2018

Summary of Changes

With the JDocument::addScriptOptions method it's possible to change script parameters like the TinyMCE plugin parameters from outside (like with a plugin). This functionality was implemented with the following PR: #3072 and in the TinyMCE here: #11157 (with examples)

The problem at the moment is, that the "new parameters" will only be merged on the first level, so for example for the TinyMCE you cannot change parameters of deeper level in an easy way.

Testing Instructions

Add the following code into your template index file:

$options = [
    'foo' => [
        'bar' => [
			'bla' => 1,
		],
		'baz' => [
			'blub' => 2
		]
    ]
];

JFactory::getDocument()->addScriptOptions('Foobar', $options);

// Update the foo => bar => bla value
$options = [
	'foo' => [
        	'bar' => [
			'bla' => 2,
		]
	]
];

JFactory::getDocument()->addScriptOptions('Foobar', $options);

Expected result

    "Foobar": {
        "foo": {
            "bar": {
                "bla": 2
            },
            "baz": {
                "blub": 2
            }
        }
    }

Actual result

    "Foobar": {
        "foo": {
            "bar": {
                "bla": 2
            }
        }
    }

More description

This example is very small, but e.g. the tinyMCE has somewhere about 50 parameters with a level of 3 or more. If you want to change one parameter in the depth, you have to rebuild the whole tree (or load the current tree, setup the variables and then set again), which makes the "merge" a bit useless.

Perhaps @Fedik and @dgt41 could look over this functionality, because they worked with it.

avatar bembelimen bembelimen - open - 24 Mar 2018
avatar bembelimen bembelimen - change - 24 Mar 2018
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 24 Mar 2018
Category Libraries
avatar dgt41
dgt41 - comment - 24 Mar 2018

@bembelimen I like it and I don't see any problems changing the behaviour. Edge case if someone used this code to actually replace the values in the storage but then again I don't think that many people are actually using or even understand the Joomla.storageOptions. Anyhows I'm happy with this change but I guess @Fedik has the final say here!

avatar bembelimen
bembelimen - comment - 24 Mar 2018

Thank you for your feedback @dgt41 if someone wants to replace, he could set the $merge parameter to "false"

avatar chmst
chmst - comment - 24 Mar 2018

I have tested this item successfully on d8b3c51

I've tested this item successfully on a 3.6.8 installation, following the testing instructions.


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

avatar chmst chmst - test_item - 24 Mar 2018 - Tested successfully
avatar Fedik
Fedik - comment - 24 Mar 2018

looks fine

the "new parameters" will only be merged on the first level

that was intent ?
to keep stuff simple, and to force developer to use: getOptions() => modify an options array => setOptions(),

if it works, then can be as @bembelimen suggested also ?

avatar Didldu-Florian Didldu-Florian - test_item - 19 Aug 2018 - Tested successfully
avatar Didldu-Florian
Didldu-Florian - comment - 19 Aug 2018

I have tested this item successfully on d8b3c51


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

avatar franz-wohlkoenig franz-wohlkoenig - change - 20 Aug 2018
Status Pending Ready to Commit
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 20 Aug 2018

Ready to Commit after two successful tests.

avatar mbabker mbabker - change - 21 Aug 2018
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2018-08-21 03:54:09
Closed_By mbabker
Labels Added: ?
avatar mbabker mbabker - close - 21 Aug 2018
avatar mbabker mbabker - merge - 21 Aug 2018

Add a Comment

Login with GitHub to post a comment