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 781607 - Use Gtk Widget templates
Use Gtk Widget templates
Status: RESOLVED FIXED
Product: gnome-usage
Classification: Other
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: GNOME Usage maintainer(s)
GNOME Usage maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2017-04-22 12:10 UTC by Felipe Borges
Modified: 2017-05-10 13:42 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
build: Install the GResources in the correct place (1.36 KB, patch)
2017-04-22 14:23 UTC, Felipe Borges
committed Details | Review
build: Require glib >= 2.38 (788 bytes, patch)
2017-04-22 14:23 UTC, Felipe Borges
committed Details | Review
header-bar: Port to Gtk+ widget template (15.41 KB, patch)
2017-04-22 14:23 UTC, Felipe Borges
committed Details | Review
window: Port to Gtk+ widget template (3.98 KB, patch)
2017-04-22 14:37 UTC, Felipe Borges
committed Details | Review
performance-view: Port to Gtk+ widget template (5.81 KB, patch)
2017-04-22 15:12 UTC, Felipe Borges
none Details | Review
process-dialog: Initial port to Gtk+ widget template (4.21 KB, patch)
2017-04-22 15:49 UTC, Felipe Borges
accepted-commit_now Details | Review
performance-view: Port to Gtk+ widget template (5.82 KB, patch)
2017-04-24 15:43 UTC, Felipe Borges
committed Details | Review
process-dialog: Initial port to Gtk+ widget template (4.21 KB, patch)
2017-04-24 15:43 UTC, Felipe Borges
committed Details | Review
Rewrite BetterBox from C language to vala. (8.10 KB, patch)
2017-04-25 13:41 UTC, Petr Štětka
none Details | Review
Rewrite BetterBox from C language to vala. (1.69 KB, patch)
2017-04-26 13:58 UTC, Petr Štětka
committed Details | Review

Description Felipe Borges 2017-04-22 12:10:48 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.
Comment 1 Felipe Borges 2017-04-22 14:23:37 UTC
Created attachment 350234 [details] [review]
build: Install the GResources in the correct place
Comment 2 Felipe Borges 2017-04-22 14:23:43 UTC
Created attachment 350235 [details] [review]
build: Require glib >= 2.38

This is necessary for the introduction of Gtk+ widget templates.
Comment 3 Felipe Borges 2017-04-22 14:23:49 UTC
Created attachment 350236 [details] [review]
header-bar: Port to Gtk+ widget template
Comment 4 Felipe Borges 2017-04-22 14:37:57 UTC
Created attachment 350237 [details] [review]
window: Port to Gtk+ widget template
Comment 5 Felipe Borges 2017-04-22 15:12:17 UTC
Created attachment 350239 [details] [review]
performance-view: Port to Gtk+ widget template
Comment 6 Felipe Borges 2017-04-22 15:49:21 UTC
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.
Comment 7 Petr Štětka 2017-04-24 14:51:32 UTC
Review of attachment 350234 [details] [review]:

Accepted
Comment 8 Petr Štětka 2017-04-24 14:51:58 UTC
Review of attachment 350235 [details] [review]:

Accepted
Comment 9 Petr Štětka 2017-04-24 15:26:56 UTC
Review of attachment 350236 [details] [review]:

Accepted.
Comment 10 Petr Štětka 2017-04-24 15:27:44 UTC
Review of attachment 350237 [details] [review]:

Accepted.
Comment 11 Petr Štětka 2017-04-24 15:36:05 UTC
Review of attachment 350239 [details] [review]:

Plase change transition-type to "slide-up-down" in performance-stack in performance-view.ui.
Comment 12 Felipe Borges 2017-04-24 15:41:29 UTC
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
Comment 13 Felipe Borges 2017-04-24 15:43:23 UTC
Created attachment 350308 [details] [review]
performance-view: Port to Gtk+ widget template
Comment 14 Felipe Borges 2017-04-24 15:43:31 UTC
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.
Comment 15 Petr Štětka 2017-04-24 15:43:54 UTC
Review of attachment 350241 [details] [review]:

Accepted.
Comment 16 Petr Štětka 2017-04-24 15:50:24 UTC
Review of attachment 350308 [details] [review]:

Accepted.
Comment 17 Petr Štětka 2017-04-24 15:52:18 UTC
Review of attachment 350309 [details] [review]:

Accepted.
Comment 18 Felipe Borges 2017-04-24 16:23:13 UTC
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.
Comment 19 Petr Štětka 2017-04-25 13:41:37 UTC
Created attachment 350396 [details] [review]
Rewrite BetterBox from C language to vala.
Comment 20 Felipe Borges 2017-04-26 12:43:02 UTC
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.
Comment 21 Petr Štětka 2017-04-26 13:58:06 UTC
Created attachment 350479 [details] [review]
Rewrite BetterBox from C language to vala.
Comment 22 Felipe Borges 2017-05-02 09:52:02 UTC
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.
Comment 24 Felipe Borges 2017-05-10 13:42:07 UTC
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).