GNOME Bugzilla – Bug 659777
GtkNotebook theming improvements
Last modified: 2011-09-27 17:45:05 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.
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.
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.
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.
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.
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.
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.
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.
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 ?
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.
Review of attachment 197204 [details] [review]: Makes sense
Lets hold these until after 3.2
(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.
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.