On Tue, Jun 21, 2016 at 10:39 PM, Eduardo Lima (Etrunko) <etrunko@xxxxxxxxxx> wrote: > On 06/21/2016 05:04 PM, Fabiano Fidêncio wrote: >> GtkRevealer was intrudced in Gtk+ 3.10 and, combined with Gtk Overlay >> (intoduced in Gtk+ 3.2), can provide a more sustainably implementation >> of the AutoDrawer functionality. >> >> This approach is completely based on the approach taken by virt-manager: >> https://github.com/virt-manager/virt-manager/commit/dc05600324f6b9a82b68581fc0a9c145f9889ce9 >> >> Resolves: https://bugs.freedesktop.org/show_bug.cgi?id=94495 >> >> Signed-off-by: Fabiano Fidêncio <fidencio@xxxxxxxxxx> >> --- >> src/Makefile.am | 8 +- >> src/resources/ui/virt-viewer.ui | 401 +++++++------- >> src/view/autoDrawer.c | 991 ---------------------------------- >> src/view/autoDrawer.h | 91 ---- >> src/view/drawer.c | 366 ------------- >> src/view/drawer.h | 83 --- >> src/view/ovBox.c | 946 -------------------------------- >> src/view/ovBox.h | 103 ---- >> src/view/virt-viewer-timed-revealer.c | 224 ++++++++ >> src/view/virt-viewer-timed-revealer.h | 74 +++ >> src/virt-viewer-window.c | 36 +- >> 11 files changed, 521 insertions(+), 2802 deletions(-) > > The joys of cleaning up code. I have some general questions before going > deep on the review. > > 1) Do you think the new files could go under src/ instead of src/view/ > directory? Those seemed to be used only for the old widget. Well, I personally don't have a strong opinion about where those files are placed. Just left there because the files used for the old widget were there. > > 2) Even better, it seems that timed-revealed has more boilerplate code > than code that really does something useful. Any specific reason why > those functions could not be part of virt-viewer-window.c ? Just because I preferred to keep the implementation details as close as possible to the one used in virt-manager. The functions could be moved to the virt-viewer-window.c, but I, particularly, fail to see a good reason for doing this. Actually, IMHO, it seems more organized the way it's now than moving the functions to the virt-viewer-window.c file. Best Regards, -- Fabiano Fidêncio _______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
