piano_composition, add replace and edit features.
ClosedPublic

Authored by amankumargupta on May 24 2018, 9:25 AM.

Diff Detail

Repository
R2 GCompris
Lint
Lint Skipped
Unit
Unit Tests Skipped
amankumargupta created this revision.May 24 2018, 9:25 AM
Restricted Application added a project: KDE Edu. · View Herald TranscriptMay 24 2018, 9:25 AM
Restricted Application added a subscriber: kde-edu. · View Herald Transcript
amankumargupta requested review of this revision.May 24 2018, 9:25 AM
amankumargupta added inline comments.
src/activities/piano_composition/SwitchableOptions.qml
43

Hardcoding has been removed. We now assign the nbOptions when we use SwitchableOptions in the activity.

jjazeix accepted this revision.May 26 2018, 9:00 AM
jjazeix added a subscriber: jjazeix.
jjazeix added inline comments.
src/activities/piano_composition/MultipleStaff.qml
40

can you use a var noteToReplace: {"noteNumber": -1, "staffNumber": -1} instead?

src/activities/piano_composition/Note.qml
62

why 6 is hardcoded?

62

positionOnStaff (missing 'i')

119

why -6, can you create a readonly property for it?

This revision is now accepted and ready to land.May 26 2018, 9:00 AM

Commit link: https://cgit.kde.org/gcompris.git/commit/?h=gsoc_aman_piano_activities&id=306dcec1501b10348a5ee455b8e8392381096268

All the improvements as suggested in the comments will be completed in the next commit along with the undo feature.

amankumargupta marked 4 inline comments as done.May 26 2018, 10:07 AM
amankumargupta added inline comments.
src/activities/piano_composition/Note.qml
62

because the notes whose position are more than 6 on the staff will have ledger lines and in that case the whole note image has to be inverted.

However I've added a readonly property variable for this to make it easy to understand for anyone reading the code.

119

-6 is the returned value denoting invalid condition. Since the numbers upto -5 are already occupied, I chose -6 to be the invalid case number.

Sure, will create a readonly property for it.