Differential D28489

digital electricity, move config to activityConfig multiple dataset style
ClosedPublic
Actions

Authored by dekumar on Apr 1 2020, 5:59 PM.

Details

Reviewers
jjazeix
Group Reviewers
GCompris: Improvements
Maniphest Tasks
T12880: digital electricity, movie config to activityConfig multiple dataset style

Diff Detail

Repository
R2 GCompris
Branch
digital_config
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 24743
Build 24761: arc lint + arc unit
dekumar created this revision.Apr 1 2020, 5:59 PM
Restricted Application added a project: KDE Edu. · View Herald TranscriptApr 1 2020, 5:59 PM
Restricted Application added a subscriber: kde-edu. · View Herald Transcript
dekumar requested review of this revision.Apr 1 2020, 5:59 PM
dekumar retitled this revision from Activity config added to digital electricity, movie config to activityConfig multiple dataset style.Apr 1 2020, 6:00 PM
dekumar added reviewers: jjazeix, GCompris: Improvements.
dekumar added a task: T12880: digital electricity, movie config to activityConfig multiple dataset style.
jjazeix added a comment.Apr 1 2020, 6:30 PM

except a small change, looks good to me

src/activities/digital_electricity/DigitalElectricity.qml
117 ↗(On Diff #79318)

can you keep this variable? It avoids using everywhere a string that we may mistypo.

jjazeix added a comment.Apr 1 2020, 6:51 PM

Error on terminal when starting the activity:
load configuration for: "digital_electricity"
qrc:/gcompris/src/activities/digital_electricity/digital_electricity.js:404: TypeError: Cannot read property 'availablePieces' of undefined
qml: no levels to load for digital_electricity

dekumar updated this revision to Diff 79228.Apr 3 2020, 3:15 PM
  • All issues fixed
  • Made all the requested changes
jjazeix added inline comments.Apr 3 2020, 3:16 PM
src/activities/digital_electricity/DigitalElectricity.qml
113

true/false for boolean

dekumar updated this revision to Diff 79229.Apr 3 2020, 3:19 PM
  • Boolean changed to true/false
dekumar marked an inline comment as done.Apr 3 2020, 3:19 PM
jjazeix added inline comments.Apr 3 2020, 3:20 PM
src/activities/digital_electricity/ActivityConfig.qml
25

not needed

src/activities/digital_electricity/DigitalElectricity.qml
119

it would be better to have it just before the line of isTutorialMode so we group together the related information

dekumar retitled this revision from digital electricity, movie config to activityConfig multiple dataset style to digital electricity, move config to activityConfig multiple dataset style.Apr 3 2020, 3:24 PM
dekumar added inline comments.
src/activities/digital_electricity/ActivityConfig.qml
25

I have imported it we need to reset the activity after the change of mode from tutorial to free and vice versa.

src/activities/digital_electricity/DigitalElectricity.qml
119

Sure, would update it by grouping together.

jjazeix added a comment.Apr 3 2020, 4:32 PM

As mentioned by Timothée on irc, if you remove the configuration file (~/config/gcompris/gcompris-qt.conf) and start GCompris, then start Digital Electricity, the tutorial is not displayed, can you fix it too?

dekumar added a comment.Apr 3 2020, 4:55 PM
In D28489#640859, @jjazeix wrote:

As mentioned by Timothée on irc, if you remove the configuration file (~/config/gcompris/gcompris-qt.conf) and start GCompris, then start Digital Electricity, the tutorial is not displayed, can you fix it too?

Sure, I will have a look.

dekumar updated this revision to Diff 79318.Apr 4 2020, 3:30 PM
  • Made all the requested changes
dekumar added a comment.Apr 4 2020, 3:31 PM

@jjazeix I have made the requested changes and fixed the config file issue too.
Thanks!

This revision is now accepted and ready to land.Apr 4 2020, 4:47 PM
jjazeix closed this revision.Apr 4 2020, 4:48 PM

Revision Contents
Changeset List

Diff 79318

View Options

src/activities/digital_electricity/ActivityConfig.qml

Loading...
View Options

src/activities/digital_electricity/DigitalElectricity.qml

Loading...