Fwd: Re: [PATCH] Provide photos summary on dive list

[Willem Ferguson]

On 20/11/2016 00:52, Robert Helling wrote:
> Willem,
>
> excellent!
>
>> Am 19.11.2016 um 13:44 schrieb Willem Ferguson 
>> <willemferguson at zoology.up.ac.za 
>> <mailto:willemferguson at zoology.up.ac.za>>:
>>
>> Please, I need someone to check my coding carefully. I tested it
>> quite extensively and have not found any problems. Also, there may
>> be places where my code is inefficient.
>
> Looks good from my reading (haven’t tried to build it yet). Just a few 
> comments:
>
> 1) You also change the resource name of the location icon. Is that 
> intentional?
>
> 2) The picture counting code could probably be simplified a bit by 
> using the FOR_EACH_PICTURE(dive) macro that iterates over the pictures 
> of a dive.
>
> 3) Did you draw the icons yourself? If not, are the licenses compatible?
>
> Best
> Robert
>
Just a short response. Thank you, Robert, I greatly appreciate your time.

The name change of the globe icon was intentional because I thought that 
the name "satellite" was highly confusing. There is an icon in the icons 
folder, named "satellite" and I thought a more appropriate name for that 
resource was needed.

I will implement the macro and resubmit.

Yes, they are my own icons. Thank Gimp and Inkscape.

A few issues:
1) I would like to have the default column width as narrow as possible 
by default. The width of 25 specified in divelistview.cpp appears not to 
be sufficient. I am not sure?

2) I could not really think of a good title in the title bar of the dive 
list, hence the solid square character. But in the dropdown list 
(right-click on title bar) where one can select fields to be displayed 
in the dive list, it also lists as the solid square character: a little 
unintuitive. I suppose one could put a decorator there in the same way 
as in the dive fields below. But I am not sure what such an icon should 
look like or whether such a graphic would show up in the dropdown list? 
What is your intuitive feeling?

Kind regards,
willem




-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.subsurface-divelog.org/pipermail/subsurface/attachments/20161120/2e0d4df8/attachment.html>