Added Multiple datasets to magician hat activity
Details
- Reviewers
timotheegiet amankumargupta scagarwal AkshayCHD - Group Reviewers
GCompris: Improvements - Maniphest Tasks
- T10908: Multiple datasets for The magician hat Activity
- Commits
- R2:3960fa03786f: magic-hat, add multiple datasets
R2:c5c9179f019b: magic-hat, fix typo
R2:1afe6395a94f: magic-hat, improve variable names and fix typos
R2:48f80916e509: magic-hat, add activity configuration to change dataset
R2:858cb05a0c27: magic-hat-plus, update ActivityInfo.qml to include newly added levels
R2:66cb83b1bf9c: magic-hat, add comments and fix typos
Commits of this diff can be found in branch gsoc_akshaychd_magicianhat_multiple_datasets
Diff Detail
- Repository
- R2 GCompris
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
lots of trailing whitespaces.
src/activities/magic-hat-minus/MagicHat.qml | ||
---|---|---|
69 | "calculation_range" would be better? | |
src/activities/magic-hat-minus/magic-hat.js | ||
66–67 ↗ | (On Diff #60821) | don't use hardcoded values. Keep it in a variable and use that variable it. Will be more flexible and easy for anyone to find modify it he wants. |
67–68 ↗ | (On Diff #60821) | missing spaces between operators and operands. |
86 ↗ | (On Diff #60821) | Put the expressions in parenthesis. Same comment for the ones below. |
130 ↗ | (On Diff #60821) | Why not use a nested loop from 0 to 2? |
142 ↗ | (On Diff #60821) | nested loop would be better |
154–161 ↗ | (On Diff #60821) | The same thing for this formula like I said previously for the one above. |
163–170 ↗ | (On Diff #60821) | Can you rename these two variables to more understandable names (this one and the one below)? |
src/activities/magic-hat-minus/StarsBar.qml | ||
---|---|---|
48 ↗ | (On Diff #60821) | use .arg() for translation, on some locale, it may not be "3 x" but "3x" or even "x3" or not x at all. |
src/activities/magic-hat-plus/resource/4/Data.qml | ||
29 ↗ | (On Diff #60821) | if it's always an int, use int as type. Note that a range is usually two bounds, not a single number, it's more a maxValue here |
@timotheegiet @amankumargupta @jjazeix I have updated the diff, you may have a look :)
- magic-hat, add multiple datasets
- magic-hat, improve variable names and fix typos
- magic-hat-plus, update ActivityInfo.qml to include newly added levels
- magic-hat, add multiple datasets
- magic-hat, improve variable names and fix typos
- magic-hat-plus, update ActivityInfo.qml to include newly added levels
- magic-hat, add comments and fix typos
- magic-hat, include an unsaved change
src/activities/magic-hat-minus/resource/1/Data.qml | ||
---|---|---|
32 ↗ | (On Diff #60821) | not needed? |
The diff is not complete (missing datasets for example). Just one point to fix, on last dataset of minus-hat, it's up to 10000 not 1000.
@echarruau , for higher levels, would it make sense to have an hint on the numbers ? When the coefficients are displayed, and the stars are displayed, it maybe difficult for children to count exactly?
- magic-hat, add multiple datasets
- magic-hat, improve variable names and fix typos
- magic-hat-plus, update ActivityInfo.qml to include newly added levels
- magic-hat, add comments and fix typos
- magic-hat, include an unsaved change
- magic-hat, remove unwanted parameters from dataset
- magic-hat, fix bugs
- magic-hat, add activity configuration to change dataset
- magic-hat, add multiple datasets
- magic-hat, improve variable names and fix typos
- magic-hat-plus, update ActivityInfo.qml to include newly added levels
- magic-hat, add comments and fix typos
- magic-hat, include an unsaved change
- magic-hat, remove unwanted parameters from dataset
- magic-hat, fix bugs
- magic-hat, add activity configuration to change dataset
- magic-hat, fix typo