@brianteeman Shouldn't the COM_REDIRECT_PLUGIN_MODAL_DISABLED message be shown in any case if the plugin is disabled, regardless of collectUrlsEnabled?

If so, code could be changed and simplified from

if (!$pluginEnabled && !$collectUrlsEnabled)
{
	$this->msg = Text::sprintf('COM_REDIRECT_PLUGIN_MODAL_DISABLED', $link);
}
if ($pluginEnabled && !$collectUrlsEnabled)
{
	$this->msg = Text::sprintf('COM_REDIRECT_COLLECT_MODAL_URLS_DISABLED', Text::_('COM_REDIRECT_PLUGIN_ENABLED'), $link);
}

to

if (!$pluginEnabled)
{
	$this->msg = Text::sprintf('COM_REDIRECT_PLUGIN_MODAL_DISABLED', $link);
}
elseif (!$collectUrlsEnabled)
{
	$this->msg = Text::sprintf('COM_REDIRECT_COLLECT_MODAL_URLS_DISABLED', Text::_('COM_REDIRECT_PLUGIN_ENABLED'), $link);
}

it should be

• if plugin disabled
• if plugin enabled and collect disabled

it should be

* if plugin disabled

* if plugin enabled and collect disabled

@brianteeman Yes, but like you have it now it is

• if plugin disabled and collect disabled
• if plugin enabled and collect disabled

If you use 2 if or one if and else is then a matter of taste.

But the first if condition if (!$pluginEnabled && !$collectUrlsEnabled) for showing the COM_REDIRECT_PLUGIN_MODAL_DISABLEDtext needs to be corrected to if (!$pluginEnabled).

@brianteeman Code review looks good to me. Is it still a draft, or shall we start testing?

you can test but I really want to k ow from @HLeithner if this is a valid approach
and still need some help with js part

Ah, yes, I forgot about the js part. Sorry I can't help with that.

Richard’s proposed code is better in that when the first check is true then there is no need to perform the second check.

Well, my code maybe might save a few atoseconds, but maybe Brian's code is easier to understand when reading, so I am ok with it.

There is way too much logic in the template, that should all be done in the model and view in my opinion and only a "if $showwarning then echo" should be in the template...

I thought that would be the case. Any help greatly appreciated

I will see what I can do tomorrow

There is way too much logic in the template, that should all be done in the model and view in my opinion and only a "if $showwarning then echo" should be in the template...

That is exactly what I suggested in #27380 (comment)

1. Create a new method in the model
2. Set the variable obtained in the display method in HtmlView
3. And only the div and echo in default.php if variable is not null

PHP code review looks good to me, and I've just tested it with success. Now only JS part is missing. Maybe this could also update the redirect view after having enabled the plugin, so the displayed messages adapt accordingly.

P.S.: Maybe just force a page reload after having enabled the plugin?

Now added the same for com_finder with the added bonus that the plugin settings are now changed in a modal and not by redirevting to com_plugins

@Fedik any help on the javascript?

@brianteeman I have made pull request, please check

Note, I would prefer inline editing, without modal. Modal always hard on mobiles.

Like:
Open plugin form => save&close => redirect back to com_finder,
In theory that should be possible with append of &return=base64-url-to-return, and maybe with some editing of the plugin form layout .

@Fedik maybe i did something wrong or maybe i explained the problem wrong

with content-smart search plugin disabled you get the message to enable the plugin
you click on the plugin and enable it and save and close

at this point I was expecting the page to refresh so that the message is no longer displayed

Did you get the page reload after "save and close"? (even if message still there)

After JS changes, it should be like you said (not forget npm , to update JS).
One thing I am afraid, it can show cached page, by browser.

Did you get the page reload after "save and close"? (even if message still there)

No

Closing as we no longer have popup alerts