Unity

Merge lp://qastaging/~3v1n0/unity/indicators-redesign into lp://qastaging/unity

indicators-redesign
Merge into trunk

Proposed by Marco Trevisan (Treviño) on 2011-08-09

Status:

Merged

Merged at revision:

1521

Proposed branch:

lp://qastaging/~3v1n0/unity/indicators-redesign

Merge into:

lp://qastaging/unity

Diff against target:

2704 lines (+927/-481)

24 files modified

UnityCore/DBusIndicators.cpp (+49/-32)
UnityCore/Indicator.cpp (+55/-37)
UnityCore/Indicator.h (+7/-3)
UnityCore/IndicatorEntry.cpp (+55/-2)
UnityCore/IndicatorEntry.h (+14/-2)
UnityCore/Indicators.cpp (+58/-10)
UnityCore/Indicators.h (+7/-1)
plugins/unityshell/src/PanelIndicatorEntryView.cpp (+42/-33)
plugins/unityshell/src/PanelIndicatorEntryView.h (+7/-6)
plugins/unityshell/src/PanelIndicatorsView.cpp (+164/-87)
plugins/unityshell/src/PanelIndicatorsView.h (+33/-27)
plugins/unityshell/src/PanelMenuView.cpp (+14/-24)
plugins/unityshell/src/PanelMenuView.h (+6/-8)
plugins/unityshell/src/PanelTray.cpp (+8/-8)
plugins/unityshell/src/PanelTray.h (+3/-6)
plugins/unityshell/src/PanelView.cpp (+60/-53)
plugins/unityshell/src/PanelView.h (+6/-4)
services/panel-main.c (+23/-6)
services/panel-root-accessible.c (+1/-1)
services/panel-service.c (+278/-107)
services/panel-service.h (+6/-1)
tests/CMakeLists.txt (+4/-4)
tests/test_indicator_entry.cpp (+20/-16)
tests/unit/TestPanelService.cpp (+7/-3)

To merge this branch:

bzr merge lp://qastaging/~3v1n0/unity/indicators-redesign

High

Fix Released

Link a bug report

Reviewer	Date Requested	Status
Florian Boucault (community)	2011-09-07	Approve on 2011-09-08
Neil J. Patel (community)	2011-08-09	Approve on 2011-09-07
Review via email: mp+70818@code.qastaging.launchpad.net

Description of the change

To implement the Desktop DX Indicators Review blueprint [1] and to implement the system indicators ordering requested by design team [2], I've redesigned the way the indicators work in unity.

This need to change both unity-panel-service, UnityCore and unityshell, and basically it works by making the panel-service to compute a "priority" for each indicator entry and then making unityshell to show the indicator entries in the correct order (thanks to the nux feature I added recently lp:~3v1n0/nux/layout-child-position-set).

I also improved the way we synced the indicators, that now are only created when needed, updated otherwise and removed when asked.

I've wrote slightly more detailed informations about the implementation in the commit messages.

Potentially this would also easily allow to introduce a setting to control the indicator ordering over the panel (for advanced users, as requested in bug #537947).

Since the changes I did to UnityCore would affect also unity-2d I've also pushed a branch for getting it work in the proper way too at lp:~3v1n0/unity-2d/unity-core-indicators-revisited

[1] https://blueprints.launchpad.net/ubuntu/+spec/desktop-dx-o-indicators-review
[2] https://wiki.ubuntu.com/MenuBar#System_status_menus_.28system_indicators.29

PS: Unfortunately I will go in holiday on August 11th, so I hope you could review it quickly to allow me to fix the eventual issues.

Revision history for this message

Marco Trevisan (Treviño) (3v1n0) wrote on 2011-08-29:

Patch updated to work with recent Unity...

Revision history for this message

Neil J. Patel (njpatel) wrote on 2011-09-02:

Hey Marco, awesome work!

Some niggles I noticed:

- The indicator order needs to have the me menu next to the messaging menu (shows up next to the clock)
- Keynav (open menu and then press left/right, move the mouse first, though, there is a bug where the keynav breaks if a mouse is still over an indicator). It follows the order of the indicator objects, not of the priorities (the bluetooth and network menus confuse it).

Apart from that, the code looks really good, nice job!

review: Needs Fixing

Revision history for this message

Neil J. Patel (njpatel) wrote on 2011-09-02:

Ignore "- The indicator order needs to have the me menu next to the messaging menu (shows up next to the clock)" that doesn't make sense :)

Revision history for this message

Marco Trevisan (Treviño) (3v1n0) wrote on 2011-09-03:

Thanks for your review... Sorry I didn't notice that keynav bug, however I've fixed it now. Maybe I've not used the more efficent way (that I wrote, tough but that is too tricky to be maintained), but it works as expected.

Revision history for this message

Marco Trevisan (Treviño) (3v1n0) wrote on 2011-09-03:

Ah, I was just wondering... What about including also a gsettings option to the panel service to manage the indicators order?

Revision history for this message

Neil J. Patel (njpatel) wrote on 2011-09-05:

Macro, I compiled and ran the latest revision and had a few issues:

- PanelTray didn't compile without a bit of massaging
- unity-panel-service crashes as soon as unity starts up, but it crashes in g_main_context, so it's not a useful backtrace :(
- Unity crashes as soon as the panel service crashes, just after Sync()

I'm testing on a fully updated Oneiric.

Revision history for this message

Marco Trevisan (Treviño) (3v1n0) wrote on 2011-09-06:

Neil, sorry for these issues, maybe they were happening in the WIP version I uploaded, however the conflict with PanelTray should have been fixed, while I'm actually not experiencing crashes and closing unity-panel-service should be managed correctly.

With the latest changes, I've made unity to update when unity-panel-service crashes or get closed by removing the indicators (if the panel service is terminated or interrupted it will sync before closing, while if it's killed unity will remove its indicators). It seems to work fine here, but if your troubles will continue I can also revert the previous version.

Revision history for this message

Neil J. Patel (njpatel) wrote on 2011-09-07:

Works well for me now, including the fixes I requested. Thank you! I'm approving but adding a review request for Florian to make sure the Unity-2D side is good too. Thanks again, hopefully we can get this merged in soon!

review: Approve

Revision history for this message

Marco Trevisan (Treviño) (3v1n0) wrote on 2011-09-07:

Cool. Thanks.

Revision history for this message

Florian Boucault (fboucault) wrote on 2011-09-08:

Change looks good and works with Unity 2D.

review: Approve