D17377: keyboard binding support to "Money" based activities in gcompis

[Aman Kumar Gupta]

amankumargupta added inline comments.

INLINE COMMENTS

> MoneyCore.qml:135
> +                                width: parent.width*1.1
> +                                height: parent.height*1.1
> +                                color: "#88111111"

Needs spaces (parent.height * 1.1). Same for above.

> MoneyCore.qml:140
> +                                visible: selected
> +                                z:-1
> +                            }

Needs space (z: -1)

> MoneyCore.qml:300
> +                                width: parent.width*1.2
> +                                height: parent.height*1.2
> +                                color: "#88111111"

width and height as multiples of 1.1 should be good, as you had done above. Also space needed.

> MoneyCore.qml:305
> +                                visible: selected
> +                                z:-1
> +                            }

Space needed. Refer above.

> MoneyCore.qml:316
> +        Keys.onPressed: {
> +
> +            if(event.key === Qt.Key_Tab){

Empty line not needed here.

> MoneyCore.qml:318
> +            if(event.key === Qt.Key_Tab){
> +
> +                if(items.area.count!==0)

Empty line not needed here.

> MoneyCore.qml:319
> +
> +                if(items.area.count!==0)
> +                   items.area.itemAt(items.itemIndex).selected=false

Space needed (items.area.count !== 0)

> MoneyCore.qml:320
> +                if(items.area.count!==0)
> +                   items.area.itemAt(items.itemIndex).selected=false
> +

Space needed (items.area.itemAt(items.itemIndex).selected = false)

> MoneyCore.qml:324
> +                    items.pocketFocus = false
> +                    items.area= answer
> +                }

Space needed (items.area = answer)

> MoneyCore.qml:328
> +                    items.pocketFocus = true
> +                    items.area= pocket
> +                }

Space needed (items.area = pocket)

> MoneyCore.qml:330
> +                }
> +                items.itemIndex=0
> +            }

Space needed.

> MoneyCore.qml:333
> +
> +            if(items.area.count!==0 ){
> +

Space needed, and remove extra space, if(items.area.count !==0) {

> MoneyCore.qml:334
> +            if(items.area.count!==0 ){
> +
> +                if(items.itemIndex>=0)

Empty line not needed.

> MoneyCore.qml:335
> +
> +                if(items.itemIndex>=0)
> +                   items.area.itemAt(items.itemIndex).selected=false

Space needed. Same for the statement below.

> MoneyCore.qml:338
> +
> +                switch(event.key) {
> +

Since you were already using if-else statements above, can you maintain the consistency by replacing the switch case statements with if-else?

> MoneyCore.qml:339
> +                switch(event.key) {
> +
> +                case Qt.Key_Right:

Extra blank line not needed.

> MoneyCore.qml:341
> +                case Qt.Key_Right:
> +                    if(items.itemIndex!= (items.area.count-1))
> +                        items.itemIndex++

Space needed on left of != operator.

> MoneyCore.qml:347
> +                case Qt.Key_Left:
> +                    if(items.itemIndex>0)
> +                        items.itemIndex--

Space needed.

> MoneyCore.qml:350
> +                    else
> +                        items.itemIndex =items.area.count-1
> +                    break;

Space needed after = operator.

> MoneyCore.qml:358
> +                        Activity.unpay(items.itemIndex)
> +                    if(items.itemIndex>0)
> +                        items.itemIndex--

Space needed

> MoneyCore.qml:361
> +                    break
> +
> +                }

Empty line not needed.

> MoneyCore.qml:364
> +                if(items.area.count !== 0)
> +                   items.area.itemAt(items.itemIndex).selected=true
> +            }

Space needed on sides of = operator

> MoneyCore.qml:366
> +            }
> +
> +        }

Empty line not needed.

> MoneyCore.qml:369
> +
> +
>          DialogHelp {

One extra empty line not needed.

> money.js:716
> +    //Keyboard reset
> +    items.itemIndex= -1
> +    items.pocketFocus = true

Space needed on left of = operator.

> money.js:720
> +
> +
>  }

One extra empty line not needed.

REPOSITORY
  R2 GCompris

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

To: smitpatil, #gcompris_improvements
Cc: amankumargupta, kde-edu, harrymecwan, ganeshredcobra, nityanandkumar, echarruau, rahulyadav, narvaez, scagarwal, apol, timotheegiet, hkaelberer, jjazeix, bcoudoin
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20181206/a5fdcc5a/attachment-0001.html>