D17377: keyboard binding support to "Money" based activities in gcompis
Aman Kumar Gupta
noreply at phabricator.kde.org
Thu Dec 6 08:15:47 GMT 2018
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>
More information about the kde-edu
mailing list