D17861: Prevent instructions from disappearing

Johnny Jazeix noreply at phabricator.kde.org
Thu Jan 10 21:35:59 GMT 2019

jjazeix added inline comments.


> Share.qml:230
> +            function toggleElide() {
> +                if(instruction.widgetVisible) {
> +                    grid.opacity = 0

you should use bindings instead of assign variables, it's more efficient and clear

> Share.qml:234
> +                    instruction.widgetVisible = false
> +                    instruction.height = instructionTxt.contentHeight
> +                } else {

This is not needed and cause issues.
The height is already binded to the contentHeight. By assigning it here, you lose the binding.
When you resize the screen, the height won't be updated anymore as the binding is lost.

> share.js:107
> +        items.instruction.widgetVisible = false
> +        items.instruction.toggleElide()
>          items.background.showCount = false

the items.instruction.widgetVisible is updated just above but the toggleElide also updates it, it's not easy to debug, variables shouldn't be assigned twice in a row in opposite values, it's prone to misunderstanding.

  R2 GCompris


To: AkshayCHD, #gcompris_improvements
Cc: echarruau, jjazeix, kde-edu, #gcompris_improvements, harrymecwan, ganeshredcobra, nityanandkumar, rahulyadav, narvaez, scagarwal, apol, timotheegiet, hkaelberer, bcoudoin
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20190110/f4fc431f/attachment.html>

More information about the kde-edu mailing list