kicad-developers team mailing list archive

[Wayne Stambaugh <stambaughw@xxxxxxxxx>]

Hey John,

I merged your patch set as is for now.  We can change this in the future
should we decide it makes sense.  wxSizerFlags::GetDefaultBorder() looks
interesting.  Maybe we should be using it instead of the hard coded 5.
It does make yet one more argument for coding dialogs by hand unless
there is a way to use wxSizerFlags in wxFB.

Cheers,

Wayne

On 10/4/2018 3:45 AM, John Beard wrote:
> Hi Wayne,
> 
> In wxSizer, there is an option to use the "default border", which is
> preferred to hardcoding a pixel value. This is preferred
> apparently[1]. The WX method to do this,
> wxSizerFlags::GetDefaultBorder(), is here.[2] (GNOME-HIG compliant 6px
> on GTK, 5px on OSX, scaled otherwise).
> 
> If we stick with a hardcoded value (as all the wxFB stuff cannot use
> the DPI-aware defaults, it just specifies '5' on all platforms and
> displays), setting the wxSizerFlags is not very concise:
> 
>      wxSizerFlags flags = KIUI::GetStdSizerFlags(); // this can't set
> a border as we don't know the directions yet
> 
>      sizer->Add( widget1, flags.Border( wxLEFT | wxTOP, 5 ) ); //
> still need to feed the '5' in
>      sizer->Add( widget2, flags.Border( wxRIGHT | wxTOP, 5 ) ); //
> can't reuse the '5', have to call Border() again with new directions
> 
> Because you can't "pre-set" the "5", a putative
> KIUI::GetStdSizerFlags() can't actually help us to avoid specifying
> "5" for every subsequent use, and we'd still need a way to get the
> "5". What would you like the API for this to look like?
> 
> Regardless of this, the first 3 commits stand alone, and this is not
> required for them to be perfectly functional.
> 
> Cheers,
> 
> John
> 
> [1] https://docs.wxwidgets.org/3.1/classwx_sizer_flags.html#a2dc000b863b8580ae3f183c0e11c9944
> [2] https://github.com/wxWidgets/wxWidgets/blob/master/include/wx/sizer.h#L107
> On Wed, Oct 3, 2018 at 6:53 PM John Beard <john.j.beard@xxxxxxxxx> wrote:
>>
>> Hi Wayne,
>>
>> I know the policy is not to convert wxFB dialogs, but in this case, I
>> don't see how that dialog could be written neatly in wxFB without
>> having the derived class tear half of the components down again if
>> m_read_only is true. The alternative is to duplicate the entire
>> dialog, which is a common tactic with making similar dialogs wxFB, but
>> it's a shame to multiply code when you could divide it!
>>
>> Especially if you use a helper like the button row widget, you'd have
>> two placeholders, so the wxFB project would be essentially a blank
>> dialog with a single filter box at the top, and you'd have to have two
>> workarounds like InstallOnPanel to place the widgets you actually
>> wanted onto them.
>>
>> Re the common UI thing - that makes sense, I had forgotten about
>> wxSizerFlags, even though I have done something similar in the past!
>> Patch coming up.
>>
>> Cheers,
>>
>> John
>>
>> On Wed, Oct 3, 2018 at 6:40 PM Wayne Stambaugh <stambaughw@xxxxxxxxx> wrote:
>>>
>>> John,
>>>
>>> Your patch set looks good and works as advertised.  I have a few
>>> nit-pick comments.
>>>
>>> I personally prefer making the hotkey list panel hard coded as you did
>>> rather than using wxFB but not all developers share my preference in
>>> this regard.  Since this makes the hotkey list panel more reusable, I'm
>>> hoping no one will protest too loudly.
>>>
>>> Your common UI code (patch 4) probably should include support for a
>>> wxSizerFlag object for the default border.  You might want to consider
>>> using term border instead of margin to prevent confusion as that is how
>>> it is defined in the wxwidgets documentation.  I'm fine either way.
>>>
>>> I will merge your if there are not objections.  You can just create
>>> another patch.
>>>
>>> Cheers,
>>>
>>> Wayne
>>>
>>> On 10/3/2018 11:20 AM, John Beard wrote:
>>>> Hi,
>>>>
>>>> Here is a patch sequence for using the hotkey editor widget in a
>>>> read-only mode to provide a filterable (and slightly prettier) hotkey
>>>> list. This is a 5.1 milestone.
>>>>
>>>> Major details other than the main aim of the list widget:
>>>>
>>>> * There are some "common UI" elements introduced here that should be
>>>> generally re-usable:
>>>> ** BUTTON_ROW_PANEL provides an easy way to construct a row of buttons
>>>> spaced out nicely
>>>> ** A place to put "generic" UI stuff like the 5px constant very often
>>>> used for margins
>>>> * The editor/list panel is no longer a wxFB project, as I couldn't get
>>>> it to play nice with optional elements: undoing half the construction
>>>> in the derived class, unbinding events and changing various stuff in
>>>> response to editabilty was not very tidy. It's substantially less code
>>>> now, partly due to the re-usable widgets above.
>>>>
>>>> Also adds tooltips for the HK editor buttons via the new BUTTON_ROW_PANEL class.
>>>>
>>>> Cheers,
>>>>
>>>> John
>>>>
>>>>
>>>> _______________________________________________
>>>> Mailing list: https://launchpad.net/~kicad-developers
>>>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>>>> Unsubscribe : https://launchpad.net/~kicad-developers
>>>> More help   : https://help.launchpad.net/ListHelp
>>>>
>>>
>>> _______________________________________________
>>> Mailing list: https://launchpad.net/~kicad-developers
>>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>>> Unsubscribe : https://launchpad.net/~kicad-developers
>>> More help   : https://help.launchpad.net/ListHelp