Hoist the actions overlay out of the frame loader.
ClosedPublic

Authored by hein on Apr 5 2017, 11:36 AM.

Details

Summary

This fixes a race condition causing the actions overlay to sometimes
appear in the drag pixmap attached to the DND cursor, as grabToImage
is run against the frame loader.

As a bonus it simplifies the code and save some cycles as it's no
longer necessary to turn the overlay column invisible during drags.

Test Plan

We used to have a problem with the actions overlay not
repositioning properly in popup Folder Views when switching them
between list and icon view mode, which partly shaped the old code
(the anchor state changes). I verified this didn't regress with the
new simplified code.

Diff Detail

Repository
R119 Plasma Desktop
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
hein created this revision.Apr 5 2017, 11:36 AM
Restricted Application added a project: Plasma. · View Herald TranscriptApr 5 2017, 11:36 AM
hein added a comment.Apr 5 2017, 11:39 AM

Hold on - I just found a small regression during further testing, this needs some more work.

hein updated this revision to Diff 13111.Apr 5 2017, 11:41 AM

Add back the visible property expression, it's still needed for
the spring-loading case after all.

broulik added inline comments.Apr 5 2017, 8:09 PM
containments/desktop/package/contents/ui/FolderItemDelegate.qml
455

Can you verify this (icon.x in conjunction with auto-mirroring left anchor) works fine in RTL mode?

459

Shouldn't be neccessary

hein updated this revision to Diff 13125.Apr 5 2017, 10:28 PM

Fix rtl.

hein updated this revision to Diff 13126.Apr 5 2017, 10:30 PM

Fix ltr :D

hein added a comment.Apr 6 2017, 10:02 PM

[10:14:07] <kbroulik> Sho_: listview mode in popup is broken in RTL in FolderView with your patch (didnt check if it's actually the patch but I recall it working)
[10:18:26] <Sho_> kbroulik: i saw that but i felt like i saw it in the past and it fixes itself if you keyboard nav so i chalked it up to the usual qt gridview rtl bugs
[10:18:46] <Sho_> are you sure it doesn't happen without the patch?
[10:20:38] <kbroulik> no, let me try
[10:21:06] <kbroulik> git stash'd my p-d and now it's working
[10:21:28] <kbroulik> git stash apply, now it's broken again

For me it happens with and without this patch, doesn't seem related.

broulik accepted this revision.Apr 12 2017, 12:05 PM

Indeed RTL seems broken with and without this patch..

This revision is now accepted and ready to land.Apr 12 2017, 12:05 PM
This revision was automatically updated to reflect the committed changes.