← Back to team overview

kicad-developers team mailing list archive

[RFC] More consistent dialog handling

 

Hi all,


you may remember that I complained about the inconsistent behaviour of
several dialogs some time ago.

Below is what I have come up with to improve that. The main purpose is
to improve consistency of dialogs, although it may occasionally simplify
the code too.

Basically it is a kind of bracket around what makes up a typical input in
a dialog, namely a label, a text entry, an optional spin button and an
optional unit indicator, kind of an extended WX_UNIT_BINDER.

The idea is that the code for the dialog should only deal with that
object and use the individual controls only in special circumstances.

Below is the declaration of WX_DIMENSION_INPUT, this is for inputting
everything that ends up as a value in internal units. Also attached is
the code for 2 (simple) dialogs using it to show what that looks like
(please note that I'm not sure the values used for design rule
validation are actually the right ones, this is more for demonstration).

What WX_DIMENSION_INPUT provides for now:

- disabling/enabling of all controls belonging to the input as a group
- setting/getting the input value directly in internal units
- allowing input of optional unit suffixes (mm, in, mi etc.)
- validating that a formally correct value was entered (meaning no more
inputting "mimimi" and getting 0 as value without error message)
- validating that the value won't overflow internal units limits
- validating the value against settable minimum/maximum limits (default
> 0, <= INT_MAX)
- error popups with consistent error messages if validation fails
(e.g. "Value of 'Track width' invalid: design rules specify a minimum of
0.2 mm."), with the value name automatically derived from the label
- after the error popup is dismissed the text entry will get focus and
the text in it will be selected
- the value can be accessed either by explicit functions or by the
usual data transfer mechanism of wxWidgets

Similar (but simpler) classes can be created for input of arbitrary
floating point numbers and for integers.

Some more features would need to be added to be able to use this for
complicated dialogs like the pad properties.

So, my question is, what do you think? Is it worth continuing in this
direction (identifiers and other details can be discussed if this ever
gets to the stage of being considered for inclusion)?


Have fun,

MGri



/*
 * This program source code file is part of KiCad, a free EDA CAD application.
 *
 * Copyright (C) 2014-2015 CERN
 * Author: Maciej Suminski <maciej.suminski@xxxxxxx>
 *
 * This program is free software; you can redistribute it and/or
 * modify it under the terms of the GNU General Public License
 * as published by the Free Software Foundation; either version 2
 * of the License, or (at your option) any later version.
 *
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 * GNU General Public License for more details.
 *
 * You should have received a copy of the GNU General Public License
 * along with this program; if not, you may find one here:
 * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
 * or you may search the http://www.gnu.org website for the version 2 license,
 * or you may write to the Free Software Foundation, Inc.,
 * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA
 */

#ifndef __WX_DIALOG_INPUT_H_
#define __WX_DIALOG_INPUT_H_

#include <common.h>
#include <wx/valnum.h>

class wxTextEntry;
class wxSpinButton;
class wxStaticText;


class WX_DIALOG_INPUT_BASE
{
public:

    /**
     * Constructor.
     * @param aParent is the parent window.
     * @param aLabel is the label displayed next to the text field.
     * @param aTextInput is the text input widget used to edit the given value (wxTextCtrl, wxComboBox, ...).
     */
    WX_DIALOG_INPUT_BASE( wxWindow* aParent, wxStaticText* aLabel, wxTextEntry* aTextInput );

    virtual ~WX_DIALOG_INPUT_BASE();

    /**
     * Function Enable
     * Enables/disables the bound widgets
     */
    virtual void Enable( bool aEnable = true );

    /**
     * Function Disable
     * Disables the bound widgets
     */
    virtual void Disable();

protected:

    ///> Label for the input.
    wxStaticText* m_label;

    ///> Text input control.
    wxTextEntry*  m_textEntry;

};


class WX_DIMENSION_VALIDATOR : public wxTextValidator
{
public:

    /**
     * Constructor
     * @param aUnit is the user unit to use.
     * @param aValueName is the name for the validated value to use in error messages.
     * @param aValue is the pointer to the value used for data transfer.
     */
    WX_DIMENSION_VALIDATOR( EDA_UNITS_T aUnit, const wxString& aValueName, int* aValue );

    /**
     * Copy constructor
     * @param aValidator is the validator to create a copy of.
     */
    WX_DIMENSION_VALIDATOR( const WX_DIMENSION_VALIDATOR& aValidator );

    virtual ~WX_DIMENSION_VALIDATOR();

    /**
     * Function Clone
     * Clone function required by wxWidgets.
     */
    virtual wxObject* Clone() const;

    /**
     * Function Validate
     * Validates the content of the input control.
     * @param aParent is the parent window.
     * @returns Whether the input is valid.
     */
    virtual bool Validate( wxWindow *aParent );

    /**
     * Function GetValidationError
     * Validates the content of the input control and returns possible error message.
     * @returns The error message or an empty string.
     */
    wxString GetValidationError() const;

    /**
     * Function IsValid
     * Validates the content of the input control and returns the result.
     * @returns Whether the value is valid.
     */
    bool IsValid() const;

    /**
     * Function TransferFromWindow
     * @see wxValidator::TransferFromWindow
     */
    virtual bool TransferFromWindow();

    /**
     * Function TransferToWindow
     * @see wxValidator::TransferToWindow
     */
    virtual bool TransferToWindow();

    /**
     * Function SetMin
     * Set the minimum allowed value in internal units.
     * @param aValue is the minimum allowed value in internal units.
     */
    void SetMin( int aValue );

    /**
     * Function SetMinErrorMessage
     * Set the error message to show if the value is smaller than the minimum.
     *
     * This only needs to be used if the default message is not sufficient (e.g.
     * to tell the user the violated constraint came from the design rules).
     *
     * @param aMessage is the error message to show if the value is below the minimum.
     */
    void SetMinErrorMessage( const wxString& aMessage );

    /**
     * Function SetMax
     * Set the maximum allowed value in internal units.
     * @param aValue is the maximum allowed value in internal units.
     */
    void SetMax( int aValue );

    /**
     * Function SetMaxErrorMessage
     * Set the error message to show if the value is larger than the maximum.
     *
     * This only needs to be used if the default message is not sufficient (e.g.
     * to tell the user the violated constraint came from the design rules).
     *
     * @param aMessage is the error message to show if the value is above the maximum.
     */
    void SetMaxErrorMessage( const wxString& aMessage );

    /**
     * Function SetValueName
     * Set the name for the value to use in error messages.
     *
     * This only needs to be used if the default name (derived from the label) is
     * not acceptable (e.g. is not unique in the dialog).
     *
     * @param aName is the name to use for the value in error messages.
     */
    void SetValueName( const wxString& aName );

private:

    // Prefix for error messages.
    static wxString m_errorPrefix;

    ///> Current user unit in use.
    EDA_UNITS_T m_units;

    ///> Minimal value in internal units.
    int m_minValue;

    ///> Error message if minimal value violated.
    wxString m_minErrorMessage;

    ///> Maximal value in internal units.
    int m_maxValue;

    ///> Error message if maximal value violated.
    wxString m_maxErrorMessage;

    ///> Name of the value to use in error messages.
    wxString m_valueName;

    ///> Pointer to the variable holding the value.
    int* m_value;
};


class WX_DIMENSION_INPUT : public WX_DIALOG_INPUT_BASE
{
public:

    /**
     * Constructor.
     * @param aParent is the parent window.
     * @param aLabel is the label displayed next to the text field.
     * @param aTextInput is the text input widget used to edit the given value (wxTextCtrl, wxComboBox, ...).
     * @param aUnitLabel is the optional units label displayed next to the text field.
     * @param aValue is an optional pointer to the variable holding the value.
     * @param aSpinButton is an optional spin button (for adjusting the input value).
     */
    WX_DIMENSION_INPUT( wxWindow* aParent, wxStaticText* aLabel, wxTextEntry* aTextInput, wxStaticText* aUnitLabel,
            int* aValue = NULL, wxSpinButton* aSpinButton = NULL );

    virtual ~WX_DIMENSION_INPUT();

    /**
     * Function Enable
     * Enables/disables the bound widgets.
     * @param aEnable is the flag for whether to enable or disable the widgets.
     */
    virtual void Enable( bool aEnable = true );

    /**
     * Function Disable
     * Disables the bound widgets.
     */
    virtual void Disable();

    /**
     * Function SetValue
     * Sets new value (in internal inits) for the text field, taking care of units conversion.
     * @param aValue is the new value.
     */
    void SetValue( int aValue );

    /**
     * Function GetValue
     * Returns the current value in internal units.
     */
    int GetValue() const;

    /**
     * Function SetMin
     * Sets the minimum value for the input in internal units.
     * @param aValue is the minimum value in internal units.
     */
    void SetMin( int aValue );

    /**
     * Function SetMinErrorMessage
     * Sets the error message to be used when the minimum value is violated.
     *
     * This function should only be used if a special error message is needed (e.g.
     * to tell the user what the violated constraint is exactly).
     *
     * The error message may contain a placeholder '%s' which will receive
     * the allowed minimum value.
     *
     * @param aMessage is the error message with optional placeholders.
     */
    void SetMinErrorMessage( const wxString& aMessage );


    /**
     * Function SetDesignRuleMin
     * Sets the minimum value for the input in internal units specified by the design rules.
     *
     * This also changes the error message to an appropriate one.
     *
     * @param aValue is the minimum value in internal units.
     */
    void SetDesignRuleMin( int aValue );


    /**
     * Function SetMax
     * Sets the maximum value for the input in internal units.
     * @param aValue is the maximum value in internal units.
     */
    void SetMax( int aValue );

    /**
     * Function SetMaxErrorMessage
     * Sets the error message to be used when the maximum value is violated.
     *
     * This function should only be used if a special error message is needed (e.g.
     * to tell the user what the violated constraint is exactly).
     *
     * The error message may contain a placeholder '%s', which will receive
     * the allowed maximum value.
     *
     * @param aMessage is the error message with optional placeholders.
     */
    void SetMaxErrorMessage( const wxString& aMessage );

    /**
     * Function SetValueName
     * Sets the name to be used for the value in error messages.
     *
     * This function should only be used if the default derived from the label is not
     * acceptable (e.g. is not unique in the dialog).
     *
     * @param aName is the name to be used for the value in error messages.
     */
    void SetValueName( const wxString& aName );

private:

    ///> Label showing currently used units.
    wxStaticText* m_unitLabel;

    ///> Spin button control.
    wxSpinButton*  m_spinButton;

    ///> Currently used units.
    EDA_UNITS_T m_units;

    ///> The validator used.
    WX_DIMENSION_VALIDATOR* m_validator;
};


#endif /* __WX_DIALOG_INPUT_H_ */
/*
 * KiRouter - a push-and-(sometimes-)shove PCB router
 *
 * Copyright (C) 2014  CERN
 * Author: Maciej Suminski <maciej.suminski@xxxxxxx>
 *
 * This program is free software: you can redistribute it and/or modify it
 * under the terms of the GNU General Public License as published by the
 * Free Software Foundation, either version 3 of the License, or (at your
 * option) any later version.
 *
 * This program is distributed in the hope that it will be useful, but
 * WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
 * General Public License for more details.
 *
 * You should have received a copy of the GNU General Public License along
 * with this program.  If not, see <http://www.gnu.or/licenses/>.
 */

/**
 * Push and Shove router track width and via size dialog.
 */

#include "dialog_track_via_size.h"
#include <base_units.h>
#include <confirm.h>
#include <boost/optional.hpp>

#include "class_board_design_settings.h"

DIALOG_TRACK_VIA_SIZE::DIALOG_TRACK_VIA_SIZE( wxWindow* aParent, BOARD_DESIGN_SETTINGS& aSettings ) :
    DIALOG_TRACK_VIA_SIZE_BASE( aParent ),
    m_trackWidth( aParent, m_staticText3, m_trackWidthText, m_trackWidthLabel ),
    m_viaDiameter( aParent, m_staticText5, m_viaDiameterText, m_viaDiameterLabel ),
    m_viaDrill( aParent, m_staticText7, m_viaDrillText, m_viaDrillLabel ),
    m_settings( aSettings )
{
    // Load router settings to dialog fields
    m_trackWidth.SetValue( m_settings.GetCustomTrackWidth() );
    m_trackWidth.SetDesignRuleMin( aSettings.m_TrackMinWidth );

    m_viaDiameter.SetValue( m_settings.GetCustomViaSize() );
    m_viaDiameter.SetDesignRuleMin( aSettings.m_ViasMinSize );

    m_viaDrill.SetValue( m_settings.GetCustomViaDrill() );
    m_viaDrill.SetDesignRuleMin( aSettings.m_ViasMinDrill );

    m_trackWidthText->SetFocus();
    m_trackWidthText->SetSelection( -1, -1 );
    m_stdButtonsOK->SetDefault();

    // Now all widgets have the size fixed, call FinishDialogSettings
    FinishDialogSettings();

    // Pressing ENTER when any of the text input fields is active applies changes
    Connect( wxEVT_TEXT_ENTER, wxCommandEventHandler( DIALOG_TRACK_VIA_SIZE::onOkClick ), NULL, this );
}


bool DIALOG_TRACK_VIA_SIZE::check()
{
    if( m_viaDrill.GetValue() >= m_viaDiameter.GetValue() )
    {
        DisplayError( GetParent(), _( "Via drill size has to be smaller than via diameter" ) );
        m_viaDrillText->SetFocus();
        return false;
    }

    return true;
}


void DIALOG_TRACK_VIA_SIZE::onClose( wxCloseEvent& aEvent )
{
    EndModal( 0 );
}


void DIALOG_TRACK_VIA_SIZE::onOkClick( wxCommandEvent& aEvent )
{
    if( Validate() && check() )
    {
        // Store dialog values to the router settings
        m_settings.SetCustomTrackWidth( m_trackWidth.GetValue() );
        m_settings.SetCustomViaSize( m_viaDiameter.GetValue() );
        m_settings.SetCustomViaDrill( m_viaDrill.GetValue() );
        EndModal( 1 );
    }
}


void DIALOG_TRACK_VIA_SIZE::onCancelClick( wxCommandEvent& aEvent )
{
    EndModal( 0 );
}
/*
 * KiRouter - a push-and-(sometimes-)shove PCB router
 *
 * Copyright (C) 2014-2015  CERN
 * Copyright (C) 2016 KiCad Developers, see AUTHORS.txt for contributors.
 * Author: Tomasz Wlostowski <tomasz.wlostowski@xxxxxxx>
 *
 * This program is free software: you can redistribute it and/or modify it
 * under the terms of the GNU General Public License as published by the
 * Free Software Foundation, either version 3 of the License, or (at your
 * option) any later version.
 *
 * This program is distributed in the hope that it will be useful, but
 * WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
 * General Public License for more details.
 *
 * You should have received a copy of the GNU General Public License along
 * with this program.  If not, see <http://www.gnu.org/licenses/>.
 */

/**
 * Push and Shove diff pair dimensions (gap) settings dialog.
 */

#include "dialog_pns_diff_pair_dimensions.h"
#include <router/pns_sizes_settings.h>

DIALOG_PNS_DIFF_PAIR_DIMENSIONS::DIALOG_PNS_DIFF_PAIR_DIMENSIONS( wxWindow* aParent, PNS::SIZES_SETTINGS& aSizes,
        NETCLASSPTR aNetClass) :
    DIALOG_PNS_DIFF_PAIR_DIMENSIONS_BASE( aParent ),
    m_traceWidth( this, m_traceWidthLabel, m_traceWidthText, m_traceWidthUnit ),
    m_traceGap( this, m_traceGapLabel, m_traceGapText, m_traceGapUnit ),
    m_viaGap( this, m_viaGapLabel, m_viaGapText, m_viaGapUnit ),
    m_sizes( aSizes )
{
    m_traceWidth.SetValue( aSizes.DiffPairWidth() );
    m_traceWidth.SetDesignRuleMin( aNetClass->GetDiffPairWidth() );

    m_traceGap.SetValue( aSizes.DiffPairGap() );
    m_traceGap.SetDesignRuleMin( aNetClass->GetDiffPairGap() );

    m_viaGap.SetValue( aSizes.DiffPairViaGap() );
    m_viaTraceGapEqual->SetValue( m_sizes.DiffPairViaGapSameAsTraceGap() );

    m_stdButtonsOK->SetDefault();

    updateCheckbox();

    GetSizer()->SetSizeHints(this);
    Centre();
}


void DIALOG_PNS_DIFF_PAIR_DIMENSIONS::updateCheckbox()
{
    if( m_viaTraceGapEqual->GetValue() )
    {
        m_sizes.SetDiffPairViaGapSameAsTraceGap( true );
        m_viaGap.Disable();
    }
    else
    {
        m_sizes.SetDiffPairViaGapSameAsTraceGap( false );
        m_viaGap.Enable();
    }
}


void DIALOG_PNS_DIFF_PAIR_DIMENSIONS::OnOkClick( wxCommandEvent& aEvent )
{
    if( Validate() ) {
        // Save widgets' values to settings
        m_sizes.SetDiffPairGap ( m_traceGap.GetValue() );
        m_sizes.SetDiffPairViaGap ( m_viaGap.GetValue() );
        m_sizes.SetDiffPairWidth ( m_traceWidth.GetValue() );

        EndModal( wxID_OK );
    }
}


void DIALOG_PNS_DIFF_PAIR_DIMENSIONS::OnViaTraceGapEqualCheck( wxCommandEvent& event )
{
    event.Skip();
    updateCheckbox();
}


Follow ups