Merge lp:~sil2100/unity/launcher_dnd_flicker into lp:unity

Proposed by Łukasz Zemczak on 2012-04-08
Status: Superseded
Proposed branch: lp:~sil2100/unity/launcher_dnd_flicker
Merge into: lp:unity
Diff against target: 38 lines (+12/-5)
1 file modified
plugins/unityshell/src/Launcher.cpp (+12/-5)
To merge this branch: bzr merge lp:~sil2100/unity/launcher_dnd_flicker
Reviewer Review Type Date Requested Status
Alex Launi (community) quality 2012-04-08 Approve on 2012-05-02
Review via email: mp+101214@code.launchpad.net

This proposal has been superseded by a proposal from 2012-05-08.

Commit message

When dragging a non-application, do not saturate all icons automatically, but only those that are needed.

Eliminates flickering before 'prelighting' the Launcher icons that can handle the current drag-and-drop (LP #863230).

Description of the change

Problem description:

When drag-and-dropping from the Dash to the Launcher a non-application icon (for instance, a PNG file), the Launcher 'flickers' a bit. This flickering means that the Launcher icons get desaturated for a moment and then loose saturation again (as originally intended), tinted (LP #863230 and more specifically - LP #971086).

The fix:

During drag and drop, instead of saturating all icons automatically when overlay is present, we only do this when we know that we're not dragging a non-application. Otherwise, only saturate the icons that we will actually 'prelight' (i.e. indicate that can handle the given file). This way, there is no flickering and all icons visualize correctly, as intended. There are no longer any tinting problems as well (as noted by JohnLea).

Test coverage:

The following fix can be tested by manual testing of the specified bugs (LP #863230, LP #971086). Problem no longer occurs.

To post a comment you must log in.
Alex Launi (alexlauni) wrote :

Please add tests to the suite in manual-tests/

review: Needs Fixing (quality)
Łukasz Zemczak (sil2100) wrote :

There already seems to be a manual-test that theoretically tests the given behavior:
manual-tests/Dash.txt -> "Launcher drop target saturation test" (from the "Drag and drop tests" section).

Should I modify it more to add details regarding this bug? But it seems rather sufficient.

Alex Launi (alexlauni) wrote :

Good catch! Thanks. Yeah the current test is sufficient.

review: Approve (quality)
Unity Merger (unity-merger) wrote :

Attempt to merge into lp:unity failed due to conflicts:

text conflict in plugins/unityshell/src/Launcher.cpp

Unmerged revisions

2253. By Łukasz Zemczak on 2012-04-08

If dragging a non-application, do not saturate all icons automatically. Only those needed.

This way, the 'flickering' animation is not visible during drag-and-drop from Dash.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'plugins/unityshell/src/Launcher.cpp'
--- plugins/unityshell/src/Launcher.cpp 2012-04-06 11:19:18 +0000
+++ plugins/unityshell/src/Launcher.cpp 2012-04-08 22:29:19 +0000
@@ -2668,9 +2668,6 @@
26682668
2669 _hide_machine->SetQuirk(LauncherHideMachine::EXTERNAL_DND_ACTIVE, true);2669 _hide_machine->SetQuirk(LauncherHideMachine::EXTERNAL_DND_ACTIVE, true);
26702670
2671 if (IsOverlayOpen())
2672 SaturateIcons();
2673
2674 for (auto it : _dnd_data.Uris())2671 for (auto it : _dnd_data.Uris())
2675 {2672 {
2676 if (g_str_has_suffix(it.c_str(), ".desktop"))2673 if (g_str_has_suffix(it.c_str(), ".desktop"))
@@ -2684,12 +2681,22 @@
2684 {2681 {
2685 for (auto it : *_model)2682 for (auto it : *_model)
2686 {2683 {
2687 if (it->QueryAcceptDrop(_dnd_data) != nux::DNDACTION_NONE)2684 if (it->QueryAcceptDrop(_dnd_data) != nux::DNDACTION_NONE)
2685 {
2686 it->SetQuirk(AbstractLauncherIcon::QUIRK_DESAT, false);
2688 it->SetQuirk(AbstractLauncherIcon::QUIRK_DROP_PRELIGHT, true);2687 it->SetQuirk(AbstractLauncherIcon::QUIRK_DROP_PRELIGHT, true);
2689 else2688 }
2689 else
2690 {
2690 it->SetQuirk(AbstractLauncherIcon::QUIRK_DROP_DIM, true);2691 it->SetQuirk(AbstractLauncherIcon::QUIRK_DROP_DIM, true);
2692 }
2691 }2693 }
2692 }2694 }
2695 else
2696 {
2697 if (IsOverlayOpen())
2698 SaturateIcons();
2699 }
2693}2700}
26942701
2695void2702void