GNOME Bugzilla – Bug 663736
GtkGrid should be used for layout in system-monitor
Last modified: 2012-07-30 11:26:26 UTC
GtkTable, GtkHBox, and GtkVBox have all been deprecated. Code should be ported to GtkGrid. See http://developer.gnome.org/gtk3/3.2/gtk-migrating-GtkGrid.html
I have been thinking lately of moving out the UI creation part (as much of it as possible) into a .ui file (to better separation between the UI and the logic) created with Glade. I guess it would not be a big thing to migrate to grid at the same time. What do you think?
For the preference dialog, sure. For the 'Processes' & 'File system' tabs its probably not worth it. For the others I'd rather keep the flexibility of leaving the layout in code, at least for now.
Created attachment 201948 [details] [review] Migrate the preferences dialog to Gtk Grid The attached patch moves most of the UI creation for the preferences dialog out of the code into a Glade ui interface file, and also migrates to use GtkGrid instead of GtkBoxes. Probably the UI file should be moved out of the src dir into a data directory, and the proper make rules should be added to also copy the ui file into something like /usr/share/... (a directory for system monitor data)
Created attachment 201949 [details] [review] Migrate the renice dialog to Gtk Grid This should be applied over the previous patch. It migrates the renice dialog from Gtk Table and GtkBox to GtkGrids, and also moves the UI creation out of the code into the Glade UI file, thus simplifying the code. With this patch, procdialogs.cpp migration to GtkGrid should be complete.
Review of attachment 201948 [details] [review]: Other than the one comment, it seems fine. ::: src/preferences.ui @@ +1,1 @@ +<?xml version="1.0" encoding="UTF-8"?> You need to let the build system know about this file.
Review of attachment 201949 [details] [review]: Seems ok. Could be merged pending changes to patch this builds upon.
(In reply to comment #2) > For the preference dialog, sure. For the 'Processes' & 'File system' tabs its > probably not worth it. For the others I'd rather keep the flexibility of > leaving the layout in code, at least for now. Disregard this. Feel free to use UI files where ever it fits.The code reduction is worth every penny. ;)
Could you please help me out a bit with the build system? I don't really know how to make the build system know about the new file, and would need some help with getting the first file in the build. Afterwards, I'd take this task.
I've applied the first attachment and my own change to the build system (commit e1d08de3) to get you started. The second attachment didn't apply correctly so I'll leave that to you, Robert. ;)
Migrated the deprecated components (VBox, HBox, and some of the tables) to GtkBoxes and GtkGrids on my migrateui branch. We have no more deprecated warnings in the build, and only a single warning left to fix bug 664524, interface.cpp:501:63: warning: cast to pointer from integer of different size. Chris, could you please check my branch and let me know if I should update anything. [1] http://git.gnome.org/browse/gnome-system-monitor/log/?h=migrateui
Very nice Robert! Thanks for working on that. I've compiled and tested it quickly. I'll go over it a little more closely, and push it by the end of the day.
I'm still getting the gtktable deprecation warnings for sysinfo.cpp. Maybe you forgot to include a commit? I've merged your migrateui branch, made a few changes and pushed to master. can't close until the sysinfo.cpp is free of those derecation warnings though. Thanks!
Created attachment 219862 [details] [review] patch to replace remaining tables with grids (In reply to comment #12) > I'm still getting the gtktable deprecation warnings for sysinfo.cpp. Maybe you > forgot to include a commit? Nope, I didn't forget, but I didn't get the deprecation warnings for gtk table, only for vbox and hbox on ubuntu. Anyway I have replaced the sysinfo tables with grids, pushed the changes to the migrateui branch, and also attaching the patch here. Review and merge the one that is easier for you to check. > > I've merged your migrateui branch, made a few changes and pushed to master. > can't close until the sysinfo.cpp is free of those derecation warnings though. Hopefully this patch/changeset will fix the remaining deprecation warnings.
commited. Thanks.