Merge lp://staging/~shippo/plank/fix-1251625 into lp://staging/plank

Proposed by Peter Feichtinger
Status: Needs review
Proposed branch: lp://staging/~shippo/plank/fix-1251625
Merge into: lp://staging/plank
Diff against target: 523 lines (+177/-62)
15 files modified
data/net.launchpad.plank.gschema.xml.in.in (+5/-0)
data/ui/preferences.ui (+24/-0)
docklets/Clippy/ClippyDockItem.vala (+1/-1)
docklets/Clock/ClockDockItem.vala (+1/-1)
docklets/Trash/TrashDockItem.vala (+1/-1)
lib/DockPreferences.vala (+3/-0)
lib/Items/ApplicationDockItem.vala (+77/-44)
lib/Items/DockElement.vala (+3/-6)
lib/Items/DockItem.vala (+12/-0)
lib/Items/FileDockItem.vala (+10/-3)
lib/Items/PlankDockItem.vala (+1/-1)
lib/Services/WindowControl.vala (+20/-0)
lib/Widgets/DockWindow.vala (+3/-3)
lib/Widgets/PreferencesWindow.vala (+13/-0)
lib/libplank.symbols (+3/-2)
To merge this branch: bzr merge lp://staging/~shippo/plank/fix-1251625
Reviewer Review Type Date Requested Status
Peter Feichtinger (community) Needs Resubmitting
Rico Tzschichholz Needs Information
Review via email: mp+317909@code.staging.launchpad.net

Description of the change

This fixes Bug #1251625 (partially).

There are two related changes in this branch which could be merged individually, but I think they belong together:
 * First, there is now a new preference setting that, when enabled, causes application items to show their menu when left clicked, instead of bringing all windows up. This way a window to be brought up can be selected from the menu without having to right click.
 * Second, the application item menu is changed to indicate the currently active window (in bold font) and allow it to be minimized by selecting it, and to indicate minimized windows by dimming their items slightly.

There is no window preview as requested in the original bug report, but I think this change makes the dock way more usable (and doesn't change the default behavior when disabled, aside from the new menu item styles).

To post a comment you must log in.
Revision history for this message
Rico Tzschichholz (ricotz) wrote :

Overall this looks good, besides some minor code-style issues.

I would prefer if the special left-click menu would *only* contain the window-listing.

Also making it *2* commits is sufficient:
 * the menuitem-style change
 * the new menu including its requirements

- don't be lazy with variable names like "lbl" -> "label"
- prefix connectors in if-conditions in case of line-breaks

review: Needs Information
Revision history for this message
Peter Feichtinger (shippo) wrote :

Hm, I don't know how to merge several commits.
Should I just revert the branch and make new commits, or is there some clever way to achieve that?

You are right concerning the menu, I will try and exclude the other entries.
And I just realized that I will then need new commits anyway :-)

As for the coding style, I followed what was already there as closely as possible.
If you want me to break consistency I will, but I'd rather not.

Revision history for this message
Peter Feichtinger (shippo) wrote :

Never mind about the coding style, I see what you mean.

Revision history for this message
Peter Feichtinger (shippo) wrote :

So, I changed the implementation but now my local branch and the one on here have diverged, because I uncommitted the changed in my local branch before making the requested changes.
How do I fix this, should I just force push (by passing --override, I don't know if this will work anyway), or is there a smarter move?

Revision history for this message
Rico Tzschichholz (ricotz) wrote :

Yes, you need to use "bzr push --overwrite lp:~shippo/plank/fix-1251625" to push your altered branch.

1589. By Peter Feichtinger

Add support for switching windows when an application item is clicked.

* It should be possible for a DockItem to show a menu based on the
  current number of open windows, so a new method in DockItem is used in
  DockWindow to check whether an item wants to show a menu when any
  button is pressed.
* ApplicationDockItem is changed to show the window menu on left click,
  when more than one window is open for the application.
* A new preference switch for using the window menu is added: when
  'Use Window Menu' is enabled, left clicking on an item with multiple
  windows will open the item menu, allowing selection of only one window
  to show. When disabled, clicking has the default behavior.

1590. By Peter Feichtinger

Add indication for active and minimized windows and allow mimizing.

This adds an indication in the window menu for the currently active
window, as well as for minimized windows. The active window is minimized
when selected.

Revision history for this message
Peter Feichtinger (shippo) wrote :

Ooh nice, launchpad already put my changed branch here :-)

So, I implemented the changes you requested:
* The menu shown when multiple windows are open only contains the windows. This is done by passing the button and modifiers to get_menu_items, which presumably breaks ABI compatibility.
* I double checked that I followed the coding style, I hope this time I really did ;-)

1591. By Peter Feichtinger

Fix window menu showing with only one item.

Revision history for this message
Peter Feichtinger (shippo) wrote :

Do I need to resubmit?

review: Needs Resubmitting
Revision history for this message
Rico Tzschichholz (ricotz) wrote :

Sorry, I still have not get around to take a closer look at this. So no need for action from your side yet.

Unmerged revisions

1591. By Peter Feichtinger

Fix window menu showing with only one item.

1590. By Peter Feichtinger

Add indication for active and minimized windows and allow mimizing.

This adds an indication in the window menu for the currently active
window, as well as for minimized windows. The active window is minimized
when selected.

1589. By Peter Feichtinger

Add support for switching windows when an application item is clicked.

* It should be possible for a DockItem to show a menu based on the
  current number of open windows, so a new method in DockItem is used in
  DockWindow to check whether an item wants to show a menu when any
  button is pressed.
* ApplicationDockItem is changed to show the window menu on left click,
  when more than one window is open for the application.
* A new preference switch for using the window menu is added: when
  'Use Window Menu' is enabled, left clicking on an item with multiple
  windows will open the item menu, allowing selection of only one window
  to show. When disabled, clicking has the default behavior.

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

to status/vote changes: