balance scales multiple dataset
ClosedPublic

Authored by dekumar on Jan 13 2020, 6:20 PM.

Details

Summary
  1. Added all multipledata for balance scales.
  2. Added all multipledata for balance scales with kgs.
  3. Added all multipledata1 for balance scales with ounces.
  4. Added a OK button to check answer.

Diff Detail

Repository
R2 GCompris
Branch
scale_mdataset
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 22443
Build 22461: arc lint + arc unit
There are a very large number of changes, so older changes are hidden. Show Older Changes

Just a question for my own understanding.

src/activities/scalesboard/scalesboard.js
81

Why do we need initCompleted here? Is displayLevel so slow that we it is possible for the user to click on ok button before displayLevel is executed? Just a question for my own knowledge :)

Please fix these:

git apply balancescale.diff 
balancescale.diff:294: new blank line at EOF.
+
balancescale.diff:375: new blank line at EOF.
+
balancescale.diff:469: new blank line at EOF.
+
balancescale.diff:563: new blank line at EOF.
+
balancescale.diff:677: new blank line at EOF.
+
warning: squelched 2 whitespace errors
warning: 7 lines add whitespace errors.

The last activity of balancescales does not work anymore: qrc:/gcompris/src/activities/scalesboard_weight_avoirdupois/ScalesboardWeight.qml:35:5: Cannot assign to non-existent property "dataset"

src/activities/scalesboard/Scalesboard.qml
244

(items.question.text !== "")

src/activities/scalesboard_weight/resource/2/Data.qml
25

keep the consistency for all datasets for the objective.
Always use g or grams, kg or kilograms but not a mix

dekumar marked an inline comment as done.Jan 26 2020, 8:36 AM
dekumar added inline comments.
src/activities/scalesboard/scalesboard.js
81

@echarruau I think just to check that the level has been displayed properly or not. As it is a boolean used in the function displaylevel() and it is true after all the items of the level has been loaded.

src/activities/scalesboard_weight/resource/2/Data.qml
25

Sure, I would take care of that in further multipledata.

Please fix these:

git apply balancescale.diff 
balancescale.diff:294: new blank line at EOF.
+
balancescale.diff:375: new blank line at EOF.
+
balancescale.diff:469: new blank line at EOF.
+
balancescale.diff:563: new blank line at EOF.
+
balancescale.diff:677: new blank line at EOF.
+
warning: squelched 2 whitespace errors
warning: 7 lines add whitespace errors.

The last activity of balancescales does not work anymore: qrc:/gcompris/src/activities/scalesboard_weight_avoirdupois/ScalesboardWeight.qml:35:5: Cannot assign to non-existent property "dataset"

As I have not implemented multiple data to this activity now and it is broke, so after implementing multiple dataset to this activity too there would be no error.

dekumar updated this revision to Diff 74392.Jan 26 2020, 2:43 PM
  • Added multipledata3 and multipledata4 for balance_scale_weights
dekumar updated this revision to Diff 74609.Jan 29 2020, 6:44 PM
  • Added multipledata5 for balance_scale_weights
dekumar updated this revision to Diff 74680.Jan 30 2020, 12:33 PM
  • Added multipledata6 for balance_scales_weights
dekumar updated this revision to Diff 74878.Feb 2 2020, 8:26 PM
  • Added multipledata1 to scalesboard_weight_avoirdupois

git apply balancescale.diff
balancescale.diff:294: new blank line at EOF.
+
balancescale.diff:375: new blank line at EOF.
+
balancescale.diff:469: new blank line at EOF.
+
balancescale.diff:563: new blank line at EOF.
+
balancescale.diff:677: new blank line at EOF.
+
warning: squelched 7 whitespace errors
warning: 12 lines add whitespace errors.

git apply balancescale.diff
balancescale.diff:294: new blank line at EOF.
+
balancescale.diff:375: new blank line at EOF.
+
balancescale.diff:469: new blank line at EOF.
+
balancescale.diff:563: new blank line at EOF.
+
balancescale.diff:677: new blank line at EOF.
+
warning: squelched 7 whitespace errors
warning: 12 lines add whitespace errors.

I would update the diff by resolving this errors.

Can you please add validation upon "Enter"/"Return" key press?
Code and test is good for now :)

src/activities/scalesboard_weight/resource/1/Data.qml
25

missing space

51

For activities that have units, it would be better to add which unit is expected in the question (for example, when we have both kg and g, it is not obvious that we expect an answer in grams)

dekumar updated this revision to Diff 74957.Feb 3 2020, 8:52 PM
dekumar edited the summary of this revision. (Show Details)
  • Added Enter/Return option to validate the answer
dekumar marked 2 inline comments as done.Feb 3 2020, 8:53 PM
dekumar updated this revision to Diff 74999.Feb 4 2020, 4:17 PM
  • Added muultipledata2 and multipledata3
dekumar updated this revision to Diff 75139.Feb 6 2020, 9:06 PM
  • Added multipledata5 and multipledata6

@echarruau @jjazeix I have added all the datasets of each of the three activities. I have tested the dataset too once and could not find any error or issue. I will test it tomorrow once again to get 100% sure about no issue with the dataset.

Thanks!
Deepak Kumar

Good for me except:

  • When we are in a level with a question ("Guess the weight"), key input and ok button should be enabled (and visible) only when the question is visible. Else we can still input numbers while we haven't put the good weights.
src/activities/scalesboard/Scalesboard.qml
315–316

@echarruau does it make sense to hide the sublevel counter when we are in a "Guess the weight" question?
For me, it should still be displayed.

src/activities/scalesboard_weight/resource/6/Data.qml
44

why is there a '+' in the middle of the second qsTr?

dekumar updated this revision to Diff 75305.Feb 9 2020, 3:38 PM
  • made all the requested changes

@jjazeix @echarruau I have done all the requested changes and tested all the multiple data again, I could not find any error or any issue now. Please test and let me know if still there's any changes I need to do :)
Thanks!

jjazeix added inline comments.Feb 10 2020, 7:34 AM
src/activities/scalesboard/Scalesboard.qml
277

true/false

dekumar updated this revision to Diff 75328.Feb 10 2020, 7:52 AM
  • Boolean changed to true/false
dekumar marked an inline comment as done.Feb 10 2020, 7:52 AM

@dekumar we can still input numbers while the question is not displayed, causing the OK button to appear and we can still validate before having the question displayed

dekumar updated this revision to Diff 75480.Feb 11 2020, 4:09 PM
  • Made the requested changes

@dekumar we can still input numbers while the question is not displayed, causing the OK button to appear and we can still validate before having the question displayed

@jjazeix I have made the requested changes. Please test it and let me know if still there any changes I need to make.
Thanks :)

jjazeix added inline comments.Feb 11 2020, 7:33 PM
src/activities/scalesboard/Scalesboard.qml
240–241

can you rework this?
It seems items.question is this "Question" so it does not make any sense to refer to it for text.

And this should not be needed as visible is set to false if there is no question?

242

there is already a "displayed" property in the Question.qml, is it necessary to duplicate it?
Note that the "displayed" property just set opacity to 0

249–261

Same as above, can it be replaced with the "displayed" property?

330

can you use a better property than "background.scaleHeight === 0"? If you just read the code, you don't understand where this comes from.

dekumar updated this revision to Diff 75565.Feb 12 2020, 5:43 PM
  • Made the requested changes

@jjazeix I have removed the displayed property and done some changes to the question item. Please review and let me know if it's all good.
Thanks!

jjazeix added inline comments.Feb 12 2020, 6:40 PM
src/activities/scalesboard/Question.qml
40–41

you don't need to set the opacity to 0 if it is not visible

58–59

same as above + opacity is a real between 0.0 and 1.0, not a boolean

src/activities/scalesboard/Scalesboard.qml
277

remove extra space

dekumar updated this revision to Diff 75570.Feb 12 2020, 7:05 PM
  • Made the requested changes
dekumar marked 3 inline comments as done.Feb 12 2020, 7:06 PM
dekumar added inline comments.
src/activities/scalesboard/Question.qml
40–41

I forget to update this. Sorry for that. I would change the opacity and update the diff.

58–59

yeah, I forgot to change this and by removing the displayed property. Would change the opacity to value between 0 to 1.

dekumar marked 2 inline comments as done.Feb 12 2020, 7:07 PM

@jjazeix I have updated the diff by making the requested changes.

Answered question about hiding sublevels when guessing the weight.

src/activities/scalesboard/Scalesboard.qml
315–316

It does not make sens to hide it. Pupils need to be able to go down a level if they are not managing this level to retrain the precedent one.
In GCompris we do not have a "punishing teaching" approach but a "try error and retry" approach which is much more adapted to the way our brain learns. We are in a dynamic of offering a reflective way of working.

timotheegiet requested changes to this revision.Feb 13 2020, 1:23 PM

The position of the OK button is not very good:
-It should be more on the top of the activity to be more visible
-right now in vertical window/portrait mode, it gets hidden under the score

That's two reasons to move it another place, preferably on the top-right corner.

This revision now requires changes to proceed.Feb 13 2020, 1:23 PM

Also, I think it would be good to hide the instruction box at the top when clicking inside the activity/on weights, right now it only hides when clicking on it directly, which is a bit bothering/not the best user experience.
And it will allow to have more easily the OK button in the top right corner and not be hidden by the instruction box.

dekumar updated this revision to Diff 75627.Feb 13 2020, 5:08 PM
  • Made the requested chnages

Also, I think it would be good to hide the instruction box at the top when clicking inside the activity/on weights, right now it only hides when clicking on it directly, which is a bit bothering/not the best user experience.
And it will allow to have more easily the OK button in the top right corner and not be hidden by the instruction box.

@timotheegiet have updated the diff by making this changes,

Thanks. Little comment though...

src/activities/scalesboard/MasseArea.qml
192

Better put that at the beginning of the onPressed

dekumar updated this revision to Diff 75632.Feb 13 2020, 6:25 PM
  • Made the requested changes

Thanks. Little comment though...

Done!

timotheegiet accepted this revision.Feb 13 2020, 8:18 PM

Looks good for me.

For the OK button position, I can take a look to move it after the patch is merged, as I think it will require a bit of layout refactoring.

This revision is now accepted and ready to land.Feb 13 2020, 8:18 PM

Code is good for me, so if the behaviour is good for Animtim, it can be commited :)
Thank you