Simplified spaghetti code

[demis-palma]

Removed duplicated code

[rdeutz]

I would use $app->isAdmin(), because it is better readable

[izharaazmi]

I agree @rdeutz, in this case $app->isAdmin() would be more appropriate.

[demis-palma]

Hi @rdeutz I have changed getClientId() with isAdmin() as per request.

[izharaazmi]

CS: Blank line before if statement.

[izharaazmi]

(int) $this->params->get($side, 0) is used twice. Can be refactored.

[izharaazmi]

@test success

[izharaazmi]

(int) $this->params->get($side, 0) is used twice. Can be refactored as $index.

The if can be more readable if changed to

if (isset($skindirs[$index]))

or whole thing as a ternary, maybe!

$side  = $app->isAdmin() ? 'skin_admin' : 'skin';
$index = (int) $this->params->get($side, 0);
$skin  = isset($skindirs[$index]) ? basename($skindirs[$index]) : 'lightgray';
$skin  = 'skin : "' . $skin . '",';

Yours is good too. I'm just suggesting. 👍

[conconnl]

What is the status?
Can we test this, if so can we have a Test scenario?

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8544.}

[izharaazmi]

IMO, This is most of a code review.

However the current code (without this patch) would break in the case where $app is neither site nor administrator. Try loading TinyMCE editor in the installation application with error display and error reporting on.

Or you can create a third application similar to administrator with client_id = 3 also. But that is too much of work for this small test.

[krim404]

I have tested this item ✅ successfully on 3ed4624

Opened TinyMCE normally, worked w/o a problem. Added a tinymce instance in installation app manually, worked as well. Code review seems fine, too. @icampus

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8544.}

[alimpam]

I have tested this item ✅ successfully on 3ed4624

tested @icampus:
simplification does what it should

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8544.}

[roland-d]

@demis-palma Could you make the suggested changes please?