Differential D7373 Diff 18450 src/activities/letter-in-word/LetterInWord.qml

Changeset View

Standalone View

View Options

src/activities/letter-in-word/LetterInWord.qml

Context not available.
64 property Item main: activity.main 64​ property Item main: activity.main
65 property alias bar: bar 65​ property alias bar: bar
66 property alias background: background 66​ property alias background: background
67 property alias wordsModel: wordsModel 67​ property alias wordsModel: wordsModel
jjazeixUnsubmitted
Done

remove whitespaces

jjazeix: remove whitespaces
68​ property bool reduceWords: false
69​ property string mode: "normal"
dmadaanUnsubmitted
Not Done

why do you keep two variables? You keep mode and two booleans easy normal mode, they both do the same work?

dmadaan: why do you keep two variables? You keep mode and two booleans easy normal mode, they both do…
jjazeixUnsubmitted
Not Done

why not directly store the number of words as int? It would have more sense than a string "easy", "normal"and would be directly be useable in the js file

jjazeix: why not directly store the number of words as int? It would have more sense than a string…
jjazeixUnsubmitted
Not Done

you can rename it to be more precise on what it does
and have "readonly int" variables to name the possible values (reading 5 and 11 at multiple places does not help to know where these values come from and if they are linked or not)

jjazeix: you can rename it to be more precise on what it does and have "readonly int" variables to name…
68 property GCAudio audioVoices: activity.audioVoices 70​ property GCAudio audioVoices: activity.audioVoices
69 property alias parser: parser 71​ property alias parser: parser
70 property alias animateX: animateX 72​ property alias animateX: animateX
jjazeixUnsubmitted
Not Done

missing space

jjazeix: missing space
dmadaanUnsubmitted
Not Done

the names can be better what is easy mode should be signigicant from the variable name.

dmadaan: the names can be better what is easy mode should be signigicant from the variable name.
Context not available.
107 Column { 109​ Column {
108 id: column 110​ id: column
109 spacing: 10 111​ spacing: 10
110 width: parent.width 112​ width: dialogActivityConfig.width
113​ height: dialogActivityConfig.height
114​ property alias easyMode: easyMode
115​ property alias normalMode: normalMode
116​
117​ GCDialogCheckBox {
118​ id: easyMode
119​ width: column.width - 50
120​ text: qsTr("Reduce Visible Words to 5")
121​ checked: (items.mode == "easy") ? true : false
122​ onCheckedChanged: {
123​ if(easyMode.checked) {
124​ items.mode = "easy"
125​ items.reduceWords = true
126​ }
127​ else{
jjazeixUnsubmitted
Not Done

you still use the numbers instead of the variables you defined

jjazeix: you still use the numbers instead of the variables you defined
128​ items.mode = "normal"
jjazeixUnsubmitted
Not Done

I still don't understand why you change mode here

jjazeix: I still don't understand why you change mode here
129​ items.reduceWords = false
130​ }
131​ normalMode.checked = !easyMode.checked
132​ }
133​ }
134​
135​ GCDialogCheckBox {
136​ id: normalMode
137​ width: column.width - 50
138​ text: qsTr("Get All Available Words")
139​ checked: (items.mode == "normal") ? true : false
140​ onCheckedChanged: {
141​ if(normalMode.checked) {
142​ items.mode = "normal"
143​ items.reduceWords = false
144​ }
145​ else{
146​ items.mode = "easy"
147​ items.reduceWords = true
148​ }
149​ easyMode.checked = !normalMode.checked
150​ }
151​ }
111 152​
112 Flow { 153​ Flow {
113 spacing: 5 154​ spacing: 5
jjazeixUnsubmitted
Not Done

missing space

jjazeix: missing space
Context not available.
125 166​
126 onClose: home() 167​ onClose: home()
127 onLoadData: { 168​ onLoadData: {
128 if(dataToSave && dataToSave["locale"]) { 169​ if(dataToSave && dataToSave["mode"]){
129 background.locale = dataToSave["locale"]; 170​ items.mode = dataToSave["mode"]
171​ console.log(items.mode + " mode loaded")
jjazeixUnsubmitted
Not Done

"modes" can be renamed to be more precise

jjazeix: "modes" can be renamed to be more precise
172​ //Activity.initLevel()
130 } 173​ }
174​
175​ if(dataToSave && dataToSave["locale"]) {
176​ background.locale = dataToSave["locale"];
177​ }
131 } 178​ }
132 onSaveData: { 179​ onSaveData: {
133 var oldLocale = background.locale; 180​ var oldLocale = background.locale;
jjazeixUnsubmitted
Not Done

I still don't understand why you use 2 different variables for the same data

jjazeix: I still don't understand why you use 2 different variables for the same data
Context not available.
137 if(newLocale.indexOf('.') != -1) { 184​ if(newLocale.indexOf('.') != -1) {
138 newLocale = newLocale.substring(0, newLocale.indexOf('.')) 185​ newLocale = newLocale.substring(0, newLocale.indexOf('.'))
139 } 186​ }
140 dataToSave = {"locale": newLocale } 187​ dataToSave = {"locale": newLocale}
188​ dataToSave["mode"] = items.mode
189​ console.log(items.mode + " mode saved")
141 190​
142 background.locale = newLocale; 191​ background.locale = newLocale;
143 192​
jjazeixUnsubmitted
Not Done

qsTr needs to be used for translatable texts.
Easy does not help the child, we should find a more explicit text on what the option does

jjazeix: qsTr needs to be used for translatable texts. Easy does not help the child, we should find a…
jjazeixUnsubmitted
Done

you should use a GCDialogCheckBox instead of directly a GCText

jjazeix: you should use a GCDialogCheckBox instead of directly a GCText
jjazeixUnsubmitted
Done

no need to change the text color

jjazeix: no need to change the text color
jjazeixUnsubmitted
Done

you should not change the text directly

jjazeix: you should not change the text directly
jjazeixUnsubmitted
Not Done

it should only be changed when clicking on the cross (so in onSaveData).
You should also check that it is well loaded when you start the activity (onLoadData).
Take a look at categorisation for example to be sure you save and load the good js type.

jjazeix: it should only be changed when clicking on the cross (so in onSaveData). You should also check…
Context not available.
220 269​
221 GCText { 270​ GCText {
222 id: questionItem 271​ id: questionItem
223 anchors.right: planeText.right 272​ anchors {
224 anchors.rightMargin: 2 * plane.width / 3 273​ right: planeText.right
225 anchors.verticalCenter: planeText.verticalCenter 274​ rightMargin: 2 * plane.width / 3
226 anchors.bottomMargin: 10 * ApplicationInfo.ratio 275​ //Turned this off as the letter was going out of the banner
276​ //verticalCenter: planeText.verticalCenter
277​ bottomMargin: 10 * ApplicationInfo.ratio
278​ }
227 fontSize: hugeSize 279​ fontSize: hugeSize
228 font.weight: Font.DemiBold 280​ font.weight: Font.DemiBold
229 color: "#2a2a2a" 281​ color: "#2a2a2a"
Context not available.