Reposition score box in hangman activity(ref: T7777)
ClosedPublic

Authored by rohitdas on Jan 21 2018, 8:10 AM.

Details

Reviewers
jjazeix
Group Reviewers
GCompris: Improvements
Maniphest Tasks
T7777: Hangman, reposition score box
Summary

Repositioning score box in hangman activity to prevent it from overriding other UI components.

Diff Detail

Repository
R2 GCompris
Lint
Lint Skipped
Unit
Unit Tests Skipped
rohitdas created this revision.Jan 21 2018, 8:10 AM
Restricted Application added a project: KDE Edu. · View Herald TranscriptJan 21 2018, 8:10 AM
Restricted Application added a subscriber: KDE Edu. · View Herald Transcript
rohitdas requested review of this revision.Jan 21 2018, 8:10 AM
rohitdas retitled this revision from Reposition score box in hangman activity to Reposition score box in hangman activity(ref: T7777).
jjazeix edited reviewers, added: GCompris: Improvements; removed: asagtani.Jan 21 2018, 7:51 PM

There are a lot of elements on this activity that can make repositioning its components difficult: https://pasteboard.co/H3ZSP7v.png
I think it would be better to reduce a bit the size of the OK button, same for score (maybe something like: width: internalTextComponent.width * 1.5 and height: internalTextComponentheight * 1.2, it's to be tested, I didn't checked if it rendered well).
And setting the score at the place of the OK button and the OK button above it.

src/activities/hangman/Hangman.qml
86

why did you change this? variant is deprecated and should not be used

Oh, sorry about the variant. The branch wasn't updated in a long time. Will change that. Also I could't test the keyboard because it doesn't appear on a computer. Thanks for the screenshot.

rohitdas updated this revision to Diff 25734.Jan 22 2018, 4:45 AM

Changes made as requested. Please review.

https://pasteboard.co/H5XzED5.png
The ok button may not even be visible when you reduce the height.
Configuration -> Virtual Keyboard to make the keyboard visible (even on desktop)

Should I put the OK button beside the score box? This way, it won't disappear or override any other elements, like the guessedText element, when the device height is reduced.

Besides should also cause overlapping issues.
My first comment was to do what you did but having all elements visible. Basically, you can consider: a top row with guessed text, a left column with the flower and a right column with ok+score and below is the bar+keyboard. The text should have all the remaining place.

rohitdas updated this revision to Diff 26587.Feb 5 2018, 2:55 PM

The position of OK button is changed wrt the virtual keyboard. Please review.

It's almost good!
https://pasteboard.co/H7cjARY.png as you can see the underscore may override the score. Can you take a look to make it not happen? I guess you can use Text.Fit for GCText "hidden" and "guessedText"

rohitdas updated this revision to Diff 26997.Feb 12 2018, 2:12 PM

Please review and let me know if the changes were as intended.

jjazeix added inline comments.Feb 12 2018, 4:48 PM
src/activities/hangman/Hangman.qml
122

please read the documentation first before doing changes: https://doc.qt.io/qt-5/qml-qtquick-text.html

rohitdas updated this revision to Diff 27044.Feb 13 2018, 10:36 AM

Please review.

asagtani added inline comments.Feb 13 2018, 7:37 PM
src/activities/hangman/Hangman.qml
370

ok button is not visible even after setting it to true, can you please check it?

rohitdas updated this revision to Diff 27158.Feb 14 2018, 3:22 PM

To view the OK button, you have to either enter the correct text to win, or exhaust your chances to lose, after which the OK button will show up above the scoreBox. Please review.

jjazeix accepted this revision.Feb 18 2018, 9:02 PM

I did fixes and I commited in https://commits.kde.org/gcompris/6a66cd44106015bd369616574d656a8dd19d15be
Please check them.
Basically, I fixed the overriding between the text to find (when letters are represented by underscores) and the score.
I changed the OK button size to a fixed size not relative to the screen width.
I also reduced the width of the score, no need to have it too big.

This revision is now accepted and ready to land.Feb 18 2018, 9:02 PM
jjazeix closed this revision.Feb 18 2018, 9:02 PM