Merge lp://qastaging/~unity-2d-team/unity-2d/launcher-layout into lp://qastaging/unity-2d/3.0

The launcher width is only 65 pixels in the mockups, not 66.

launcher/UnityApplications/iconimageprovider.cpp:

QDebug include is in the middle of the file.

Fixed the QDebug. Regarding the 66 vs. 65 pixels: it's 66 in the current natty implementation, 65 in the mockup. I didn't fix it for now, and you will figure it out with Otto later as agreed.

=== modified file 'launcher/Launcher.qml'

@@ -44,11 +53,7 @@
                     list.visibleMenu.hide()
                 }
                 list.visibleMenu = item.menu
-
- /* The menu needs to never overlap with the MouseArea of
- item otherwise flickering happens when the mouse is on
- an overlapping pixel (hence the -4). */
- item.menu.show(width-4, y+height/2-list.contentY+panel.y)
+ item.menu.show(width, y + height / 2 - list.contentY + panel.y)

The contextual menu's vertical position is slightly off, it should be around 3 pixels below. The point of the arrow of the contextual menu should be at the same height as the point of the arrow showing that the application is focused.

Can you add an unexpected change to that merge request? The width of the ubuntu circle of friends button in the panel has to coincide with the width of the launcher so that the line of the button and the border line of the launcher match perfectly and form only one continuous vertical line.

Thanks.

=== modified file 'launcher/LauncherItem.qml'

+ It is tile composed by a colored background layer, an icon (with 'icon' as source),
+ and a shine layer on top.

does not make sense.

=== modified file 'launcher/UnityApplications/launcherapplication.cpp'

@@ -394,6 +404,8 @@
     }
     if (m_has_visible_window != prev)
         emit hasVisibleWindowChanged(m_has_visible_window);
+
+ Q_EMIT windowCountChanged(windowCount());
 }

This is a hack taking advantage of the fact that LauncherApplication::updateHasVisibleWindow is called at the right time.

Instead you need to define a slot that will do Q_EMIT windowCountChanged(windowCount()); and connect it to BamfApplication::WindowAdded and BamfApplication::WindowRemoved signals.

> === modified file 'launcher/UnityApplications/launcherapplication.cpp'
>
> @@ -394,6 +404,8 @@
> }
> if (m_has_visible_window != prev)
> emit hasVisibleWindowChanged(m_has_visible_window);
> +
> + Q_EMIT windowCountChanged(windowCount());
> }
>
>
> This is a hack taking advantage of the fact that
> LauncherApplication::updateHasVisibleWindow is called at the right time.
>
> Instead you need to define a slot that will do Q_EMIT
> windowCountChanged(windowCount()); and connect it to
> BamfApplication::WindowAdded and BamfApplication::WindowRemoved signals.

I suggest calling the slot 'updateWindowCount'.

=== modified file 'launcher/app/launcherview.cpp'

@@ -32,6 +32,9 @@
 #include <X11/Xlib.h>
 #include <X11/Xatom.h>

+#define MAX(a,b) ((a >= b) ? a : b)
+#define MIN(a,b) ((a <= b) ? a : b)
+

Remove and replace with Qt's qMax and qMin.

(http://doc.qt.nokia.com/latest/qtglobal.html#qMax)

=== modified file 'launcher/app/launcher.cpp'
--- launcher/app/launcher.cpp 2011-02-08 10:41:55 +0000
+++ launcher/app/launcher.cpp 2011-02-11 11:57:46 +0000
@@ -76,12 +76,15 @@
     launcherView->setFocus();

     launcherView->engine()->addImportPath(unity2dImportPath());
+ launcherView->engine()->addImportPath(unity2dDirectory() + "/libunity-2d-private/");

This should _only_ be done when running un-installed. Please take example from places/app/places.cpp:

    if (!isRunningInstalled()) {
        [...]
        /* Place.qml imports Unity2d */
        view.engine()->addImportPath(unity2dDirectory() + "/libunity-2d-private/");
    }

=== modified file 'launcher/app/launcher.cpp'

     launcherView->rootContext()->setContextProperty("launcherView", launcherView);
     launcherView->rootContext()->setContextProperty("panel", &panel);
+ launcherView->rootContext()->setContextProperty("engineBaseUrl",
+ launcherView->engine()->baseUrl().toLocalFile());

This looks like a workaround for the lack of support of images in path relative to the launcher binary in BlendedImageProvider. Let's document it quickly for now.

=== added file 'launcher/artwork/launcher_arrow_ltr.png'
=== added file 'launcher/artwork/launcher_arrow_rtl.png'
=== added file 'launcher/artwork/launcher_pip_ltr.png'

No abbreviations. Replace 'ltr' with 'left' and 'rtl' with 'right'.

I sometimes get a WARNING (hopefully not important):

LauncherItem.qml:89 Unable to assign undefined value

=== modified file 'libunity-2d-private/Unity2d/blendedimageprovider.cpp'

        qWarning() << "BlendedImageProvider: faile to match id:" << id;
should read
        qWarning() << "BlendedImageProvider: failed to match id:" << id;

"[...] when a sting isn't an SVG color name"
should read
"[...] when a string is not a SVG color name (eg. "red", "yellow", etc.)

Only 'launcher/LauncherItem.qml' left to do and the review should be complete.

Fixed pretty much everything, except the renaming of the icons which stay as they are since they were copied from Unity.

Also please check my fix in commit 391 for the following warning:
"LauncherItem.qml:89 Unable to assign undefined value"
and let me know if you have a better workaround.

Sounds good to me, ship that good job!