- Added all multipledata for balance scales.
- Added all multipledata for balance scales with kgs.
- Added all multipledata1 for balance scales with ounces.
- Added a OK button to check answer.
Details
Details
Diff Detail
Diff Detail
- Repository
- R2 GCompris
- Branch
- scale_mdataset
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 21468 Build 21486: arc lint + arc unit
There are a very large number of changes, so older changes are hidden. Show Older Changes
Comment Actions
Just a question for my own understanding.
| src/activities/scalesboard/scalesboard.js | ||
|---|---|---|
| 81 ↗ | (On Diff #73979) | 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 :) |
Comment Actions
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 | ||
|---|---|---|
| 247 ↗ | (On Diff #73979) | (items.question.text !== "") |
| src/activities/scalesboard_weight/resource/2/Data.qml | ||
| 24 ↗ | (On Diff #74331) | keep the consistency for all datasets for the objective. |
| src/activities/scalesboard/scalesboard.js | ||
|---|---|---|
| 81 ↗ | (On Diff #73979) | @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 | ||
| 24 ↗ | (On Diff #74331) | Sure, I would take care of that in further multipledata. |
Comment Actions
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.
Comment Actions
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.
Comment Actions
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 | ||
|---|---|---|
| 24 ↗ | (On Diff #74878) | missing space |
| 50 ↗ | (On Diff #74878) | 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) |
Comment Actions
@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
Comment Actions
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 | ||
|---|---|---|
| 320 ↗ | (On Diff #73979) | @echarruau does it make sense to hide the sublevel counter when we are in a "Guess the weight" question? |
| src/activities/scalesboard_weight/resource/6/Data.qml | ||
| 43 ↗ | (On Diff #75139) | why is there a '+' in the middle of the second qsTr? |
Comment Actions
@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!
| src/activities/scalesboard/Scalesboard.qml | ||
|---|---|---|
| 282 ↗ | (On Diff #73979) | true/false |
Comment Actions
@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
Comment Actions
@jjazeix I have made the requested changes. Please test it and let me know if still there any changes I need to make.
Thanks :)
| src/activities/scalesboard/Scalesboard.qml | ||
|---|---|---|
| 242–243 ↗ | (On Diff #73979) | can you rework this? And this should not be needed as visible is set to false if there is no question? |
| 245 ↗ | (On Diff #73979) | there is already a "displayed" property in the Question.qml, is it necessary to duplicate it? |
| 254–266 ↗ | (On Diff #73979) | Same as above, can it be replaced with the "displayed" property? |
| 336 ↗ | (On Diff #73979) | can you use a better property than "background.scaleHeight === 0"? If you just read the code, you don't understand where this comes from. |
Comment Actions
@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!
Comment Actions
Answered question about hiding sublevels when guessing the weight.
| src/activities/scalesboard/Scalesboard.qml | ||
|---|---|---|
| 320 ↗ | (On Diff #73979) | 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. |
Comment Actions
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.
Comment Actions
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.
Comment Actions
Thanks. Little comment though...
| src/activities/scalesboard/MasseArea.qml | ||
|---|---|---|
| 192 ↗ | (On Diff #75627) | Better put that at the beginning of the onPressed |
Comment Actions
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.
Comment Actions
Code is good for me, so if the behaviour is good for Animtim, it can be commited :)
Thank you
Comment Actions
Commited in https://commits.kde.org/gcompris/c88e952c1ce655368d6ef3f84b2dd643bf30dcaf
Thank you