kicad-developers team mailing list archive

[Wayne Stambaugh <stambaughw@xxxxxxxxxxx>]

On 2/18/2014 12:22 PM, Henner Zeller wrote:
> On 18 February 2014 08:31, Wayne Stambaugh <stambaughw@xxxxxxxxxxx> wrote:
>> On 2/18/2014 2:40 AM, Henner Zeller wrote:
>>> Hi,
>>>
>>> This is an update patch to the new component chooser containing the
>>> planned followup-change to display a mini-preview of the component.
>>> Clicking on that box opens the 'big' component browser (Also fixes
>>> [Bug 1280567]). (Since we already can preselect a lot, it would be
>>> good if that browser would take these defaults; planned for another
>>> patch).
>>>
>>> Also, the component browser now allows to choose not only the
>>> component itself, but the units (or 'Gates' or 'Parts' however you'd
>>> call them). The four NANDs in a 7400 for instance, are provided in a
>>> sub-tree to select (the preview also shows the right unit).
>>>
>>> The best part of that: the chosen unit is also kept in the history. So
>>> if you previously selected 'Unit B' of the 7400, you now can press 'a'
>>> (add component - opens dialog), cursor-down (go from 'Unit B' to 'Unit
>>> C') and ENTER: now you can place 'Unit C'. I find that this very much
>>> speeds up the way I choose components.
>>>
>>> Also now: internally it works with the Aliases instead of the
>>> Components - the previous implementation just gave out component
>>> names, but it should've given out Alias names (so even if you chose
>>> 74HC00, you got 74LS00. This is now fixed).
>>>
>>> The preview is based on the suggestion by jp; and like he predicted
>>> there were several places that I needed to change to properly deal
>>> with NULL-aPanels. Included in this patch..
>>>
>>> Commit message:
>>>  * Allow to select units in components that have more than one right
>>> in the component chooser dialog.
>>>  * Keep chosen unit in history.
>>>  * Show preview of current component-unit as thumbnail image next to
>>> the description box.
>>>  * Fixes 1280567
>>>
>>> View here:
>>> https://github.com/hzeller/kicad/compare/master...component-selection#files_bucket
>>>
>>> Download here:
>>> https://github.com/hzeller/kicad/compare/master...component-selection.diff
>>>
>>> -h
>>>
>>
>> Henner,
>>
>> Nice work.  This is infinitely better than the old component selection
>> dialog.  I will commit this patch as is if no one has any objections but
>> I have a few comments and one minor bug.  On windows builds with
>> wxWidget 3.0.0, the component view panel does not update when a new
>> component is selected.  When the dialog is closed and reopen it, then
>> the selected component is shown correctly.
> 
> This might be due to the way I am force-updating the panel. I'll have
> to fully grok how wxWidgets does redraws I suppose :)
> It was according to jc's suggestion, but I suppose since it draws into
> the panel without explicit redraw command, it
> might be lost depending on how the underlying windowing system is
> dealing with it.
> 
> In general it would be better if this panel was its own separate
> component (derived from wxPanel) that does the
> component draw. If I would do that, do I just override a Render(wxDC *) method ?
> I might not get around working on it the next couple of days (busy
> short week), is it ok if we submit this now and I'll iterate from here
> ?

I agree with your assessment about EDA_DRAW_PANEL being decoupled from
the frame window.  My guess is that would be quite a significant
undertaking given the current design.  If this is something you want to
tackle, I would do it one function at a time until all occurrences of
GetParent() and GetScreen() are gone and any parent code other than
default or overloaded wxWindows member functions are pushed up the stack
into EDA_DRAW_FRAME or converted to a wxMessage and sent to the parent
window.  Otherwise, you could break things is significant ways if you
are not careful.  I'm not sure what your best course of action is in
this case.  I never tried to use EDA_DRAW_PANEL in any window other than
an EDA_DRAW_FRAME so I don't know how much help I can give you.

I will try to get this patch committed ASAP.

> 
>>
>> I could not open the dialog fbp file with wxFormBuilder 3.4.0 or 3.3.04.
>>  I got an error saying that the dialog was created with a newer version
>> of wxFormBuilder.  What version of wxFormBuilder are you using and if
>> it's greater than 3.4.0 where did you get it?  The latest version on the
>> wxFormBuilder website is 3.4.0.
> 
> Since there was no version in the package repositories for my debian
> system, I compiled it from source. I deliberately took the 3.x branch
> not the 4.x branch for that, but looks like it still newer. Though it
> seems the format didn't change much; if you manually change the minor
> version from 12 to 11, you might be able to read it.

Thanks.  I'll give that try.  There is a wxFormBuilder PPA at
https://launchpad.net/~wxformbuilder/+archive/release that you can use
for binaries.  Granted they are compiled with Ubuntu in mind but I find
that I can use them without issue as long as I don't use one of the
latest Ubuntu release builds.

> 
>> One minor comment on your dialog layout, take a look at some of the
>> layout space in some of the other dialogs.  The dialog buttons do not
>> have enough padding between them and the dialog frame.  I've been using
>> the GNOME HIG at
>> https://developer.gnome.org/hig-book/stable/design-window.html.en for my
>> dialog design.
> 
> k, looks like a little bit of padding around the dialog might help.
> 
>>
>> I also found a few coding policy issues:
>>
>> * if ( foo ) should be if( foo )
> 
> Ah, I didn't see this explicitly mentioned in the coding style guide.
> Though there are
> examples in style guide that show it like this.
> 
>> * In cpp file, functions should be separated by two lines.
> 
> updated.
> 
>> * Control blocks (if, while, BOOST_FOREACH, etc.) should be have blank
>> lines before and after so it's easy to see where each block begins and ends.
> 
> done.
> 
>> * GetChars(foo) should be GetChars( foo ).
> 
> done.
> 
> Updated the patch.
> 
> -h
>