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

Changeset View

Standalone View

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

Context not available.
24		24
			jjazeixUnsubmitted Not Done you shouldn't change anything on checking/unchecking the box, it should only be done when the user clicks on the Cross to go back to the activity (onSaveData). jjazeix: you shouldn't change anything on checking/unchecking the box, it should only be done when the…
25	import QtQuick 2.6	25		import QtQuick 2.6
26	import QtGraphicalEffects 1.0	26		import QtGraphicalEffects 1.0
		27		import QtQuick.Controls 1.5
			jjazeixUnsubmitted Not Done still needed? jjazeix: still needed?
27	import GCompris 1.0	28		import GCompris 1.0
28	import "../../core"	29		import "../../core"
29	import "letter-in-word.js" as Activity	30		import "letter-in-word.js" as Activity
			jjazeixUnsubmitted Not Done you should use ExclusiveGroup (https://doc.qt.io/qt-5/qml-qtquick-controls-exclusivegroup.html) jjazeix: you should use ExclusiveGroup (https://doc.qt.io/qt-5/qml-qtquick-controls-exclusivegroup.html)
			jjazeixUnsubmitted Not Done if I'm not wrong, you never restore the reduceWords when starting the activity jjazeix: if I'm not wrong, you never restore the reduceWords when starting the activity
			jjazeixUnsubmitted Not Done why do you change it here? It should only be changed when clicking on the cross (save method). Also, the names of the states are not the good ones. jjazeix: why do you change it here? It should only be changed when clicking on the cross (save method).
Context not available.
65	property alias bar: bar	66		property alias bar: bar
66	property alias background: background	67		property alias background: background
			jjazeixUnsubmitted Done remove whitespaces jjazeix: remove whitespaces
67	property alias wordsModel: wordsModel	68		property alias wordsModel: wordsModel
		69		property int currentMode
			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…
		70		readonly property int easyModeWordCount: 5
		71		readonly property int normalModeWordCount: 11
68	property GCAudio audioVoices: activity.audioVoices	72		property GCAudio audioVoices: activity.audioVoices
69	property alias parser: parser	73		property alias parser: parser
70	property alias animateX: animateX	74		property alias animateX: animateX
Context not available.
91	animateX.restart();	95		animateX.restart();
92	}	96		}
93		97
			jjazeixUnsubmitted Not Done missing space jjazeix: missing space
		98		ExclusiveGroup {
		99		id: configOptions
		100		}
		101
94	DialogActivityConfig {	102		DialogActivityConfig {
95	id: dialogActivityConfig	103		id: dialogActivityConfig
96	currentActivity: activity	104		currentActivity: activity
97	content: Component {	105		content: Component {
98	Item {	106		Item {
99	property alias localeBox: localeBox	107		property alias localeBox: localeBox
		108		property alias easyModeConfig: easyModeConfig
			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.
		109		property alias normalModeConfig: normalModeConfig
100	height: column.height	110		height: column.height
101		111
102	property alias availableLangs: langs.languages	112		property alias availableLangs: langs.languages
Context not available.
107	Column {	117		Column {
108	id: column	118		id: column
109	spacing: 10	119		spacing: 10
110	width: parent.width	120		width: dialogActivityConfig.width
		121		height: dialogActivityConfig.height
		122
		123		GCDialogCheckBox {
		124		id: normalModeConfig
		125		width: column.width - 50
		126		text: qsTr("All words")
		127		checked: (items.currentMode === items.normalModeWordCount) ? true : false
			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		exclusiveGroup: configOptions
			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		}
		130
		131		GCDialogCheckBox {
		132		id: easyModeConfig
		133		width: column.width - 50
		134		text: qsTr("Only 5 words")
		135		checked: (items.currentMode === 5) ? true : false
		136		exclusiveGroup: configOptions
		137		}
111		138
112	Flow {	139		Flow {
113	spacing: 5	140		spacing: 5
Context not available.
125		152
126	onClose: home()	153		onClose: home()
127	onLoadData: {	154		onLoadData: {
		155		if(dataToSave && dataToSave["savedMode"]) {
			jjazeixUnsubmitted Not Done missing space jjazeix: missing space
		156		items.currentMode = dataToSave["savedMode"] === "5" ? items.easyModeWordCount : items.normalModeWordCount
		157		}
		158
128	if(dataToSave && dataToSave["locale"]) {	159		if(dataToSave && dataToSave["locale"]) {
129	background.locale = dataToSave["locale"];	160		background.locale = dataToSave["locale"];
130	}	161		}
Context not available.
137	if(newLocale.indexOf('.') != -1) {	168		if(newLocale.indexOf('.') != -1) {
138	newLocale = newLocale.substring(0, newLocale.indexOf('.'))	169		newLocale = newLocale.substring(0, newLocale.indexOf('.'))
139	}	170		}
140	dataToSave = {"locale": newLocale }	171		dataToSave = {"locale": newLocale}
		172		items.currentMode = dialogActivityConfig.loader.item.easyModeConfig.checked ? items.easyModeWordCount : items.normalModeWordCount
		173		dataToSave["savedMode"] = items.currentMode
			jjazeixUnsubmitted Not Done "modes" can be renamed to be more precise jjazeix: "modes" can be renamed to be more precise
		174
		175		Activity.initLevel()
141		176
142	background.locale = newLocale;	177		background.locale = newLocale;
143		178
			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 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.
220		255
221	GCText {	256		GCText {
222	id: questionItem	257		id: questionItem
223	anchors.right: planeText.right	258		anchors {
224	anchors.rightMargin: 2 * plane.width / 3	259		right: planeText.right
			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…
225	anchors.verticalCenter: planeText.verticalCenter	260		rightMargin: 2 * plane.width / 3
			jjazeixUnsubmitted Done you should not change the text directly jjazeix: you should not change the text directly
226	anchors.bottomMargin: 10 * ApplicationInfo.ratio	261		//Turned this off as the letter was going out of the banner
		262		//verticalCenter: planeText.verticalCenter
		263		bottomMargin: 10 * ApplicationInfo.ratio
		264		}
227	fontSize: hugeSize	265		fontSize: hugeSize
228	font.weight: Font.DemiBold	266		font.weight: Font.DemiBold
229	color: "#2a2a2a"	267		color: "#2a2a2a"
Context not available.