D26637: balance scales multiple dataset

[Johnny Jazeix]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20200211/c225aa7a/attachment-0001.html>