[virt-tools-list] [virt-viewer 2/2] window: Replace autoDrawer with native Gtk widgets

[Eduardo Lima (Etrunko)]

On 06/21/2016 05:50 PM, Fabiano Fidêncio wrote:
> On Tue, Jun 21, 2016 at 10:39 PM, Eduardo Lima (Etrunko)
> <etrunko at redhat.com> 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 at redhat.com>
>>> ---
>>>  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.

Yeah, if you want my suggestion, get rid of that directory. :P

> 
>>
>> 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.

Okay, I just read the code, I like it the way it is. I have posted a few
comments about it.

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etrunko at redhat.com