D18600: Improvements in ascending_order activity

[Johnny Jazeix]

jjazeix added inline comments.

INLINE COMMENTS

> ActivityInfo.qml:1
> +/* GCompris - ActivityInfo.qml
> + *

if the activity didn't exist before, don't forget to change the copyright and year for the corresponding files

> dataset.json:17
> +        {
> +            "maxNumber": 3,
> +            "minNumber": 0

just a quick point, it's not "natural" (at least for me, maybe it's not universal :)) to have the max value before the min

> ordering_numbers.js:106
> +    var lowerBound
> +    upperBound = allData[items.bar.level - 1].maxNumber
> +    lowerBound = allData[items.bar.level - 1].minNumber

you can directly assign on declaration

> ordering_numbers.js:108
> +    lowerBound = allData[items.bar.level - 1].minNumber
> +    while(num.length < n) {
> +        var randomNumber = Math.ceil(Math.random() * (upperBound - lowerBound) + lowerBound)

can it be more generic? For now, if we add a new mode, we have to change multiple files. If we can have the possibility to limit the changes on the specific activity files, it would be way better

> ordering_numbers.js:176
> +    if(items.mode == "number") {
> +        sortedItems.sort(function(a, b) { return a - b })
> +    } else if(items.mode == "alphabets" || items.mode == "image") {

why do we need a specific sort function and the sort without argument doesn't work?

REPOSITORY
  R2 GCompris

REVISION DETAIL
  https://phabricator.kde.org/D18600

To: AkshayCHD, #gcompris_activities, jjazeix
Cc: jjazeix, echarruau, kde-edu, #gcompris_activities, parimalprasoon, harrymecwan, ganeshredcobra, nityanandkumar, rahulyadav, narvaez, scagarwal, apol, timotheegiet, bcoudoin
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20190303/ab126d1c/attachment.html>