GNOME Bugzilla – Bug 781607
Use Gtk Widget templates
Last modified: 2017-05-10 13:42:07 UTC
Separating/decoupling the UI machinery from the the programming logic is a very good way to keep readability of the code base. As an example, check how gnome-boxes does it elegantly with Vala.
Created attachment 350234 [details] [review] build: Install the GResources in the correct place
Created attachment 350235 [details] [review] build: Require glib >= 2.38 This is necessary for the introduction of Gtk+ widget templates.
Created attachment 350236 [details] [review] header-bar: Port to Gtk+ widget template
Created attachment 350237 [details] [review] window: Port to Gtk+ widget template
Created attachment 350239 [details] [review] performance-view: Port to Gtk+ widget template
Created attachment 350241 [details] [review] process-dialog: Initial port to Gtk+ widget template Should port the GraphBlock and ProcessDialogHeaderBar to gtk+ template widgets to fully decouple the ui code here.
Review of attachment 350234 [details] [review]: Accepted
Review of attachment 350235 [details] [review]: Accepted
Review of attachment 350236 [details] [review]: Accepted.
Review of attachment 350237 [details] [review]: Accepted.
Review of attachment 350239 [details] [review]: Plase change transition-type to "slide-up-down" in performance-stack in performance-view.ui.
Thanks! Attachment 350234 [details] pushed as fb6f4a2 - build: Install the GResources in the correct place Attachment 350235 [details] pushed as 24c23c1 - build: Require glib >= 2.38 Attachment 350236 [details] pushed as 8044f70 - header-bar: Port to Gtk+ widget template Attachment 350237 [details] pushed as ccfb845 - window: Port to Gtk+ widget template
Created attachment 350308 [details] [review] performance-view: Port to Gtk+ widget template
Created attachment 350309 [details] [review] process-dialog: Initial port to Gtk+ widget template Should port the GraphBlock and ProcessDialogHeaderBar to gtk+ template widgets to fully decouple the ui code here.
Review of attachment 350241 [details] [review]: Accepted.
Review of attachment 350308 [details] [review]: Accepted.
Review of attachment 350309 [details] [review]: Accepted.
Attachment 350308 [details] pushed as 5970ee8 - performance-view: Port to Gtk+ widget template Attachment 350309 [details] pushed as 0e45df0 - process-dialog: Initial port to Gtk+ widget template Thanks! I will leave this bug opened so we can attach more ports to Gtk+ widget templates here.
Created attachment 350396 [details] [review] Rewrite BetterBox from C language to vala.
Review of attachment 350396 [details] [review]: Thanks for your patch. Other than that, is it worth it having a BetterBox just to arbitrarily set the max_width_request? Couldn't we, somehow, achieve the same simply with margins? If you think it is worth having it, that's fine. ::: src/better-box.vala @@ +1,1 @@ +namespace Usage { Make another/separate commit adding the license header, please. @@ +7,3 @@ + construct + { + max_width_request = -1; couldn't this be set as the default of the "max_width_request" property? public int max_width_request { get; set; default = -1 } @@ +10,3 @@ + } + + public new void get_preferred_width(out int minimum_width, out int natural_width) I guess this one can be private. @@ +12,3 @@ + public new void get_preferred_width(out int minimum_width, out int natural_width) + { + int min_width; please, stick to 4 spaces indentation.
Created attachment 350479 [details] [review] Rewrite BetterBox from C language to vala.
Review of attachment 350479 [details] [review]: Thanks for your patch. This one looks good. Before you push it, prefix the commit message with "better-box: Rewrite BetterBox from C to Vala", prefixing the commit message with the changed file/module/component, and trying to make the first line shorter than 50 characters (when possible). Try to make the change as atomic as possible. In this case I suggest to write 4 commits: 1. This one introducing the BetterBox vala code. 2. Add a license header to the file. 3. A patch replacing the use of the C version by the Vala version in the files it is used (cpu-sub-view.vala, memory-sub-view.vala..) 4. Another patch removing the better-box.c file, the vapi and removing it from the build scripts.
Review of attachment 350479 [details] [review]: I split the path into three commit and pushed it as 68ef94a, be1f9a9 and 08dc0c0. https://git.gnome.org/browse/gnome-usage/commit/?id=68ef94a https://git.gnome.org/browse/gnome-usage/commit/?id=be1f9a9 https://git.gnome.org/browse/gnome-usage/commit/?id=08dc0c0
It looks great! Thanks! Be careful about the git history. In the future when the code will be way more complex and touched by different people, we will need to be able to bisect a single commit which introduced a feature/bug, or to git blame a line of code. I will close this bug and we can open new ones for other ports (one bug for each widget which is ported to Gtk+ templates).