kicad-developers team mailing list archive

[Wayne Stambaugh <stambaughw@xxxxxxxxx>]

Fabrizio,

This patch has far more changes than the plot dialogs mentioned in your
commit message.  In the future, please either make your commit message
describe all of the changes related to the patch.  I would prefer that
you only make one wxFormbuilder dialog change per patch.  wxFormbuilder
changes tend to be noisy so it takes longer to review them.

You are using the incorrect capitalization in text box labels.  Text box
labels use sentence capitalization not header capitalization per the UI
guidelines[1].

Please fix your patch accordingly so I can commit it.

Cheers,

Wayne

[1]:http://docs.kicad-pcb.org/doxygen/md_Documentation_development_ui-policy.html

On 7/3/2017 4:45 AM, Fabrizio Tappero wrote:
> Hi JP,
> thanks for it. I updated my wxformbuilder to the latest (3.6v)
> version. And I fixed the (mm) issue you mentioned.
> 
> In attachment the patch.
> 
> thank you
> Fabrizio
> 
> 
> On Sat, Jul 1, 2017 at 8:40 PM, jp charras <jp.charras@xxxxxxxxxx> wrote:
>> Le 30/06/2017 à 15:34, Fabrizio Tappero a écrit :
>>> Hello,
>>> in attachment the patch reviewed after Wayne comments.
>>>
>>> Best regards
>>> Fabrizio
>>>
>>
>> Hi Fabrizio,
>>
>> I had a look at your last patch, and found a few issues:
>> for instance the change:
>>
>> -       m_defaultLineWidthTitle = new wxStaticText( sbSizerPlotFormat->GetStaticBox(), wxID_ANY,
>> _("Default line thickness"), wxDefaultPosition, wxDefaultSize, 0 );
>> +       m_defaultLineWidthTitle = new wxStaticText( sbSizerPlotFormat->GetStaticBox(), wxID_ANY, _("Line
>> Thickness (mm):"), wxDefaultPosition, wxDefaultSize, 0 );
>>
>>
>> is not good for 2 reasons:
>> - "Default line thickness" is the right title (see a previous Wayne's answer)
>> - "Line Thickness (mm):" is incorrect:
>> the units symbol " (mm):" is added a run time, because units can be mm or inches.
>> so never change titles like "Value" to "Value (mm):".
>> It creates a wrong title.
>>
>> Besides your wxFormBuilder version is rather old and creates incorrect C++ sources: please update it.
>>
>> Thanks.
>>
>> --
>> Jean-Pierre CHARRAS
>>
>>
>> _______________________________________________
>> 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