Merge lp:~damiannohales/plank/window-border-margin into lp:plank

Proposed by Damián Nohales
Status: Needs review
Proposed branch: lp:~damiannohales/plank/window-border-margin
Merge into: lp:plank
Diff against target: 112 lines (+25/-4)
4 files modified
lib/DockRenderer.vala (+1/-0)
lib/Drawing/DockTheme.vala (+9/-0)
lib/PositionManager.vala (+9/-4)
lib/Widgets/DockWindow.vala (+6/-0)
To merge this branch: bzr merge lp:~damiannohales/plank/window-border-margin
Reviewer Review Type Date Requested Status
Rico Tzschichholz Needs Fixing
Review via email: mp+223290@code.launchpad.net

Description of the change

This adds the WindowBorderMargin setting to the dock, that allows to the user to add an extra spacing between the window border and the dock.

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

This sounds reasonable if a theme defines a negative TopPadding. Considering that this should be a parameter in *dock.theme* not in settings. ;-)

So please move it to DockTheme as TopMargin. You should be able to contain the needed changes to DockTheme.vala, DockPreferences.vala and PositionManager.vala.

review: Needs Fixing
Revision history for this message
Damián Nohales (damiannohales) wrote :

> This sounds reasonable if a theme defines a negative TopPadding. Considering
> that this should be a parameter in *dock.theme* not in settings. ;-)
>
> So please move it to DockTheme as TopMargin. You should be able to contain the
> needed changes to DockTheme.vala, DockPreferences.vala and
> PositionManager.vala.

You are describing my first try when implementing this :P

I changed it to a setting because the plank settings also defines "outside" alignment stuff, but is ok, let's move it to the theme.

Revision history for this message
Damián Nohales (damiannohales) wrote :

I've overwritten the branch with the fixed commit.

1034. By Rico Tzschichholz

positionmanager: Precalculate top_offset to have it include TopPadding

This avoids doing this over and over again in several places.

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

Do not trigger setting the struts unconditionally.

TopMargin should taken into account as follow:
* only in case of HideMode==HideType.NONE
* ignore it for non-compositing, so there is no "spacing"
  if the (solid) background covers the item-regions
* don't have it influence the cursor-region or item-regions

Please rebase it on the current trunk.

review: Needs Fixing
1035. By Rico Tzschichholz

Make sure to update visibile-items in prepare

Not pinned items doesnt show up initially.

Regression caused by r1029

Revision history for this message
Damián Nohales (damiannohales) wrote :

> Do not trigger setting the struts unconditionally.
>
> TopMargin should taken into account as follow:
> * only in case of HideMode==HideType.NONE
> * ignore it for non-compositing, so there is no "spacing"
> if the (solid) background covers the item-regions
> * don't have it influence the cursor-region or item-regions
>
> Please rebase it on the current trunk.

I think a don't understand the 3rd point, could you explain?

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

I mean this margin should not change the size of areas which are used to determine whether the mouse cursor interacts with the dock or not. This doesn't mean it isn't needed to calculate them.

1036. By Damián Nohales

Add TopMargin theme option

This option allows to add an extra margin between
the window border and the top of the dock.

Revision history for this message
Damián Nohales (damiannohales) wrote :

Updated, but I still don't understand how this can affect the calculation of the cursor/items region, because the TopMargin parameter is only used in PositionManager::get_struts method.

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

I am not speaking about your current implementation in specific. Putting things directly in the struts-calcution is elegant, but this might change if you follow the other requirements.

Unmerged revisions

1036. By Damián Nohales

Add TopMargin theme option

This option allows to add an extra margin between
the window border and the top of the dock.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/DockRenderer.vala'
2--- lib/DockRenderer.vala 2014-05-20 15:36:44 +0000
3+++ lib/DockRenderer.vala 2014-06-26 15:05:18 +0000
4@@ -161,6 +161,7 @@
5 reset_position_manager_timer = 0;
6 controller.position_manager.reset_caches (theme);
7 controller.position_manager.update_regions ();
8+ controller.window.update_struts ();
9
10 return false;
11 });
12
13=== modified file 'lib/Drawing/DockTheme.vala'
14--- lib/Drawing/DockTheme.vala 2013-10-07 10:25:43 +0000
15+++ lib/Drawing/DockTheme.vala 2014-06-26 15:05:18 +0000
16@@ -40,6 +40,9 @@
17
18 [Description(nick = "bottom-padding", blurb = "The padding on the bottom dock edge, in tenths of a percent of IconSize.")]
19 public double BottomPadding { get; set; }
20+
21+ [Description(nick = "top-margin", blurb = "The distance between the window border and the dock, in tenths of a percent of IconSize.")]
22+ public double TopMargin { get; set; }
23
24 [Description(nick = "item-padding", blurb = "The padding between items on the dock, in tenths of a percent of IconSize.")]
25 public double ItemPadding { get; set; }
26@@ -120,6 +123,7 @@
27 HorizPadding = 0.0;
28 TopPadding = -11.0;
29 BottomPadding = 2.5;
30+ TopMargin = 0.0;
31 ItemPadding = 2.0;
32 IndicatorSize = 5.0;
33 IconShadowSize = 1.0;
34@@ -542,6 +546,11 @@
35 if (BottomPadding < 0)
36 BottomPadding = 0;
37 break;
38+
39+ case "TopMargin":
40+ if (TopMargin < 0.0)
41+ TopMargin = 0.0;
42+ break;
43
44 case "ItemPadding":
45 if (ItemPadding < 0)
46
47=== modified file 'lib/PositionManager.vala'
48--- lib/PositionManager.vala 2014-06-17 07:25:12 +0000
49+++ lib/PositionManager.vala 2014-06-26 15:05:18 +0000
50@@ -213,6 +213,10 @@
51 */
52 public int BottomPadding { get; private set; }
53 /**
54+ * Theme-based top margin, scaled by icon size.
55+ */
56+ public int TopMargin { get; private set; }
57+ /**
58 * Theme-based item padding, scaled by icon size.
59 */
60 public int ItemPadding { get; private set; }
61@@ -308,6 +312,7 @@
62 HorizPadding = (int) (theme.HorizPadding * scaled_icon_size);
63 TopPadding = (int) (theme.TopPadding * scaled_icon_size);
64 BottomPadding = (int) (theme.BottomPadding * scaled_icon_size);
65+ TopMargin = (int) (theme.TopMargin * scaled_icon_size);
66 ItemPadding = (int) (theme.ItemPadding * scaled_icon_size);
67 UrgentBounceHeight = (int) (theme.UrgentBounceHeight * IconSize);
68 LineWidth = theme.LineWidth;
69@@ -1070,22 +1075,22 @@
70 switch (controller.prefs.Position) {
71 default:
72 case PositionType.BOTTOM:
73- struts [Struts.BOTTOM] = (VisibleDockHeight + controller.window.get_screen ().get_height () - monitor_geo.y - monitor_geo.height) * window_scale_factor;
74+ struts [Struts.BOTTOM] = (VisibleDockHeight + TopMargin + controller.window.get_screen ().get_height () - monitor_geo.y - monitor_geo.height) * window_scale_factor;
75 struts [Struts.BOTTOM_START] = monitor_geo.x * window_scale_factor;
76 struts [Struts.BOTTOM_END] = (monitor_geo.x + monitor_geo.width) * window_scale_factor - 1;
77 break;
78 case PositionType.TOP:
79- struts [Struts.TOP] = (monitor_geo.y + VisibleDockHeight) * window_scale_factor;
80+ struts [Struts.TOP] = (monitor_geo.y + VisibleDockHeight + TopMargin) * window_scale_factor;
81 struts [Struts.TOP_START] = monitor_geo.x * window_scale_factor;
82 struts [Struts.TOP_END] = (monitor_geo.x + monitor_geo.width) * window_scale_factor - 1;
83 break;
84 case PositionType.LEFT:
85- struts [Struts.LEFT] = (monitor_geo.x + VisibleDockWidth) * window_scale_factor;
86+ struts [Struts.LEFT] = (monitor_geo.x + VisibleDockWidth + TopMargin) * window_scale_factor;
87 struts [Struts.LEFT_START] = monitor_geo.y * window_scale_factor;
88 struts [Struts.LEFT_END] = (monitor_geo.y + monitor_geo.height) * window_scale_factor - 1;
89 break;
90 case PositionType.RIGHT:
91- struts [Struts.RIGHT] = (VisibleDockWidth + controller.window.get_screen ().get_width () - monitor_geo.x - monitor_geo.width) * window_scale_factor;
92+ struts [Struts.RIGHT] = (VisibleDockWidth + TopMargin + controller.window.get_screen ().get_width () - monitor_geo.x - monitor_geo.width) * window_scale_factor;
93 struts [Struts.RIGHT_START] = monitor_geo.y * window_scale_factor;
94 struts [Struts.RIGHT_END] = (monitor_geo.y + monitor_geo.height) * window_scale_factor - 1;
95 break;
96
97=== modified file 'lib/Widgets/DockWindow.vala'
98--- lib/Widgets/DockWindow.vala 2014-04-12 14:58:48 +0000
99+++ lib/Widgets/DockWindow.vala 2014-06-26 15:05:18 +0000
100@@ -684,6 +684,12 @@
101 get_window ().input_shape_combine_region (new Region.rectangle (rect), 0, 0);
102 }
103 }
104+
105+ public void update_struts ()
106+ {
107+ if (screen.is_composited() && controller.prefs.HideMode == HideType.NONE)
108+ set_struts();
109+ }
110
111 void set_struts ()
112 {

Subscribers

People subscribed via source and target branches

to status/vote changes: