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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.