GNOME Bugzilla – Bug 599442
Port dash section headers to CSS
Last modified: 2009-11-04 21:48:34 UTC
I had originally started doing some work on making the section headers into real St.Buttons, breaking this work out as a separate patch series.
Created attachment 146125 [details] [review] [StBoxLayout] Implement raise and lower Code copied from ClutterGroup.
Created attachment 146126 [details] [review] [StBin] Add missing allow-none for st_bin_set_child
Created attachment 146127 [details] [review] [StWidget] Add missing allow-none for set_style_pseudo_class
Created attachment 146128 [details] [review] [dash] Port Dash section headers to CSS
Comment on attachment 146125 [details] [review] [StBoxLayout] Implement raise and lower >+ /* See comment in group_raise for this */ s/group_raise/st_box_container_raise/
Comment on attachment 146128 [details] [review] [dash] Port Dash section headers to CSS >+ background-color: rgba(00,00,00,c0); rgba() takes 3 decimal ints and a float. >+ padding: 0px 4px 0px 4px; i think that's equivalent to just "0px 4px" >-const DASH_CORNER_RADIUS = 5; >-const DASH_PADDING_SIDE = 14; (I'm going to assume without checking that you actually did remove all the now-unused consts.) >+ this.actor.connect('notify::allocation', Lang.bind(this, function (actor) { >+ let box = this.actor.allocation; could also just connect to allocation-changed, which has the box as a param >+ themeNode.get_color("-shell-gradient-top", false, gradientTopColor); check return value and bail out if it's false? >+ if (priv->draw_border) Needs a better name. ("draw_border_ourselves" or something. Just "draw_border" suggests that when it's false, there's no border.) Actually, can you just do "if (border_width > 0 && !priv->border_image)" ? Also, why did you add this case? Just to be slightly more efficient than the BigBox way? >+ for (i = 0; i < 4; i++) s/i/side/ ? >+ case ST_SIDE_TOP: >+ cogl_rectangle (0, 0, >+ w, border); >+ break; >+ case ST_SIDE_RIGHT: >+ { >+ double border_top = st_theme_node_get_border_width (node, ST_SIDE_TOP); >+ cogl_rectangle (w - border, >+ border_top, >+ w, h - border_top); >+ } >+ break; I don't understand why RIGHT and LEFT take the top border width into account, but not the bottom width, and TOP and BOTTOM don't take any of the borders into account...
(In reply to comment #6) > >+ if (priv->draw_border) > > Needs a better name. ("draw_border_ourselves" or something. Just "draw_border" > suggests that when it's false, there's no border.) Actually, can you just do > "if (border_width > 0 && !priv->border_image)" ? > > Also, why did you add this case? Just to be slightly more efficient than the > BigBox way? I think that the idea is to support different borders per side, which BigRectangle doesn't support. (But if there's a border radius then this doesn't work because the code in BigRectangle is needed.) This is a good example of why writing a commit describing what the patch does is good before attaching a patch for review since it wasn't clear to me scanning the patch what the rational was either, though the light eventually went on.
(In reply to comment #7) > (In reply to comment #6) > > >+ if (priv->draw_border) > > > > Needs a better name. ("draw_border_ourselves" or something. Just "draw_border" > > suggests that when it's false, there's no border.) Actually, can you just do > > "if (border_width > 0 && !priv->border_image)" ? > > > > Also, why did you add this case? Just to be slightly more efficient than the > > BigBox way? > > I think that the idea is to support different borders per side, which > BigRectangle doesn't support. (But if there's a border radius then this doesn't > work because the code in BigRectangle is needed.) > > This is a good example of why writing a commit describing what the patch does > is good before attaching a patch for review since it wasn't clear to me > scanning the patch what the rational was either, though the light eventually > went on. I forgot to mention I was "bugzilla stashing" this patch; yes, will break up this last patch.
Created attachment 146870 [details] [review] Remove (out) "caller-allocates" annotations GObject Introspection+gjs doesn't presently support (out) for caller-allocates scenarios.
Created attachment 146871 [details] [review] Remove (out) "caller-allocates" annotations GObject Introspection+gjs doesn't presently support (out) for caller-allocates scenarios.
Created attachment 146872 [details] [review] Implement non-uniform borders (when not using corner_radius) StTheme CSS supports different border widths for different sides. Implement it for StWidget by drawing the border internally. However, we don't support a nonzero corner-radius with nonuniform borders.
Created attachment 146873 [details] [review] Port Dash section headers to CSS
Comment on attachment 146872 [details] [review] Implement non-uniform borders (when not using corner_radius) >+ case ST_SIDE_TOP: >+ cogl_rectangle (border_left, 0, >+ w - border_right, border_top); >+ break; >+ case ST_SIDE_RIGHT: >+ cogl_rectangle (w - border_right, border_top, >+ w, h - border_bottom); >+ break; It seems like this leaves the corners unpainted. >+ for (side = ST_SIDE_TOP; side <= ST_SIDE_LEFT; side++) >+ for (corner = ST_CORNER_TOPLEFT; corner <= ST_CORNER_BOTTOMLEFT; corner++) I realize both of those were already there, but I don't like them. There's no intuitively obvious ordering of the enum values, so it's not obvious from reading these if we're iterating over the whole range or if we're (intentionally or accidentally) skipping some values.
Comment on attachment 146873 [details] [review] Port Dash section headers to CSS looks right afaict
Attachment 146125 [details] pushed as f549269 - [StBoxLayout] Implement raise and lower Attachment 146870 [details] pushed as e2e513f - Remove (out) "caller-allocates" annotations Attachment 146872 [details] pushed as 4f456b9 - Implement non-uniform borders (when not using corner_radius) Attachment 146873 [details] pushed as 626f679 - Port Dash section headers to CSS