New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New Feature: Implements a modal to edit the Redirect plugin settings when needed #16844

Merged
merged 12 commits into from Jul 26, 2017

Conversation

@roland-d
Contributor

roland-d commented Jun 24, 2017

Pull Request for Issue #7259

Summary of Changes

This is a redo of the original done at #7259 with some small code changes

Testing Instructions

  1. Make sure that the Redirect plugin is either disabled or the Collection of URLs is disabled
  2. Go to administrator/index.php?option=com_redirect
  3. When the collection of URLs is disabled you will see this message:
    image

When the plugin is disabled you will see this message:
image

  1. Click on the Redirect System Plugin link and you are taken to a new page where you can edit the plugin settings. Now you need to navigate back to the Redirect plugin.
  2. Apply patch
  3. When the collection of URLs is disabled you will see this message:
    image

When the plugin is disabled you will see this message:
image

I changed the regular link to a button so it is easier to see where to click.
7. Click on the Redirect System Plugin link and you are shown a modal in which you can change the settings. Update the settings and click Save.
8. The page is refreshed and the message has changed.

Expected result

I stay on the same page and can edit the settings.

Actual result

I am taking to a new page and need to navigate back.

Documentation Changes Required

None

Pinging @brianteeman @franz-wohlkoenig for testing :)

@franz-wohlkoenig

This comment has been minimized.

Show comment
Hide comment
@franz-wohlkoenig

franz-wohlkoenig Jun 24, 2017

can you please change Color on hover for easier reading?
bildschirmfoto 2017-06-24 um 10 43 13

franz-wohlkoenig commented Jun 24, 2017

can you please change Color on hover for easier reading?
bildschirmfoto 2017-06-24 um 10 43 13

@roland-d

This comment has been minimized.

Show comment
Hide comment
@roland-d

roland-d Jun 24, 2017

Contributor

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

Contributor

roland-d commented Jun 24, 2017

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

@franz-wohlkoenig

This comment has been minimized.

Show comment
Hide comment
@franz-wohlkoenig

franz-wohlkoenig Jun 24, 2017

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

franz-wohlkoenig commented Jun 24, 2017

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

@franz-wohlkoenig

This comment has been minimized.

Show comment
Hide comment
@franz-wohlkoenig

franz-wohlkoenig Jun 24, 2017

I have tested this item successfully on 4a8b5f8


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

franz-wohlkoenig commented Jun 24, 2017

I have tested this item successfully on 4a8b5f8


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

@Quy

This comment has been minimized.

Show comment
Hide comment
@Quy

Quy Jun 24, 2017

Contributor

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

Contributor

Quy commented Jun 24, 2017

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

@brianteeman

This comment has been minimized.

Show comment
Hide comment
@brianteeman

brianteeman Jun 24, 2017

Contributor

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


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

Contributor

brianteeman commented Jun 24, 2017

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


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

Fixed CSS class and copyright
Changed reload of parent page
@roland-d

This comment has been minimized.

Show comment
Hide comment
@roland-d

roland-d Jun 24, 2017

Contributor

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
Contributor

roland-d commented Jun 24, 2017

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
@franz-wohlkoenig

This comment has been minimized.

Show comment
Hide comment
@franz-wohlkoenig

franz-wohlkoenig Jun 24, 2017

I have tested this item successfully on cb243fc

But Link is difficult to read:
bildschirmfoto 2017-06-24 um 17 05 59


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

franz-wohlkoenig commented Jun 24, 2017

I have tested this item successfully on cb243fc

But Link is difficult to read:
bildschirmfoto 2017-06-24 um 17 05 59


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16844.
@roland-d

This comment has been minimized.

Show comment
Hide comment
@roland-d

roland-d Jun 24, 2017

Contributor

@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?

Contributor

roland-d commented Jun 24, 2017

@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?

@mbabker

This comment has been minimized.

Show comment
Hide comment
@mbabker

mbabker Jun 24, 2017

Member

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".

Member

mbabker commented Jun 24, 2017

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".

@roland-d

This comment has been minimized.

Show comment
Hide comment
@roland-d

roland-d Jun 24, 2017

Contributor

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

Contributor

roland-d commented Jun 24, 2017

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

@franz-wohlkoenig

This comment has been minimized.

Show comment
Hide comment
@franz-wohlkoenig

franz-wohlkoenig Jun 24, 2017

I have tested this item successfully on 6f1d7aa


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

franz-wohlkoenig commented Jun 24, 2017

I have tested this item successfully on 6f1d7aa


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

@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Jun 24, 2017

Member

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.

Member

infograf768 commented Jun 24, 2017

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.

@roland-d

This comment has been minimized.

Show comment
Hide comment
@roland-d

roland-d Jun 24, 2017

Contributor

@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.

Contributor

roland-d commented Jun 24, 2017

@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.

@dgrammatiko

This comment has been minimized.

Show comment
Hide comment
@dgrammatiko

dgrammatiko Jun 24, 2017

Contributor

@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)

Contributor

dgrammatiko commented Jun 24, 2017

@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)

else
{
$link = JRoute::_('index.php?option=com_plugins&task=plugin.edit&extension_id=' . RedirectHelper::getRedirectPluginId());
$app->enqueueMessage(JText::sprintf('COM_REDIRECT_PLUGIN_DISABLED', $link), 'error');
$this->redirectPluginId = RedirectHelper::getRedirectPluginId();

This comment has been minimized.

@andrepereiradasilva

andrepereiradasilva Jun 25, 2017

Contributor

since you already added the id toJPluginHelper db query return data, can't you just use JPluginHelper::getPlugin('system', 'redirect')->id and remove/deprecate RedirectHelper::getRedirectPluginId?

@andrepereiradasilva

andrepereiradasilva Jun 25, 2017

Contributor

since you already added the id toJPluginHelper db query return data, can't you just use JPluginHelper::getPlugin('system', 'redirect')->id and remove/deprecate RedirectHelper::getRedirectPluginId?

This comment has been minimized.

@roland-d

roland-d Jun 25, 2017

Contributor

You cannot do that because when the plugin is disabled the function will return an empty array.

@roland-d

roland-d Jun 25, 2017

Contributor

You cannot do that because when the plugin is disabled the function will return an empty array.

This comment has been minimized.

@andrepereiradasilva

andrepereiradasilva Jun 25, 2017

Contributor

Right! Forget that

@andrepereiradasilva

andrepereiradasilva Jun 25, 2017

Contributor

Right! Forget that

Show outdated Hide outdated administrator/components/com_redirect/views/links/tmpl/default.php
@franz-wohlkoenig

This comment has been minimized.

Show comment
Hide comment
@franz-wohlkoenig

franz-wohlkoenig Jun 25, 2017

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

franz-wohlkoenig commented Jun 25, 2017

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

@roland-d

This comment has been minimized.

Show comment
Hide comment
@roland-d

roland-d Jun 25, 2017

Contributor

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

Contributor

roland-d commented Jun 25, 2017

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

@roland-d

This comment has been minimized.

Show comment
Hide comment
@roland-d

roland-d Jun 25, 2017

Contributor

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

Contributor

roland-d commented Jun 25, 2017

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

@franz-wohlkoenig

This comment has been minimized.

Show comment
Hide comment
@franz-wohlkoenig

franz-wohlkoenig Jun 25, 2017

I have tested this item successfully on 6f5fc46


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

franz-wohlkoenig commented Jun 25, 2017

I have tested this item successfully on 6f5fc46


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

@roland-d

This comment has been minimized.

Show comment
Hide comment
@roland-d

roland-d Jun 26, 2017

Contributor

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

Contributor

roland-d commented Jun 26, 2017

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

@Quy

This comment has been minimized.

Show comment
Hide comment
@Quy

Quy Jun 26, 2017

Contributor

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>
Contributor

Quy commented Jun 26, 2017

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>
@roland-d

This comment has been minimized.

Show comment
Hide comment
@roland-d

roland-d Jun 26, 2017

Contributor

@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.

Contributor

roland-d commented Jun 26, 2017

@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.

@zero-24

This comment has been minimized.

Show comment
Hide comment
@zero-24

zero-24 Jun 26, 2017

Contributor

Thanks

Contributor

zero-24 commented Jun 26, 2017

Thanks

@zero-24

This comment has been minimized.

Show comment
Hide comment
@zero-24

zero-24 Jun 26, 2017

Contributor

Sorry broken autocorrect

Contributor

zero-24 commented Jun 26, 2017

Sorry broken autocorrect

@roland-d

This comment has been minimized.

Show comment
Hide comment
@roland-d

roland-d Jun 29, 2017

Contributor

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

Contributor

roland-d commented Jun 29, 2017

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

@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Jun 30, 2017

Member

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

Member

infograf768 commented Jun 30, 2017

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

@roland-d

This comment has been minimized.

Show comment
Hide comment
@roland-d

roland-d Jul 3, 2017

Contributor

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

Contributor

roland-d commented Jul 3, 2017

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

@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Jul 3, 2017

Member

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'

Member

infograf768 commented Jul 3, 2017

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'

@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Jul 3, 2017

Member

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 :)

screen shot 2017-07-03 at 12 07 04

Member

infograf768 commented Jul 3, 2017

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 :)

screen shot 2017-07-03 at 12 07 04

@roland-d

This comment has been minimized.

Show comment
Hide comment
@roland-d

roland-d Jul 4, 2017

Contributor

@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.

Contributor

roland-d commented Jul 4, 2017

@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.

@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Jul 4, 2017

Member

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

Member

infograf768 commented Jul 4, 2017

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

@roland-d

This comment has been minimized.

Show comment
Hide comment
@roland-d

roland-d Jul 4, 2017

Contributor

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

Contributor

roland-d commented Jul 4, 2017

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

@Quy

This comment has been minimized.

Show comment
Hide comment
@Quy

Quy Jul 5, 2017

Contributor

The sentence is repeated.

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

Contributor

Quy commented Jul 5, 2017

The sentence is repeated.

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

@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Jul 5, 2017

Member

@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>
Member

infograf768 commented Jul 5, 2017

@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>
@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Jul 5, 2017

Member

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

Member

infograf768 commented Jul 5, 2017

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

@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Jul 5, 2017

Member

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.

Member

infograf768 commented Jul 5, 2017

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.

roland-d added a commit to roland-d/joomla-cms that referenced this pull request Jul 5, 2017

@roland-d

This comment has been minimized.

Show comment
Hide comment
@roland-d

roland-d Jul 5, 2017

Contributor

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.

Contributor

roland-d commented Jul 5, 2017

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.

@franz-wohlkoenig

This comment has been minimized.

Show comment
Hide comment
@franz-wohlkoenig

franz-wohlkoenig Jul 5, 2017

I have tested this item successfully on fc25aa7

Tested Hathor, Isis; English, Persian.


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

franz-wohlkoenig commented Jul 5, 2017

I have tested this item successfully on fc25aa7

Tested Hathor, Isis; English, Persian.


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

@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Jul 6, 2017

Member

I have tested this item successfully on fc25aa7


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

Member

infograf768 commented Jul 6, 2017

I have tested this item successfully on fc25aa7


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

@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Jul 6, 2017

Member

RTC. Thanks!


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

Member

infograf768 commented Jul 6, 2017

RTC. Thanks!


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

@joomla-cms-bot joomla-cms-bot added the RTC label Jul 6, 2017

@infograf768 infograf768 changed the title from Implements a modal to edit the Redirect plugin settings when needed to New Feature: Implements a modal to edit the Redirect plugin settings when needed Jul 6, 2017

@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Jul 6, 2017

Member

@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.

Member

infograf768 commented Jul 6, 2017

@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.

@brianteeman

This comment has been minimized.

Show comment
Hide comment
@brianteeman

brianteeman Jul 6, 2017

Contributor

As its a new feature it should be 3.8 surely

Contributor

brianteeman commented Jul 6, 2017

As its a new feature it should be 3.8 surely

@rdeutz rdeutz added this to the Joomla 3.8.0 milestone Jul 6, 2017

@rdeutz rdeutz removed their assignment Jul 6, 2017

@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Jul 6, 2017

Member

@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.

Member

infograf768 commented Jul 6, 2017

@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.

@roland-d

This comment has been minimized.

Show comment
Hide comment
@roland-d

roland-d Jul 6, 2017

Contributor

@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.

Contributor

roland-d commented Jul 6, 2017

@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.

@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Jul 6, 2017

Member

it IS part of core ;)

Member

infograf768 commented Jul 6, 2017

it IS part of core ;)

@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Jul 6, 2017

Member

Looks like it solved itself, weird.

Member

infograf768 commented Jul 6, 2017

Looks like it solved itself, weird.

@roland-d

This comment has been minimized.

Show comment
Hide comment
@roland-d

roland-d Jul 6, 2017

Contributor

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

Contributor

roland-d commented Jul 6, 2017

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

@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Jul 6, 2017

Member

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

Member

infograf768 commented Jul 6, 2017

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

@mbabker

This comment has been minimized.

Show comment
Hide comment
@mbabker

mbabker Jul 25, 2017

Member

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).

Member

mbabker commented Jul 25, 2017

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).

Merge branch 'staging' into redo-7259-modal-for-quicker-edit
* staging: (274 commits)
  Add JCryptCipherSodium to support libsodium (#16754)
  Performance 2 (libraries/legacy) (#12220)
  Performance 6 (templates) (#12233)
  Fixed typehint (#16425)
  Fix for: Repeatable field is no longer rendered with Chosen layout (#16471)
  Fix the path for the ajax-loader.gif (#16701)
  Menu items list parent filter (#17060)
  Text Filters layout (#17113)
  mod_login showon option (#17153)
  com_banners incorret tooltip (#17157)
  fix joomla.content.options_default (#17123)
  remove the never working limitstart call (#17184)
  Update phpDocumentor build
  set 3.8.0 Dev State
  Prepare 3.7.4 Stable Release
  fixed a logic change in #12294, thanks @Hoffi1
  Update sv-SE.ini
  Update pt-BR.ini
  Update lv-LV.ini
  Update fa-IR.ini
  ...
@roland-d

This comment has been minimized.

Show comment
Hide comment
@roland-d

roland-d Jul 26, 2017

Contributor

@mbabker Done as requested

Contributor

roland-d commented Jul 26, 2017

@mbabker Done as requested

@mbabker mbabker merged commit bc1e0a0 into joomla:staging Jul 26, 2017

3 of 4 checks passed

continuous-integration/drone/pr this build is pending
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
hound No violations found. Woof!

@joomla-cms-bot joomla-cms-bot removed the RTC label Jul 26, 2017

@brianteeman

This comment has been minimized.

Show comment
Hide comment
@brianteeman

brianteeman Jul 26, 2017

Contributor

thank you

Contributor

brianteeman commented Jul 26, 2017

thank you

@roland-d roland-d deleted the roland-d:redo-7259-modal-for-quicker-edit branch Jul 28, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment