D26637: balance scales multiple dataset
Johnny Jazeix <[email protected]>
| Newsgroups | gmane.comp.kde.education |
|---|---|
| Message-ID | <[email protected]> |
jjazeix added inline comments.
INLINE COMMENTS
> Scalesboard.qml:241
> width: parent.width - y
> - text: items.dataset[bar.level - 1].question && background.scaleHeight == 0 ?
> - items.dataset[bar.level - 1].question : ""
> + text: items.question.text && background.scaleHeight == 0 ?
> + items.question.text : ""
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?
> Scalesboard.qml:244
> answer: items.giftWeight
> + visible: background.scaleHeight === 0 ? true : false
>
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
> Scalesboard.qml:246
>
> - property bool hasText: items.dataset[bar.level - 1].question ? true : false
> + property bool hasText: items.question.text !== "" ? true : false
> + }
Same as above, can it be replaced with the "displayed" property?
> Scalesboard.qml:334
> + }
> + else if(question.displayed && background.scaleHeight === 0) {
> + numpad.updateAnswer(event.key, true);
can you use a better property than "background.scaleHeight === 0"? If you just read the code, you don't understand where this comes from.
REPOSITORY
R2 GCompris
REVISION DETAIL
https://phabricator.kde.org/D26637
To: dekumar, #gcompris_improvements, jjazeix, echarruau, timotheegiet
Cc: kde-edu, narvaez, apol