add keyboard navigation in simplepaint
ClosedPublic

Authored by jjazeix on Nov 29 2018, 6:45 PM.

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped
jjazeix created this revision.Nov 29 2018, 6:45 PM
Restricted Application added a project: KDE Edu. · View Herald TranscriptNov 29 2018, 6:45 PM
Restricted Application added a subscriber: kde-edu. · View Herald Transcript

Thank you for the PR, I did some comments

src/activities/simplepaint/PaintCursor.qml
2

update the filename

src/activities/simplepaint/Simplepaint.qml
44

the coding rules incites to add a space before the {
In the if condition too (if(true) { ... )

48

can you add spaces before and after '='?

99

isColorTab = !isColorTab

146

use an "int" if you already know the type. var is slower

349

I think you can directly put the background.refreshCursor() in the initLevel().
In this case, you don't need to call it on onPreviousLevelClicked and onNextLevelClicked as they call the initLevel()

jjazeix added inline comments.Nov 30 2018, 7:30 AM
src/activities/simplepaint/Simplepaint.qml
163

can you clean up the log, it's not needed when we release

jjazeix updated this revision to Diff 46558.Nov 30 2018, 2:12 PM
jjazeix updated this revision to Diff 46560.Nov 30 2018, 2:15 PM
jjazeix updated this revision to Diff 46586.Nov 30 2018, 8:45 PM

Good for me. Please push it and I'll update the icon with a new one more appropriate.

jjazeix updated this revision to Diff 47366.Dec 11 2018, 7:45 PM
jjazeix updated this revision to Diff 47384.Dec 11 2018, 8:16 PM
jjazeix updated this revision to Diff 47385.
timotheegiet accepted this revision.Jun 18 2020, 7:15 PM

Patch merged in commit 42142a0ce8b441916987f6be8463e73244ecdb93 ,
and then improved upon it in next commit 7433227a9f89d6c47ad94e515197e4d95dacaaed

This revision is now accepted and ready to land.Jun 18 2020, 7:15 PM
timotheegiet closed this revision.Jun 18 2020, 7:15 PM