sailfishos / sailfish-office

Sailfish Office
GNU General Public License v2.0
73 stars 26 forks source link

[office] Correct a blicking artefact happening on a link context menu after back button was pressed. #98

Closed dcaliste closed 8 years ago

dcaliste commented 8 years ago

I found a corner case when opening a context menu make the contentY change quickly to an offset position before returning to normal.

To reproduce:

The root of this is very hidden. It's the _flickableContentY property of the ContextMenu that triggers this. Following the previous steps:

Beside updating ContextMenu to ensure that _flickableConentYAtOpen is set to -1 or any non significant value, when hide() is called, I think it's safer to detach any menu from the ContextMenuHook when the hook is set to a different target. This is the purpose of this PR. examplePDF.pdf

pvuorela commented 8 years ago

Current version: click on top link to open menu, rotate the device -> menu remains open

This version: menu disappears and this gets output:

[W] unknown:66 - file:///usr/lib/qt5/qml/Sailfish/Office/ContextMenuHook.qml:66: TypeError: Cannot read property '_flickable' of null

dcaliste commented 8 years ago

That's right, I forgot that changing orientation changes the clickedBox in PDFView, so resetting the target…

I've moved the _menu._flickable = null into the on_OpenedChanged callback and removed the _menu = null to avoid cycle in setting _opened. The setTarget() function is not necessary anymore, but I suggest to keep it anyway, because it gives more sense IMHO when reading the code than setting two properties.

Seeing all this, I'm wondering if this possible to setup automatic tests for UI program and QML ? Having something like load PDF, emulate tap at (x,y) check that menu is opened… That would be great, especially, I'm currently fighting with the title proposition of the design team, adding a header component to PDFView.qml (see my title branch, it's almost working). It has created a mess because we rely a lot on contentY being linearly modified by page rotation, while now with header, contentY is not modified linearly anymore, but modified linearly plus a constant.

pvuorela commented 8 years ago

Looks good and works now better

Seeing all this, I'm wondering if this possible to setup automatic tests for UI program and QML

There's Qt Quick Test & qmltestrunner. Could be of help: https://publications.theseus.fi/bitstream/handle/10024/98894/Johansson_Lauri.pdf?sequence=1

dcaliste commented 8 years ago

There's Qt Quick Test & qmltestrunner.

Very interesting, thank you, I'll try to prepare something for the PDF UI in the coming weeks.