unity-2d

Merge lp://qastaging/~mardy/unity-2d/unmaximize into lp://qastaging/unity-2d

  1. unmaximize
  2. Merge into trunk
Proposed by Alberto Mardegan
Status: Superseded
Proposed branch: lp://qastaging/~mardy/unity-2d/unmaximize
Merge into: lp://qastaging/unity-2d
Prerequisite: lp://qastaging/~mardy/unity-2d/spread-geometry
Diff against target: 679 lines (+351/-53)
12 files modified
libunity-2d-private/Unity2d/plugin.cpp (+5/-0)
libunity-2d-private/src/CMakeLists.txt (+1/-0)
libunity-2d-private/src/dashsettings.cpp (+117/-0)
libunity-2d-private/src/dashsettings.h (+72/-0)
libunity-2d-private/src/panelstyle.cpp (+6/-0)
libunity-2d-private/src/panelstyle.h (+2/-1)
panel/applets/appname/appnameapplet.cpp (+36/-10)
panel/applets/appname/windowhelper.cpp (+93/-3)
panel/applets/appname/windowhelper.h (+4/-0)
places/app/dashdeclarativeview.cpp (+3/-15)
places/app/places.cpp (+1/-0)
places/dash.qml (+11/-24)
To merge this branch: bzr merge lp://qastaging/~mardy/unity-2d/unmaximize
Reviewer Review Type Date Requested Status
Lohith D Shivamurthy (community) Approve
Michał Sawicz functional Approve
Review via email: mp+78214@code.qastaging.launchpad.net

This proposal has been superseded by a proposal from 2011-11-29.

Commit message

[dash, panel] Add window controls to dash window

Show window controls for closing, minimizing and maximizing the Dash.

Description of the change

  [dash, panel] Add window controls to dash window

  Show window controls for closing, minimizing and maximizing the Dash.

One potentially big issue:

Although the Dash window state (maximized/unmaximized) is set in DConf, at the next startup the Dash ignores that settings: this is because the Dash itself determines what window mode is best right after it's started. We need to clarify with the UI designers whether this behaviour is the intended one.

To post a comment you must log in.
Revision history for this message
Michał Sawicz (saviq) wrote :

Works fine here. Only thing I'm not sure of - what's the purpose of minimizing the Dash?

review: Approve (functional)
Revision history for this message
Michał Sawicz (saviq) wrote :

> Works fine here. Only thing I'm not sure of - what's the purpose of minimizing
> the Dash?
Talking to @fboucault, the minimize button should be disabled / greyed out for the dash.

review: Needs Fixing
Revision history for this message
Alberto Mardegan (mardy) wrote :

I will change the minimize button not to do anything when pressed, and remove the highlight effect when hovering it.
However, I'm not sure it's the good solution, seeing bug 867910 and bug 862775.

Revision history for this message
Alberto Mardegan (mardy) wrote :

Merge request updated: the minimize button is now insensitive.

Revision history for this message
Michał Sawicz (saviq) wrote :

I agree with the bugs mentioned - the minimize button looks out-of-place - broken, not inactive, with that change. Unless we have a "inactive" state of the buttons elsewhere in the desktop - which we don't - we should simply remove the minimize button. Modal windows, for example, only have the close button, not all of them, but with only the close one working.

That will probably require ACK from design.

Revision history for this message
Michał Sawicz (saviq) wrote :

When we're always fullscreen (Netbook form factor?), the unmaximize button should also be disabled.

Also, when the dash or alt+f2 is active, panel still displays the "Desktop" label.

review: Needs Fixing
Revision history for this message
Alberto Mardegan (mardy) wrote :

Updated:
- Don't show desktop title when Dash is running
- Disable the maximize/unmaximize button when the screen resolution is too low

Revision history for this message
Michał Sawicz (saviq) wrote :

Functionally works great, will have to take a longer look at the code, unless someone else wants to do that :]

review: Approve (functional)
Revision history for this message
Lohith D Shivamurthy (dyams) wrote :

> Functionally works great, will have to take a longer look at the code, unless
> someone else wants to do that :]

I finished reviewing this MR. IMO, We should merge it already.
I have noticed a functional issue though.
But this patch itself is worth more than that small issue. :)

I can log a separate lp bug for that issue later.

Revision history for this message
Lohith D Shivamurthy (dyams) wrote :

The issue is :
1) Open Dash
2) Make sure its window buttons are not visible.
3) Move the mouse over panel
4) Window buttons(Close, Minimize, Maximize) not visible yet
5) Move the mouse away from panel
6) The Window buttons becomes visible now.

Expected:
at (4) the buttons should have been visible and
at (6) they should have been hidden.

Actual:
Moving the mouse over panel when window buttons are not visible, the buttons are still hidden.
When cursor is moved away, they become visible.

Revision history for this message
Alberto Mardegan (mardy) wrote :

I'm rather sure that the issue your found was not present when I tested this -- it's a rather big issue that wouldn't pass unnoticed.
However, unfortunately I won't be able to work on this either. :-(

Revision history for this message
Lohith D Shivamurthy (dyams) wrote :

The code is neat and clean. Happy to approve it.
Thanks a lot.

review: Approve
Revision history for this message
Gerry Boland (gerboland) wrote :

Sorry, but make check not passing yet, so merge will fail.

Revision history for this message
Unity Merger (unity-merger) wrote :

No proposals found for merge of lp:~mardy/unity-2d/spread-geometry into lp:unity-2d.

Unmerged revisions

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.

Subscribers

People subscribed via source and target branches