D17861: Prevent instructions from disappearing

[Johnny Jazeix]

jjazeix added inline comments.

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.

REPOSITORY
  R2 GCompris

REVISION DETAIL
  https://phabricator.kde.org/D17861

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>