? Pending

User tests: Successful: Unsuccessful:

avatar PhilETaylor
PhilETaylor
9 Nov 2018

Closes #22971
Related #22981

This PR fixes the double alert and colours for the JS alerts when there is a Joomla extension or Joomla Update available.

// paging @infograf768

avatar PhilETaylor PhilETaylor - open - 9 Nov 2018
avatar PhilETaylor PhilETaylor - change - 9 Nov 2018
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 9 Nov 2018
Category JavaScript Repository Front End Plugins
avatar PhilETaylor PhilETaylor - change - 9 Nov 2018
Labels Added: ?
avatar PhilETaylor PhilETaylor - change - 9 Nov 2018
The description was changed
avatar PhilETaylor PhilETaylor - edited - 9 Nov 2018
avatar infograf768
infograf768 - comment - 10 Nov 2018

Tested.
I have an issue with the limited variants used by Joomla.renderMessages

I explain.
With this patch I get what you want, i.e. a red background for error

screen shot 2018-11-10 at 07 10 17

BUT informing the user that there are updates is not imho an "Error"

So I modified the patch to use info and I get.

screen shot 2018-11-10 at 07 15 41

If you think that "Info" is not important enough , it's rather easy to add the "warning" type to get:

screen shot 2018-11-10 at 07 28 19

What do you think?
(I close now my related PR)

EDIT: even if it is decided to not use warning for these specific quickicons, I suggest we add this possibility to Joomla.renderMessages

avatar infograf768
infograf768 - comment - 10 Nov 2018

Forgot to say that the css for warning does exist already but was forgotten from renderMessages.

joomla-alert[type="warning"] {
  color: #7d5a29;
  background-color: #fcefdc;
  border-color: #fbe8cd; }
  joomla-alert[type="warning"] hr {
    border-top-color: #f9ddb5; }
  joomla-alert[type="warning"] .alert-link {
    color: #573e1c; }

Therefore this PR could be modified this way

diff --git a/build/media_src/plg_quickicon_extensionupdate/js/extensionupdatecheck.es6.js b/build/media_src/plg_quickicon_extensionupdate/js/extensionupdatecheck.es6.js
index c300c60..7c61583 100644
--- a/build/media_src/plg_quickicon_extensionupdate/js/extensionupdatecheck.es6.js
+++ b/build/media_src/plg_quickicon_extensionupdate/js/extensionupdatecheck.es6.js
@@ -31,8 +31,7 @@
             } else {
               const messages = {
-                message: [
-                  `${Joomla.JText._('PLG_QUICKICON_EXTENSIONUPDATE_UPDATEFOUND_MESSAGE').replace('%s', `<span class="badge badge-pill badge-danger">${updateInfoList.length}</span>`)}<button class="btn btn-primary" onclick="document.location='${options.url}'">${Joomla.JText._('PLG_QUICKICON_EXTENSIONUPDATE_UPDATEFOUND_BUTTON')}</button>`,
+                warning: [
+                  `${Joomla.JText._('PLG_QUICKICON_EXTENSIONUPDATE_UPDATEFOUND_MESSAGE').replace('%s', `<span class="badge badge-pill badge-danger">${updateInfoList.length}</span>`)}<button class="btn btn-sm btn-primary" onclick="document.location='${options.url}'">${Joomla.JText._('PLG_QUICKICON_EXTENSIONUPDATE_UPDATEFOUND_BUTTON')}</button>`,
                 ],
-                error: ['info'],
               };
 
diff --git a/build/media_src/plg_quickicon_joomlaupdate/js/jupdatecheck.es6.js b/build/media_src/plg_quickicon_joomlaupdate/js/jupdatecheck.es6.js
index 7c63fee..b408c7a 100644
--- a/build/media_src/plg_quickicon_joomlaupdate/js/jupdatecheck.es6.js
+++ b/build/media_src/plg_quickicon_joomlaupdate/js/jupdatecheck.es6.js
@@ -34,10 +34,9 @@
             if (updateInfo.version !== options.version) {
               const messages = {
-                message: [
+                warning: [
                   `${Joomla.JText._('PLG_QUICKICON_JOOMLAUPDATE_UPDATEFOUND_MESSAGE').replace('%s', `<span class="badge badge-danger">${updateInfoList.length}</span>`)}`
                   + `<button class="btn btn-primary" onclick="document.location='${options.url}'">`
                   + `${Joomla.JText._('PLG_QUICKICON_JOOMLAUPDATE_UPDATEFOUND_BUTTON')}</button>`,
                 ],
-                error: ['info'],
               };
 
diff --git a/build/media_src/system/js/core.es6.js b/build/media_src/system/js/core.es6.js
index 0e013b5..7e4fe4d 100644
--- a/build/media_src/system/js/core.es6.js
+++ b/build/media_src/system/js/core.es6.js
@@ -395,6 +395,10 @@
    *          Example:
    *          const messages = {
-   *              "message": ["Message one", "Message two"],
-   *              "error": ["Error one", "Error two"]
+   *              "message": ["This will be a green message", "So will this"],
+   *              "error": ["This will be a red message", "So will this"],
+   *              "info": ["This will be a blue message", "So will this"],
+   *              "warning": ["This will be an orange message", "So will this"],
+   *              "notice": ["This will be same as info message", "So will this"],
+   *              "my_custom_type": ["This will be same as info message", "So will this"]
    *          };
    * @param  {string} selector The selector of the container where the message will be rendered
@@ -429,8 +433,9 @@
         messagesBox = document.createElement('joomla-alert');
 
-        if (['notice', 'message', 'error'].indexOf(type) > -1) {
+        if (['notice', 'message', 'error', 'warning'].indexOf(type) > -1) {
           alertClass = (type === 'notice') ? 'info' : type;
           alertClass = (type === 'message') ? 'success' : alertClass;
           alertClass = (type === 'error') ? 'danger' : alertClass;
+          alertClass = (type === 'warning') ? 'warning' : alertClass;
         } else {
           alertClass = 'info';
@@ -448,8 +453,9 @@
 
         // Message class
-        if (['notice', 'message', 'error'].indexOf(type) > -1) {
+        if (['notice', 'message', 'error', 'warning'].indexOf(type) > -1) {
           alertClass = (type === 'notice') ? 'info' : type;
           alertClass = (type === 'message') ? 'success' : alertClass;
           alertClass = (type === 'error') ? 'danger' : alertClass;
+          alertClass = (type === 'warning') ? 'warning' : alertClass;
         } else {
           alertClass = 'info';
diff --git a/plugins/quickicon/extensionupdate/extensionupdate.php b/plugins/quickicon/extensionupdate/extensionupdate.php
index 3e8e2f9..d8e85c8 100644
--- a/plugins/quickicon/extensionupdate/extensionupdate.php
+++ b/plugins/quickicon/extensionupdate/extensionupdate.php
@@ -71,4 +71,8 @@
 		Text::script('PLG_QUICKICON_EXTENSIONUPDATE_UPDATEFOUND_BUTTON', true);
 		Text::script('PLG_QUICKICON_EXTENSIONUPDATE_ERROR', true);
+		Text::script('MESSAGE', true);
+		Text::script('ERROR', true);
+		Text::script('INFO', true);
+		Text::script('WARNING', true);
 
 		HTMLHelper::_('behavior.core');
diff --git a/plugins/quickicon/joomlaupdate/joomlaupdate.php b/plugins/quickicon/joomlaupdate/joomlaupdate.php
index 3e39ef5..0b2de8b 100644
--- a/plugins/quickicon/joomlaupdate/joomlaupdate.php
+++ b/plugins/quickicon/joomlaupdate/joomlaupdate.php
@@ -81,4 +81,8 @@
 		Text::script('PLG_QUICKICON_JOOMLAUPDATE_UPDATEFOUND', true);
 		Text::script('PLG_QUICKICON_JOOMLAUPDATE_UPTODATE', true);
+		Text::script('MESSAGE', true);
+		Text::script('ERROR', true);
+		Text::script('INFO', true);
+		Text::script('WARNING', true);
 
 		$this->app->getDocument()->addScriptOptions(
avatar infograf768
infograf768 - comment - 11 Nov 2018

@PhilETaylor
Can you modify your PR with my proposal (Using warning) ?

avatar PhilETaylor
PhilETaylor - comment - 12 Nov 2018

The css for joomla-alert[type="warning"] is only used by custom elements and not Joomla.renderMessages (which uses markup like <span class="badge badge-danger">) as far as my previous tests showed - maybe Im wrong?

I had done all this work for warning but removed it in an attempt to actually get the fix PR approved and merged.

avatar infograf768
infograf768 - comment - 12 Nov 2018

I think you are wrong, @PhilETaylor
My code above works perfectly.
the key word is type="warning" here.

screen shot 2018-11-12 at 07 25 07

We have indeed a <span class="badge badge-pill badge-danger">1</span> but it only concerns the figure in the alert, not the background color of the message.
Code is
'<span class="badge badge-pill badge-danger">' + updateInfoList.length + '</span>')

avatar PhilETaylor
PhilETaylor - comment - 12 Nov 2018

hmmm looks like the span is inside the custom element - ok, well it never worked when I tried last time - I'll try again as soon as I can.

avatar infograf768
infograf768 - comment - 12 Nov 2018

The code is just above ?

avatar infograf768
infograf768 - comment - 13 Nov 2018

@PhilETaylor
Can you modify now so that we can merge as I have an other change to do in the joomlaupdate js ?

avatar PhilETaylor
PhilETaylor - comment - 13 Nov 2018

today is my first day in the office - I'll take a look today

avatar PhilETaylor
PhilETaylor - comment - 13 Nov 2018

@infograf768 Done.

avatar PhilETaylor
PhilETaylor - comment - 13 Nov 2018

@infograf768 approved this pull request.

Phew :-)

avatar PhilETaylor PhilETaylor - change - 13 Nov 2018
The description was changed
avatar PhilETaylor PhilETaylor - edited - 13 Nov 2018
avatar infograf768
infograf768 - comment - 13 Nov 2018

I have tested this item successfully on 7a5a89f

@laoneo @wilsonge

Test Ok, please merge as I need to do another PR for jupdatecheck.es6.js


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

avatar infograf768 infograf768 - test_item - 13 Nov 2018 - Tested successfully
avatar laoneo laoneo - change - 13 Nov 2018
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2018-11-13 10:03:40
Closed_By laoneo
avatar laoneo laoneo - close - 13 Nov 2018
avatar laoneo laoneo - merge - 13 Nov 2018
avatar laoneo
laoneo - comment - 13 Nov 2018

Thanks

Add a Comment

Login with GitHub to post a comment