Unity

Merge lp://qastaging/~dylanmccall/unity/panel-drag-frontmost-maximized into lp://qastaging/unity

  1. panel-drag-frontmost-maximized
  2. Merge into trunk
Proposed by Dylan McCall
Status: Merged
Merged at revision: 992
Proposed branch: lp://qastaging/~dylanmccall/unity/panel-drag-frontmost-maximized
Merge into: lp://qastaging/unity
Diff against target: 423 lines (+180/-44)
8 files modified
src/PanelMenuView.cpp (+78/-42)
src/PanelMenuView.h (+5/-0)
src/PanelTitlebarGrabAreaView.cpp (+6/-0)
src/PanelTitlebarGrabAreaView.h (+1/-0)
src/PluginAdapter.cpp (+60/-0)
src/PluginAdapter.h (+4/-0)
src/WindowManager.cpp (+21/-1)
src/WindowManager.h (+5/-1)
To merge this branch: bzr merge lp://qastaging/~dylanmccall/unity/panel-drag-frontmost-maximized
Reviewer Review Type Date Requested Status
Didier Roche-Tolomelli Approve
Review via email: mp+53552@code.qastaging.launchpad.net

Description of the change

This branch changes the mechanics for the top panel acting as a titlebar. Previously, the top panel could be dragged in order to unmaximize (and move) a maximized window, but only if that window was in focus. This opened up some issues, which are covered in bug #716177.

With this branch, Unity's PanelMenuView maintains a list of maximized windows. (Note: When I say “maximized window,” I really mean “a window that Unity has started decorating instead of the window decorator.” Right now, that is limited to maximized windows).

When the user clicks the top panel, that click affects the front-most of the PanelMenuView's maximized windows. Previously, OnMaximizedGrab would only affect the active window if it was maximized.
I also added some code to Activate the front-most maximized window, bringing it into focus when the top panel is clicked.

There is some intricacy involving multiple viewports and desktops, where the top panel should never act as a titlebar for a window that is not visible (be it behind another maximized window or on a different viewport). I believe I have covered the appropriate cases in WindowManager::IsWindowOnCurrentDesktop and WindowManager::IsWindowObscured.

To post a comment you must log in.
Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

Thanks a lot of your work there and to tackle this usability issue. Your work is really appreciated there.

However, I prefer first to continue the discussion on the bug report with the design team so that we can decide how to deal effectively with this case. (like the menuproxy which becomes "weird" with your path)

Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

Put in needs information for now (but from the bug report

review: Needs Information
Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

Dylan, can you check https://bugs.launchpad.net/ayatana-design/+bug/716177/comments/10 ? I think that you fix mostly fit with bug #723882 which is the desired behavior for natty, isn't it?

Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

Are you taking into account the double clicking and single click from the bug report? Probably, but just want to confirm before giving a deeper look ;)

As well, our compiz wizard told me that we should'nt use window->onCurrentDesktop (), only the second part of the statement is reliable, so you can nuke the first part, can you make that change? :)

Revision history for this message
Dylan McCall (dylanmccall) wrote :

Hi Didier!

Yep, it raises the window in a single click as asked in the bug report. It also magically did double clicking, and I'm pushing a new version that makes it explicit instead of something that just happened to work.

There's another bug I noticed, where the double click event is emitted regardless of which button is pressed. I'm going to deal with that in another bug report. (You might notice it testing this, though).

I'm not quite clear on what I should do about onCurrentDesktop. Do you mean we're fine just checking if it's on the current Viewport? Same with comparing window->desktop () under IsWindowObscured?
(Weirdly, I can't find a way to switch Compiz _desktops_ in Unity so I don't know how to test that in the first place).

Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

On onCurrentDesktop, just keep window_vp == m_Screen->vp () the first test is broken (compiz just support well viewport, the virtual desktop implementation is broken as told sam).
You can sneak the double click fix in this merge, that should be easy (button == 1), you have some examples in launcher.cpp if needed.

So, once that is done, it seems that everything looks ready for merging! ;)

Revision history for this message
Dylan McCall (dylanmccall) wrote :

Okay, those new commits should cover it. I also (think I) fixed the issue I reported in bug #736580, since I change a bit of code in that area anyway and merge conflicts suck. And it's my change's fault for making it obvious :b

Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

Thanks a lot Dylan, will merge your awesome work now :)

Just a note though: bug #736491: Panel Titlebar double click is emitted for any mouse button doesn't seem to be fixed (can double click right and see the issue quite obviously). Apart from that, setting as fix committed and merging your branch :)

No more false positive on maximizing/reducing an application thanks to your work. Woooow :)

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.