can you please change Color on hover for easier reading?

@franz-wohlkoenig I can if you tell me which class I should be using :)

i can't, i'm the one who ask;-) CSS-Gurus here?

I have tested this item ✅ successfully on 4a8b5f8

@roland-d Use the alert-link class instead of a button.

@roland-d other than the previouslt mentioned css issue its all good - thanks

Thanks everybody for your input. I have made the following changes:

• Fixed the copyright
• Changed the CSS class from btn to alert-link
• Changed the reload of the parent page because the old way showed JS errors and had issues if you filtered the list before clicking the plugin modal

I have tested this item ✅ successfully on cb243fc

But Link is difficult to read:

@franz-wohlkoenig I could remove the label class and then it is just bold text. That just doesn't look clear enough to me that it is a link you can click. @Quy Any other options for styling this so it looks more like a clickable link?

Improve the implementation of the alert-link class if you don't think it stands out as a link (which is just a backport from Bootstrap 3). Please don't try to fudge the display with extra UI elements to make it "stand out".

Ok, I removed the label class. Declaring styling out of scope :)

I have tested this item ✅ successfully on 6f1d7aa

Can't we make this generic? I mean not specific to the redirect plugin?
I am talking about the edit.php
window.top.setTimeout('window.parent.location = \"index.php?option=com_redirect&view=links\"', 1000);

We could use the modal for other instances of editing a plugin when we have such situation:
in com_associations for example for the system - languagefilter and in com_fields for system - Fields.

@infograf768 This was generic and then I made the mistake of putting that URL in there :P I will change this so it is generic.

@infograf768 in Joomla4 we have events so this can be a lot nicer and of course not generic.

Also this probably needs some attention when merging static to j4 (we don't have inline scripts anymore)

@roland-d please give a go if PR is ready for Test.

@franz-wohlkoenig Let's ask @andrepereiradasilva if he wants anymore changes. If not, we can test.

@franz-wohlkoenig Looks like we can test, @andrepereiradasilva has not raised any other issues.

I have tested this item ✅ successfully on 6f5fc46

Thanks for the feedback but I am going to wait a week to make changes in case anybody else comes up with something.

In \administrator\templates\hathor\html\com_redirect\links\default.php line 148, it is missing an argument.

				<span class="enabled"><?php echo JText::_('COM_REDIRECT_COLLECT_URLS_DISABLED'); ?></span>

@Quy requested a change in the Hathor override but what we have now is 2 messages. One is the enqueued message and the other the regular displayed message. The enqueued message contains the modal link which doesn't work in Hathor anyway. Are we leaving it like this or add a harcoded check for Hathor? A template which is going to be removed anyway.

Thanks

Sorry broken autocorrect

Can we move this forward or does it need more changes?

This will not be B/C language wise if you just modify the string values. We get a 404 with non updated language packs (Tested with fa-IR and fr-FR)
I suggest to create new strings/constants and marked the other ones as deprecated by a comment above them.

Obsolete:
; The following string is deprecated and will be removed with 4.0.
COM_REDIRECT_COLLECT_URLS_DISABLED
; The following string is deprecated and will be removed with 4.0.
COM_REDIRECT_PLUGIN_DISABLED

Possible new constants:
COM_REDIRECT_COLLECT_MODAL_URLS_DISABLED
COM_REDIRECT_PLUGIN_MODAL_DISABLED

Also I remarked an RTL issue that was also present before this patch
We should not use
$app->enqueueMessage(JText::_('COM_REDIRECT_PLUGIN_ENABLED') . ' ' . JText::sprintf('COM_REDIRECT_COLLECT_URLS_DISABLED', $link), 'notice');

As this is not RTL aware. The first string displays on the left in RTL.

I think we should rather have a sprintf with 2 variables, 1 for the link, the other for the string.
something like:
$app->enqueueMessage(JText::sprintf('COM_REDIRECT_COLLECT_URLS_DISABLED', JText::_('COM_REDIRECT_PLUGIN_ENABLED'), $link), 'notice');

COM_REDIRECT_COLLECT_URLS_DISABLED="%1$s The 'Collect URLs' option in the %2$s is disabled. Error page URLs will not be collected by this component."

@infograf768

We get a 404 with non updated language packs (Tested with fa-IR and fr-FR)

You can't get a 404 when using JText. If a language string doesn't exist it will just show the supplied string. What 404 are you talking about?

I will look into the sprintf for RTL

You can't get a 404 when using JText. If a language string doesn't exist it will just show the supplied string. What 404 are you talking about?

As you have not changed the constant, the original value for the string is
COM_REDIRECT_PLUGIN_DISABLED="The <a href="_QQ_"%s"_QQ_">Redirect System Plugin</a> is disabled. It needs to be enabled for this component to work."

Therefore, if the language pack is NOT updated, % is the $link value
and we get a 404 because <a href="_QQ_"JRoute::_('index.php?option=com_plugins&task=plugin.edit&extension_id=' . RedirectHelper::getRedirectPluginId());"_QQ_">Redirect System Plugin</a> makes no sense.

I suggest you just install French available pack for example + your PR and test.

@roland-d
If you like, I will make an animated gif

@infograf768 No need I think, I get the issue you are pointing out.

@infograf768 I believe it should be correct now, I tested this with both French and English.

In Hathor, invalid markup <a href="<a href=:

<span class="enabled">The 'Collect URLs' option in the <a href="<a href="index.php?option=com_plugins&filter_search=redirect" >Redirect System Plugin</a>">Redirect System Plugin</a> is disabled. Error page URLs will not be collected by this component.</span>

@roland-d
Hathor still broken here:

The link in the Error Message looks correct but it does not display the Modal.
I get a js error
Empty string passed to getElementById().

The link in the table footer is wrong. It should be a sprintf
line 150 to 152 should be

		<?php else : ?>
			<span class="disabled"><?php echo JText::sprintf('COM_REDIRECT_PLUGIN_DISABLED', 'index.php?option=com_plugins&filter_search=redirect'); ?></span>
		<?php endif; ?>

I don't think there should be a modal in Hathor, should there be?

We use Modals in Hathor, for example when we have associations.
But, yes, I don't think it is necessary, taking into account the fact that we are not supposed to code for hathor anymore. We just should not destroy what works ;)
The Error message should have the same link as the table footer, i.e. link to the old behavior
'index.php?option=com_plugins&filter_search=redirect'

I could get it to work in a basic way though, by adding

<?php if ($this->redirectPluginId) : ?>
			<?php $link = JRoute::_('index.php?option=com_plugins&client_id=0&task=plugin.edit&extension_id=' . $this->redirectPluginId . '&tmpl=component&layout=modal'); ?>
			<?php echo JHtml::_(
				'bootstrap.renderModal',
				'plugin' . $this->redirectPluginId . 'Modal',
				array(
					'url'        => $link,
					'title'      => JText::_('COM_REDIRECT_EDIT_PLUGIN_SETTINGS'),
					'height'     => '400px',
					'modalWidth' => '60',
					'footer'     => '<button class="btn" data-dismiss="modal" aria-hidden="true">'
						. JText::_("JLIB_HTML_BEHAVIOR_CLOSE") . '</button>'
						. '<button class="btn btn-success" data-dismiss="modal" aria-hidden="true" onclick="jQuery(\'#plugin' . $this->redirectPluginId . 'Modal iframe\').contents().find(\'#saveBtn\').click();">'
						. JText::_("JSAVE") . '</button>'
				)
			); ?>
		<?php endif; ?>

after line 53

But it does not look very good :)

@infograf768 I agree, we shouldn't destroy what is working ;) Instead of keeping the link you gave I did update the link to go directly to the edit screen as this also works in the isis template. Both links are the same now and not using a modal.

You forgot

The link in the table footer is wrong. It should be a sprintf
line 150 to 152 should be

this is used when the plugin is disabled. We need the link.

		<?php else : ?>
			<span class="disabled"><?php echo JText::sprintf('COM_REDIRECT_PLUGIN_DISABLED', 'index.php?option=com_plugins&filter_search=redirect'); ?></span>
		<?php endif; ?>

Or use as link
index.php?option=com_plugins&task=plugin.edit&extension_id=' . RedirectHelper::getRedirectPluginId()

as other wise we get this type of url:
http://mysite.com/administrator/%s

@infograf768 Thanks, updated things again and checked and checked but who knows what else I forgot :)

The sentence is repeated.

The Redirect Plugin is enabled. The Redirect Plugin is enabled. The option 'Collect URLs' is enabled.

@roland-d
we also have to take care of RTL.
I suggest this

	<p class="footer-tip">
		<?php if ($this->enabled && $this->collect_urls_enabled) : ?>
			<span class="enabled"><?php echo JText::sprintf('COM_REDIRECT_COLLECT_URLS_ENABLED', JText::_('COM_REDIRECT_PLUGIN_ENABLED')); ?></span>
		<?php elseif ($this->enabled && !$this->collect_urls_enabled) : ?>
			<span class="enabled"><?php echo JText::sprintf('COM_REDIRECT_COLLECT_MODAL_URLS_DISABLED', JText::_('COM_REDIRECT_PLUGIN_ENABLED'), 'index.php?option=com_plugins&task=plugin.edit&extension_id=' . RedirectHelper::getRedirectPluginId()); ?></span>
		<?php elseif (!$this->enabled) : ?>
			<span class="disabled"><?php echo JText::sprintf('COM_REDIRECT_PLUGIN_DISABLED', 'index.php?option=com_plugins&task=plugin.edit&extension_id=' . RedirectHelper::getRedirectPluginId()); ?></span>
		<?php endif; ?>
	</p>

hmm, still a small issue.
we get
The Redirect Plugin is enabled. The 'Collect URLs' option in the index.php?option=com_plugins&task=plugin.edit&extension_id=427 is disabled. Error page URLs will not be collected by this component.
coming back with corrected proposal

Here it is:

	<p class="footer-tip">
		<?php if ($this->enabled && $this->collect_urls_enabled) : ?>
			<span class="enabled"><?php echo JText::sprintf('COM_REDIRECT_COLLECT_URLS_ENABLED', JText::_('COM_REDIRECT_PLUGIN_ENABLED')); ?></span>
		<?php elseif ($this->enabled && !$this->collect_urls_enabled) : ?>
			<?php $link = JHtml::_(
					'link',
					JRoute::_('index.php?option=com_plugins&task=plugin.edit&extension_id=' . RedirectHelper::getRedirectPluginId()),
					JText::_('COM_REDIRECT_SYSTEM_PLUGIN')
				);
			?>
			<span class="enabled"><?php echo JText::sprintf('COM_REDIRECT_COLLECT_MODAL_URLS_DISABLED', JText::_('COM_REDIRECT_PLUGIN_ENABLED'), $link); ?></span>
		<?php elseif (!$this->enabled) : ?>
			<span class="disabled"><?php echo JText::sprintf('COM_REDIRECT_PLUGIN_DISABLED', 'index.php?option=com_plugins&task=plugin.edit&extension_id=' . RedirectHelper::getRedirectPluginId()); ?></span>
		<?php endif; ?>
	</p>

this should work fine now.

Thank you @infograf768 I have added the code verbatim as I don't have time to test it further at this moment but at least if it is all good others can test as well.

I have tested this item ✅ successfully on fc25aa7

Tested Hathor, Isis; English, Persian.

I have tested this item ✅ successfully on fc25aa7

RTC. Thanks!

@rdeutz
did not set milestone. I guess it can go into 3.7.4. as we can in further releases use the new modal for other places in admin.

As its a new feature it should be 3.8 surely

@roland-d
Just remarked that you are deleting in this PR
administrator/manifests/packages/pkg_weblinks.xml

I guess it is just an error. Please correct.

@infograf768 That is a mistake, for some reason this thing keeps popping up. I don't think this file is even part of the core. I will fix this.

it IS part of core ;)

Looks like it solved itself, weird.

@infograf768 Still part huh? :/ No, it didn't solve itself, I removed the previous commit and made a new commit.

Then github is weird as, normally, a new commit would take off the RTC label...
Anyhow, all is fine.

Needs to be synced with staging since the plugin helper class has now moved to its namespaced location (should just be able to git merge staging and be good, I had no issues testing another more complex PR).

@mbabker Done as requested

thank you