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

roland-d · 2017-06-24T08:35:59Z

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

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

When the plugin is disabled you will see this message:

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.
Apply patch
When the collection of URLs is disabled you will see this message:

When the plugin is disabled you will see this message:

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

…n settings when needed

ghost · 2017-06-24T08:44:04Z

can you please change Color on hover for easier reading?

roland-d · 2017-06-24T08:45:22Z

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

ghost · 2017-06-24T08:46:48Z

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

ghost · 2017-06-24T08:47:18Z

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

brianteeman · 2017-06-24T08:52:58Z

administrator/components/com_plugins/views/plugin/tmpl/modal.php

+ * @package     Joomla.Administrator
+ * @subpackage  com_plugins
+ *
+ * @copyright   Copyright (C) 2005 - 2015 Open Source Matters, Inc. All rights reserved.


Quy · 2017-06-24T12:46:05Z

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

brianteeman · 2017-06-24T13:46:51Z

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

Changed reload of parent page

roland-d · 2017-06-24T14:53:18Z

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

ghost · 2017-06-24T15:06:46Z

I have tested this item ✅ successfully on cb243fc

But Link is difficult to read:

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

roland-d · 2017-06-24T15:31:42Z

@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 · 2017-06-24T15:40:16Z

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 · 2017-06-24T16:19:32Z

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

ghost · 2017-06-24T16:24:20Z

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 · 2017-06-24T16:43:31Z

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 · 2017-06-24T16:50:30Z

@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 · 2017-06-24T16:53:47Z

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

andrepereiradasilva · 2017-06-25T02:36:27Z

administrator/components/com_redirect/views/links/view.html.php

 		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();


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

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

Right! Forget that

andrepereiradasilva · 2017-06-25T02:38:53Z

administrator/components/com_redirect/views/links/tmpl/default.php

+		<?php
+			if ($this->redirectPluginId)
+			{
+				$link = JRoute::_('index.php?option=com_plugins&amp;client_id=0&amp;task=plugin.edit&amp;extension_id=' . $this->redirectPluginId . '&amp;tmpl=component&amp;layout=modal');


why use & in JRoute::_()?

No reason, just copy and paste :)

ghost · 2017-06-25T07:25:59Z

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

roland-d · 2017-06-25T07:28:38Z

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

roland-d · 2017-06-25T09:40:10Z

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

ghost · 2017-06-25T09:44:31Z

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

Quy · 2017-06-26T13:12:17Z

administrator/components/com_redirect/views/links/view.html.php

+
+			if ($this->enabled && !$this->collect_urls_enabled)
+			{
+				$app->enqueueMessage(JText::_('COM_REDIRECT_PLUGIN_ENABLED') . JText::sprintf('COM_REDIRECT_COLLECT_URLS_DISABLED', $link), 'notice');


Add a space between sentences.

infograf768 · 2017-07-05T06:08:50Z

@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 · 2017-07-05T06:33:45Z

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 · 2017-07-05T06:49:13Z

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 · 2017-07-05T18:31:19Z

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.

ghost · 2017-07-05T18:46:45Z

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 · 2017-07-06T06:18:46Z

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 · 2017-07-06T06:19:32Z

RTC. Thanks!

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

infograf768 · 2017-07-06T06:22:31Z

@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 · 2017-07-06T06:24:51Z

As its a new feature it should be 3.8 surely

infograf768 · 2017-07-06T06:44:05Z

@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 · 2017-07-06T07:16:56Z

@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 · 2017-07-06T09:16:30Z

it IS part of core ;)

infograf768 · 2017-07-06T09:23:44Z

Looks like it solved itself, weird.

roland-d · 2017-07-06T09:25:21Z

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

infograf768 · 2017-07-06T09:26:49Z

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

mbabker · 2017-07-25T23:37:17Z

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

@Hoffi1

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

roland-d · 2017-07-26T06:34:59Z

@mbabker Done as requested

brianteeman · 2017-07-26T11:52:58Z

thank you

Redo of joomla#7259 and implements a modal to edit the Redirect plugi…

4a8b5f8

…n settings when needed

joomla-cms-bot added Language Change This is for Translators PR-staging labels Jun 24, 2017

roland-d mentioned this pull request Jun 24, 2017

new feature: plugin modal view needed for quicker edit #7259

Closed

brianteeman reviewed Jun 24, 2017

View reviewed changes

Fixed CSS class and copyright

cb243fc

Changed reload of parent page

Removed label class

6f1d7aa

Make location generic

df1de20

andrepereiradasilva reviewed Jun 25, 2017

View reviewed changes

Changed amp usage

6f5fc46

Quy reviewed Jun 26, 2017

View reviewed changes

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

RTL support in Hathor joomla#16844

fc25aa7

joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jul 6, 2017

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

infograf768 assigned rdeutz Jul 6, 2017

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

rdeutz removed their assignment Jul 6, 2017

RTL support in Hathor joomla#16844 Redo

34dc757

roland-d force-pushed the redo-7259-modal-for-quicker-edit branch from fc25aa7 to 34dc757 Compare July 6, 2017 07:24

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

joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jul 26, 2017

roland-d deleted the redo-7259-modal-for-quicker-edit branch July 28, 2017 06:26

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

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

Conversation

roland-d commented Jun 24, 2017 • edited by infograf768

Summary of Changes

Testing Instructions

Expected result

Actual result

Documentation Changes Required

ghost commented Jun 24, 2017

roland-d commented Jun 24, 2017

ghost commented Jun 24, 2017

ghost commented Jun 24, 2017

brianteeman Jun 24, 2017

Choose a reason for hiding this comment

Quy commented Jun 24, 2017

brianteeman commented Jun 24, 2017

roland-d commented Jun 24, 2017

ghost commented Jun 24, 2017 • edited by ghost

roland-d commented Jun 24, 2017

mbabker commented Jun 24, 2017

roland-d commented Jun 24, 2017

ghost commented Jun 24, 2017

infograf768 commented Jun 24, 2017

roland-d commented Jun 24, 2017

dgrammatiko commented Jun 24, 2017

andrepereiradasilva Jun 25, 2017 • edited

Choose a reason for hiding this comment

roland-d Jun 25, 2017

Choose a reason for hiding this comment

andrepereiradasilva Jun 25, 2017

Choose a reason for hiding this comment

andrepereiradasilva Jun 25, 2017

Choose a reason for hiding this comment

roland-d Jun 25, 2017

Choose a reason for hiding this comment

ghost commented Jun 25, 2017

roland-d commented Jun 25, 2017

roland-d commented Jun 25, 2017

ghost commented Jun 25, 2017

Quy Jun 26, 2017

Choose a reason for hiding this comment

infograf768 commented Jul 5, 2017

infograf768 commented Jul 5, 2017

infograf768 commented Jul 5, 2017

roland-d commented Jul 5, 2017

ghost commented Jul 5, 2017

infograf768 commented Jul 6, 2017

infograf768 commented Jul 6, 2017

infograf768 commented Jul 6, 2017

brianteeman commented Jul 6, 2017

infograf768 commented Jul 6, 2017

roland-d commented Jul 6, 2017

infograf768 commented Jul 6, 2017

infograf768 commented Jul 6, 2017

roland-d commented Jul 6, 2017

infograf768 commented Jul 6, 2017 • edited

mbabker commented Jul 25, 2017

roland-d commented Jul 26, 2017

brianteeman commented Jul 26, 2017

roland-d commented Jun 24, 2017 •

edited by infograf768

ghost commented Jun 24, 2017 •

edited by ghost

andrepereiradasilva Jun 25, 2017 •

edited

infograf768 commented Jul 6, 2017 •

edited