GNOME Bugzilla – Bug 745530
Theme: refactoring stylesheet
Last modified: 2015-03-05 19:37:33 UTC
This trivial patch makes a code easier to read and maintain (refactor), and generates a smaller final file.
Created attachment 298423 [details] [review] Refactoring stylesheet
Review of attachment 298423 [details] [review]: Thanks for the patch! This looks really nice. I do not know a lot of CSS tho, have you verified that this does not change anything? ::: data/gnome-maps.css @@ +9,3 @@ .layer-radio-button { background-image: none; + padding: 0; Why remove the px here? You keep the px for the other padding attributes below. And other attributes also has it.
Oh, yes, this is because zero (0) doesn't matter if is 0px or 0em, zero is "adimensional" in CSS. But is mandatory put px (or whatever) in (X)px where X >= 1.
Review of attachment 298423 [details] [review]: Did this run ok for you? I got: (org.gnome.Maps:31466): Gjs-WARNING **: JS ERROR: Gtk.CssProviderError: application.css:62:13 Expected a length ::: data/gnome-maps.css @@ +61,2 @@ #instruction-box > GtkImage { + padding: auto 6px; Is auto really ok to use here?
(In reply to Jonas Danielsson from comment #4) > Review of attachment 298423 [details] [review] [review]: > > Did this run ok for you? > > I got: > (org.gnome.Maps:31466): Gjs-WARNING **: JS ERROR: Gtk.CssProviderError: > application.css:62:13 Expected a length > > ::: data/gnome-maps.css > @@ +61,2 @@ > #instruction-box > GtkImage { > + padding: auto 6px; > > Is auto really ok to use here? Ok, I think is my fault. I was confused about this error after see "application.css", then I thought to myself: "looks like not the same stylesheet". I didn't find the mentioned stylesheet and I thought it could be due by an other bug. I think that Gtk CSS support is not quite "complete" as I thought and I made the patch without testing properly. But on the other hand, I don't understand why Gtk CSS do not accept auto, I have reviewed the docs and I didn't find any alternative for auto. Auto is something like "default" or inherited value. I'm going to workaround the issue and upload again the patch. I am sorry for the inconvenience.
Created attachment 298481 [details] [review] The good one patch