? Pending

User tests: Successful: Unsuccessful:

avatar infograf768
infograf768
13 Nov 2018

Pull Request for NOTE in Issue #23047 (comment)

Summary of Changes

Display the JVersion instead of 0 when an Update is available, both in the Message and the icon itself.
Make the Update Now button the same size as for Extensions Update message by using btn-sm

Testing Instructions

To test on 4.0-dev branch
Modify version.php line 63 to const EXTRA_VERSION = 'alpha4-dev';
Go to database and Fix database.
Then in JoomlaUpdate Options choose Custom URL and add this link (Thanks Tobias) :
https://update.joomla.org/core/nightlies/next_major_list.xml
Save Options.
Display Control Panel.

Run npm ci

After patch

screen shot 2018-11-13 at 11 27 09

@laoneo @PhilETaylor

avatar infograf768 infograf768 - open - 13 Nov 2018
avatar infograf768 infograf768 - change - 13 Nov 2018
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 13 Nov 2018
Category JavaScript Repository
avatar PhilETaylor
PhilETaylor - comment - 13 Nov 2018

This is not a moan at you at all, these moans predate this PR, but all this is a mess.

A Colon at the end of a sentence.
A massive blue floated left button
The quick icon is red (danger/error) yet the popup is warning (orange)
A comma floating after the version number, Now 3 rows of text it looks cramped.
A trailing number in the updates quick icon, in grey background.

:-)

avatar infograf768
infograf768 - comment - 13 Nov 2018

@PhilETaylor
I propose to deal with some of these aspects in a different PR.
I volunteer... ;)
This one has the only purpose to get the JVersion instead of a 0
Please test and let's get it in so we (I) can concentrate on the rest.

avatar PhilETaylor
PhilETaylor - comment - 13 Nov 2018

:-) No worries.... Look and feel (and JS) are not my big areas and I shy away from them - I just know when something looks wrong or tacky :)

avatar infograf768
infograf768 - comment - 14 Nov 2018

A comma floating after the version number, Now 3 rows of text it looks cramped.
A massive blue floated left button

That one is only due to the fact that the available version is an alpha.
Going as far as we can on the version number of figures i.e. 4.19.19, taking off the coma in the lang string and the : in the Message, adding some css gives :

screen shot 2018-11-14 at 08 44 40

NOTE as a reminder:
add :
warning: ['<div class="text-right">' + Joomla.JText._('PLG_QUICKICON_JOOMLAUPDATE_UPDATEFOUND_MESSAGE') etc.

has done the trick here for LTR languages.
==>Similar code to be used for Extensions update.

For compatibility with RTL, we need to create a new specific class to replace text-right, which would be overriden in the rtl template. For example message-alert. The Close and title of the Message are also placed wrongly and this should modified by overriding some css
For this last part we need to add in template-rtl.css

joomla-alert .joomla-alert--close, joomla-alert .joomla-alert-button--close {
    right: 0;
    left: -1.25rem;
}
joomla-alert .joomla-alert--close, joomla-alert .joomla-alert-button--close {
    float: left;
}

The quick icon is red (danger/error) yet the popup is warning (orange)

I do not really see this as a problem, but, if desired, we can use the Warning color for quickicons and get
screen shot 2018-11-14 at 08 58 54

A trailing number in the updates quick icon, in grey background.

This is also a simple change in the JS.
We should, normally, be able to modify that JS to use a plural form. I did not find the trick when we are not in php.
See https://github.com/joomla/joomla-cms/blob/4.0-dev/libraries/src/Language/Text.php#L186-L188

avatar rdeutz rdeutz - change - 14 Nov 2018
Labels Added: ?
avatar dgrammatiko
dgrammatiko - comment - 14 Nov 2018

@infograf768 what is the problem, the code seems fine?

avatar dgrammatiko
dgrammatiko - comment - 14 Nov 2018

@infograf768 hold on you cannot use the plural version of a string in the browser (js) without prior sending all the required stings to the browser!!! I don't think that Joomla has a way to deal with this atm. What you can do is send the required strings in the browser using $doc->addScriptOptions() and then write a small case logic to pick the right string, eg

switch (number) {
case 1: 
    el.innerHTML = string1;
   break;
} 
avatar infograf768
infograf768 - comment - 14 Nov 2018

not sure I made myself clear. I would like to use our specific JText:plural in the js with strings like

WHATEVER_N_ITEMS_1
WHATEVER_N_ITEMS_MORE

as the method seems to allow.

avatar dgrammatiko
dgrammatiko - comment - 14 Nov 2018

as the method seems to allow.

@infograf768 sorry I don't think that the method allows it. I don't see anywhere in that method the string to be enqueued for the javascript world. Thus you need to queue manually all the instances WHATEVER_N_ITEMS_1
WHATEVER_N_ITEMS_MORE using addScriptOption() and then do some custom logic for the replace per case.

PS the array("script"=>true) in the plural method is only for correctly escaping what needs to be escaped not to make all the related strings available in the browser...

EDIT I'm wrong, the method should push all the strings in the browser.
can you inspect the Joomla object in your browser? Are those strings there?

avatar infograf768
infograf768 - comment - 14 Nov 2018

on ipad now. will post here tomorrow what i tried to do and th error I got.

avatar dgrammatiko
dgrammatiko - comment - 14 Nov 2018

@infograf768 I was right for the part that the dev has to push all the variations to the js. So here is what I did:
in the lang file:
screenshot 2018-11-14 at 20 51 08

In the plugin:
screenshot 2018-11-14 at 20 51 01

And in the browser console:
screenshot 2018-11-14 at 20 50 47

So you still have to push each and every string you need to the js and then introduce that switch (or if/else statement) to control which of the available strings will be displayed, eg:

const a = 1;
if (a === 1) { 
alert(Joomla.Text._('PLG_QUICKICON_JOOMLAUPDATE_TEST_ITEMS_' + a))
 } else {
alert(Joomla.Text._('PLG_QUICKICON_JOOMLAUPDATE_TEST_ITEMS_MORE' ))
}

screenshot 2018-11-14 at 20 56 34

Hope this helps

avatar infograf768
infograf768 - comment - 15 Nov 2018

Thanks @dgrammatiko
Basically indeed I am trying to avoid the horrible 1 Extension Update(s) are available

Therefore we can't use your proposal because, depending on the language, the plural form may be different and quite complex, i.e for example in Russian, one would have 1, 2, MORE
PLG_QUICKICON_JOOMLAUPDATE_TEST_ITEMS_1
PLG_QUICKICON_JOOMLAUPDATE_TEST_ITEMS_2
PLG_QUICKICON_JOOMLAUPDATE_TEST_ITEMS_MORE

public static function getPluralSuffixes($count)
	{
		if ($count == 0) {
			$return = array('0');
		} else {
			$return = array(($count%10==1 && $count%100!=11 ? '1' : ($count%10>=2 && $count%10<=4 && ($count%100<10 || $count%100>=20) ? '2' : 'MORE')));
		}
		return $return;
	}

and other languages even more variations. For Scottish
0, 1, 2, FEW

public static function getPluralSuffixes($count) {
		if ($count == 0 || $count > 19) {
            $return =  array('0');
        }
        elseif($count == 1 || $count == 11) {
               $return =  array('1');
        }
        elseif($count == 2 || $count == 12) {
               $return =  array('2');
	    }
	    elseif(($count > 2 && $count < 12) || ($count > 12 && $count < 19)) {
                $return =  array('FEW');
		}
		return $return;
     }

Let me explain what I expected would work. I included Text::plural in the existing js, ie. based on the method comment:
* <script>alert(Joomla.JText._('<?php echo Text::plural("COM_PLUGINS_N_ITEMS_UNPUBLISHED", 1, array("script"=>true)); ?>'));</script>

warning: [Joomla.JText._(Text::plural('PLG_QUICKICON_EXTENSIONUPDATE_UPDATEFOUND_N_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>']

and added the necessary strings (depending on language plural forms) in the ini file.
and the usual in the plugin, i.e.
Text::script('PLG_QUICKICON_EXTENSIONUPDATE_UPDATEFOUND_N_MESSAGE', true);

Moved around the supplementary closing parenthesis and always got this error:
SyntaxError: missing ) after argument list

It would really be great to implement a true plural form for direct js. Can do?
See https://github.com/joomla/joomla-cms/blob/4.0-dev/build/media_src/system/js/core.es6.js#L191-L241

avatar dgrammatiko
dgrammatiko - comment - 15 Nov 2018

Can do?

Possible, but someone else needs to code that

avatar infograf768
infograf768 - comment - 15 Nov 2018

@Fedik
Can you take that challenge? ;)

In the mean time, after discussing with @wilsonge, I may close this PR and make the more ambitious one modifying the display of the Message CSS + RTL aware + lang changes to avoid touching at the same files multiple times.

avatar dgrammatiko
dgrammatiko - comment - 15 Nov 2018

@infograf768 you can do it, it's an easy on:

  • On the PHP part change the function so whenever the ['script' => true] is present all the possible plural strings will be transferred to the javascript. (eg a simple for loop)

  • Change the docBlock to mention that the strings are available through the Joomla.Text API (core.js) and remove that example that works only with inline scripts...

  • On the core.js add something like

Text = { // This part already exists
  // Add the following code before the last closing bracket `}`
  plural = function (name, value) {

    if (isNaN(value)) {
      return;
    }

  switch (value) {
      case 1:
        if (Joomla.Text._(name + '_1')) {
          return Joomla.Text._(name + '_1');
        } else {
          throw new Error('Plural sting: ' + name + '_1' + ' is not available, check the PHP part');
        }
      case 2:
        if (Joomla.Text._(name + '_2')) {
          return Joomla.Text._(name + '_2');
        } else if (Joomla.Text._(name + '_MORE')) {
          return Joomla.Text._(name + '_MORE');
        } else {
          throw new Error('Plural sting: ' + name + '_2' + ' is not available, check the PHP part');
        }
        // ... continue this pattern for all possible values...
    }
  }
}
  • Add some docBlock to showcase how this can be used: eg
Joomla.Text.plural('SOME_STRING_ITEMS', 1) 
// Will output the value of the string SOME_STRING_ITEMS_1
Joomla.Text.plural('SOME_STRING_ITEMS', 2)
 // Will output the value of the string SOME_STRING_ITEMS_2 if that exists or SOME_STRING_ITEMS_MORE
avatar infograf768
infograf768 - comment - 15 Nov 2018

Sorry, it is not an easy one at all for me. On the PHP part (I guess you mean in the public static function plural($string, $n) method), but also in core.js part where

// ... continue this pattern for all possible values...

because I have no way to know all possible values.

avatar dgrammatiko
dgrammatiko - comment - 15 Nov 2018

because I have no way to know all possible values.

Well, I'm not aware of the possible values as well but I guess with some research on the language files and their plural implementation someone can come up with the highest number the for loop in PHP and the switch in JS should go through. Anyways this was my 2c here

avatar tuum
tuum - comment - 15 Nov 2018

I have tested this item successfully on 60d9c23

Works as described


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

avatar tuum tuum - test_item - 15 Nov 2018 - Tested successfully
avatar mbabker
mbabker - comment - 15 Nov 2018

@dgrammatiko Even that snippet wouldn't work in the JS API, to do it right the JS API would need something very similar to the PHP API's getPluralSuffixes() method. Otherwise you'd impose an artificial restriction of "you must have a suffix for each numeric value that does NOT use a _MORE suffixed string".

avatar dgrammatiko
dgrammatiko - comment - 15 Nov 2018

@mbabker I guess my proposal was on En-UK. For each other language you're right the plural function needs to be a 1-1 thing of the PHP, but then that doesn't need to be included in each page load. Thus, similarly to the approach of the calendar locales a small script that overrides this function (whenever the plural method in PHP is invoked with the ['script => true]` will work here as well. The idea that core.js will hold a plural method that fits all languages is very wrong IMHO. Anyways my 2c

avatar Fedik
Fedik - comment - 15 Nov 2018

I not very imagine how it can work.
There need to have PHP to load the strings (all plural forms), and JS to use of them (requested plural form), it kind of duplication.

Not a quick fix ?
Just translate them on server.

avatar infograf768
infograf768 - comment - 16 Nov 2018

@tuum
Thanks for testing!
As I am making another PR which will also solve the Message display in LTR and RTL (See #23061 (comment) ), I will include this in it and close this one.

avatar infograf768
infograf768 - comment - 16 Nov 2018

Closing. Please test #23095

avatar infograf768 infograf768 - change - 16 Nov 2018
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2018-11-16 10:53:08
Closed_By infograf768
avatar infograf768 infograf768 - close - 16 Nov 2018

Add a Comment

Login with GitHub to post a comment