After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 659777 - GtkNotebook theming improvements
GtkNotebook theming improvements
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: GtkNotebook
unspecified
Other All
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2011-09-22 00:19 UTC by Cosimo Cecchi
Modified: 2011-09-27 17:45 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
notebook: use the current state to get the padding values (3.19 KB, patch)
2011-09-22 00:20 UTC, Cosimo Cecchi
committed Details | Review
notebook: allow using different padding values for the active tab state (5.25 KB, patch)
2011-09-22 00:20 UTC, Cosimo Cecchi
committed Details | Review
notebook: really deprecate tab_vborder and tab_hborder (5.65 KB, patch)
2011-09-22 00:20 UTC, Cosimo Cecchi
committed Details | Review
notebook: unconditionally apply padding to the tab content (1.69 KB, patch)
2011-09-22 00:20 UTC, Cosimo Cecchi
committed Details | Review
stylecontext: add style classes for top/bottom/right/left areas (2.95 KB, patch)
2011-09-22 00:20 UTC, Cosimo Cecchi
committed Details | Review
notebook: add top/bottom/left/right style classes to the tab region (7.60 KB, patch)
2011-09-22 00:20 UTC, Cosimo Cecchi
committed Details | Review

Description Cosimo Cecchi 2011-09-22 00:19:57 UTC
There are various things wrong in GtkNotebook with regard to theming support currently. Namely:
- it's impossible to use a different padding values for the active tab state
- the tab content is rendered off center in some cases
- there's no way for the CSS theme to apply a different style according to the tab position

This patchset fixes these issues; there's an extra patch that attempts to really deprecate tab_vborder and tab_hborder, which I believe it's actually a 2.x->3.0 migration leftover.
Comment 1 Cosimo Cecchi 2011-09-22 00:20:05 UTC
Created attachment 197201 [details] [review]
notebook: use the current state to get the padding values

We want to enable the use of different padding values between active and
inactive tabs, so that the two are completely separated (but limited by
the active tab size).
This way themes can decide how bigger the active tab is drawn compared
to the normal one just specifying a different padding value from the
CSS, like this:

.notebook tab {
  padding: 2;
}

.notebook tab:active {
  padding: 4;
}

As a first step, fetch the padding values with the right state flags
from GtkStyleContext.
Comment 2 Cosimo Cecchi 2011-09-22 00:20:08 UTC
Created attachment 197202 [details] [review]
notebook: allow using different padding values for the active tab state

The code before was basically adding and removing the same padding value
in two different places during the allocation cycle.
Instead, what we want to do is to offset the inactive tab allocation by
the difference with the active tab padding, to ensure the tab content is
always drawn centered and in the right position.
Comment 3 Cosimo Cecchi 2011-09-22 00:20:11 UTC
Created attachment 197203 [details] [review]
notebook: really deprecate tab_vborder and tab_hborder

The setter for this was deprecated in 2.x and removed in 3.0.
I don't see any reason why we should hardcode 2px for this value;
instead, deprecated the getter and make it always return zero, and stop
using the variables internally.
Comment 4 Cosimo Cecchi 2011-09-22 00:20:14 UTC
Created attachment 197204 [details] [review]
notebook: unconditionally apply padding to the tab content

Not only when on left or top, otherwise the other position look
off-centered.
Comment 5 Cosimo Cecchi 2011-09-22 00:20:17 UTC
Created attachment 197205 [details] [review]
stylecontext: add style classes for top/bottom/right/left areas

This is useful to e.g. theme notebook tabs differently according to
their position directly from the CSS sheet.

GtkNotebook support in a separate commit.
Comment 6 Cosimo Cecchi 2011-09-22 00:20:20 UTC
Created attachment 197206 [details] [review]
notebook: add top/bottom/left/right style classes to the tab region

When we use the style context to get information for the tab region,
also add a style class to indicate its position, so that the relevant
information is pulled off from the theme.
Comment 7 Matthias Clasen 2011-09-22 01:24:17 UTC
Review of attachment 197201 [details] [review]:

::: gtk/gtknotebook.c
@@ +6294,3 @@
     }
 
+  if (state == GTK_STATE_FLAG_NORMAL)

Using the state here doesn't make sense to me. I'd rather keep the explicit page comparison for this.
Comment 8 Matthias Clasen 2011-09-22 01:26:35 UTC
Review of attachment 197202 [details] [review]:

::: gtk/gtknotebook.c
@@ +5993,3 @@
+          padding.right = MAX (0, active_padding.right - normal_padding.right);
+          padding.bottom = MAX (0, active_padding.bottom - normal_padding.bottom);
+          padding.left = MAX (0, active_padding.left - normal_padding.left);

Is there an assumption here that active_padding will always be larger than normal_padding ?
Comment 9 Matthias Clasen 2011-09-22 01:28:46 UTC
Review of attachment 197203 [details] [review]:

Makes sense, but I'd like to look through bugzilla for complaints about the demise of h/vborder and see if there's something we should do.
Comment 10 Matthias Clasen 2011-09-22 01:29:22 UTC
Review of attachment 197204 [details] [review]:

Makes sense
Comment 11 Matthias Clasen 2011-09-22 01:31:36 UTC
Lets hold these until after 3.2
Comment 12 Cosimo Cecchi 2011-09-22 02:02:41 UTC
(In reply to comment #7)
> Review of attachment 197201 [details] [review]:
> 
> ::: gtk/gtknotebook.c
> @@ +6294,3 @@
>      }
> 
> +  if (state == GTK_STATE_FLAG_NORMAL)
> 
> Using the state here doesn't make sense to me. I'd rather keep the explicit
> page comparison for this.

True...OTOH that code is removed in the following patch. I will squash the patches differently.

(In reply to comment #8)

> Is there an assumption here that active_padding will always be larger than
> normal_padding ?

Yes, the idea is that you cannot have an inactive tab taller than the active one. The padding we're building is basically the offset between the two tab states, so in case the style specifies normal_padding > active_padding we remove the offset and draw them with the same height (the padding will still be applied to the tab content though). I will put this in a comment.

(In reply to comment #9)
> Review of attachment 197203 [details] [review]:
> 
> Makes sense, but I'd like to look through bugzilla for complaints about the
> demise of h/vborder and see if there's something we should do.

I found these two bug reports asking for this to be removed basically

https://bugzilla.gnome.org/show_bug.cgi?id=612567
https://bugzilla.gnome.org/show_bug.cgi?id=643633

Actually the comment in the second report confirms this really looks like a leftover from the 2.x->3.0 transition.
Comment 13 Cosimo Cecchi 2011-09-27 17:44:47 UTC
Attachment 197201 [details] pushed as d4f83cd - notebook: use the current state to get the padding values
Attachment 197202 [details] pushed as 2500a95 - notebook: allow using different padding values for the active tab state
Attachment 197203 [details] pushed as 34ee6d0 - notebook: really deprecate tab_vborder and tab_hborder
Attachment 197204 [details] pushed as d6a58e5 - notebook: unconditionally apply padding to the tab content
Attachment 197205 [details] pushed as 8d3f7e3 - stylecontext: add style classes for top/bottom/right/left areas
Attachment 197206 [details] pushed as c2f5d3d - notebook: add top/bottom/left/right style classes to the tab region

I merged this patches to master with the suggested fixes now that GTK+ branched.