kicad-developers team mailing list archive
-
kicad-developers team
-
Mailing list archive
-
Message #29390
Re: Component table improvements
JP,
You are correct, wxID_CANCEL was all that was needed.
#004 is attached
Thanks for the feedback :)
On Mon, May 8, 2017 at 4:40 PM, jp charras <jp.charras@xxxxxxxxxx> wrote:
> Le 08/05/2017 à 05:29, Oliver Walters a écrit :
> > JP,
> >
> > I was expecting this response :)
> >
> > I was happy to acquiesce initially on this style change as it was only
> my preference over yours, but
> > reading through the responses of a lot of people on the original thread,
> I believe that the
> > usability of simple OK and CANCEL buttons is not what people (not just
> myself) expect.
> >
> > a) Lots of people calling for an "Apply changes" button. This means
> "apply changes now, but keep
> > editing".
> > b) The "Discard changes" button is very useful, and is also different
> from "Apply changes and close"
> > c) In the context of the component table, "OK" and "Cancel" are somewhat
> obtuse.
> > d) An "OK" button performs the same actions as "Apply changes", then
> "Close"
> > e) A "Cancel" button performs the same actions as "Discard changes" then
> "Close"
> >
> > If I revert to using the wxStdDialogButtonSizer, there will be 4 buttons
> which perform confusing
> > combinations of the same actions!
> >
> > I think that "Apply Changes" "Revert changes" and "Close" are sufficient
> and have the benefit of
> > accurately describing what they do.
> >
> > I propose that I catch the ESCAPE_KEY event and this performs the same
> action as pressing the
> > "Close" button (asks for confirmation if there are unsaved changes,
> otherwise closes).
> >
> > Would this be acceptable?
> >
> > Kind regards
> > Oliver
>
>
> AFAIK, the ESC key issue is due to the fact there is no wxID_CANCEL event
> id attached to a button in
> the dialog.
> (the wxStdDialogButtonSizer fixes this issue just because it uses
> wxID_CANCEL for the Cancel button)
>
> Just using wxID_CANCEL for the close button should fix this issue.
> Try it.
>
> >
> > On Sun, May 7, 2017 at 9:00 PM, jp charras <jp.charras@xxxxxxxxxx
> <mailto:jp.charras@xxxxxxxxxx>> wrote:
> >
> > Le 06/05/2017 à 07:35, Oliver Walters a écrit :
> > > After feedback on the original thread
> > (https://lists.launchpad.net/kicad-developers/msg29057.html
> > <https://lists.launchpad.net/kicad-developers/msg29057.html>)
> > >
> > > I have made some further improvements to the component table:
> > >
> > > a) Add values from template fields where no value exists
> > > b) Allow users to specify which fields are used for sorting
> (default = all fields)
> > > c) Add an "Apply changes" button which updates components
> accordingly
> > > d) UI improvements
> > >
> > > Please find patches attached.
> >
> > Thanks, Oliver for all this very good work.
> >
> > But your last patch is partially a regression:
> > the ESC key does not work anymore.
> >
> > I suggest you to use the wxStdDialogButtonSizer container to manage
> OK, Cancel, Apply buttons (they
> > are all standard buttons).
> > This wxStdDialogButtonSizer container has many advantages for
> multi-platform applications (for
> > instance look and fell).
> >
> > "Revert Changes" button must be managed outside this container.
> >
> > >From my point of view, confirmation when the dialog is closed
> should not occur when clicking OK or
> > Cancel button, only ESC key.
> >
> > However I am not sure if it is easy to manage ESC key and Cancel
> button with different options.
> >
> > --
> > Jean-Pierre CHARRAS
> >
> > _______________________________________________
> > Mailing list: https://launchpad.net/~kicad-developers <
> https://launchpad.net/%7Ekicad-developers>
> > Post to : kicad-developers@xxxxxxxxxxxxxxxxxxx <mailto:
> kicad-developers@xxxxxxxxxxxxxxxxxxx>
> > Unsubscribe : https://launchpad.net/~kicad-developers <
> https://launchpad.net/%7Ekicad-developers>
> > More help : https://help.launchpad.net/ListHelp <
> https://help.launchpad.net/ListHelp>
> >
> >
>
>
> --
> Jean-Pierre CHARRAS
>
From bace329a706796187b2d43b0b1ce3ffdfbab47ca Mon Sep 17 00:00:00 2001
From: Oliver Walters <oliver.henry.walters@xxxxxxxxx>
Date: Mon, 8 May 2017 17:18:12 +1000
Subject: [PATCH] ESCAPE key closes dialog
- Set ID of CLOSE button to wxID_CANCEL
---
eeschema/dialogs/dialog_bom_editor_base.cpp | 4 ++--
eeschema/dialogs/dialog_bom_editor_base.fbp | 4 ++--
eeschema/dialogs/dialog_bom_editor_base.h | 3 +--
3 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/eeschema/dialogs/dialog_bom_editor_base.cpp b/eeschema/dialogs/dialog_bom_editor_base.cpp
index c2c1643..4907965 100644
--- a/eeschema/dialogs/dialog_bom_editor_base.cpp
+++ b/eeschema/dialogs/dialog_bom_editor_base.cpp
@@ -23,7 +23,7 @@ BEGIN_EVENT_TABLE( DIALOG_BOM_EDITOR_BASE, DIALOG_SHIM )
EVT_DATAVIEW_SELECTION_CHANGED( wxID_ANY, DIALOG_BOM_EDITOR_BASE::_wxFB_OnSelectionChanged )
EVT_BUTTON( ID_BUTTON_APPLY, DIALOG_BOM_EDITOR_BASE::_wxFB_OnApplyFieldChanges )
EVT_BUTTON( ID_BUTTON_REVERT, DIALOG_BOM_EDITOR_BASE::_wxFB_OnRevertFieldChanges )
- EVT_BUTTON( ID_BUTTON_CLOSE, DIALOG_BOM_EDITOR_BASE::_wxFB_OnCloseButton )
+ EVT_BUTTON( wxID_CANCEL, DIALOG_BOM_EDITOR_BASE::_wxFB_OnCloseButton )
END_EVENT_TABLE()
DIALOG_BOM_EDITOR_BASE::DIALOG_BOM_EDITOR_BASE( wxWindow* parent, wxWindowID id, const wxString& title, const wxPoint& pos, const wxSize& size, long style ) : DIALOG_SHIM( parent, id, title, pos, size, style )
@@ -118,7 +118,7 @@ DIALOG_BOM_EDITOR_BASE::DIALOG_BOM_EDITOR_BASE( wxWindow* parent, wxWindowID id,
bSizer71->Add( 0, 0, 1, wxEXPAND, 5 );
- m_closeButton = new wxButton( this, ID_BUTTON_CLOSE, _("Close"), wxDefaultPosition, wxDefaultSize, 0 );
+ m_closeButton = new wxButton( this, wxID_CANCEL, _("Close"), wxDefaultPosition, wxDefaultSize, 0 );
bSizer71->Add( m_closeButton, 0, wxALL, 5 );
diff --git a/eeschema/dialogs/dialog_bom_editor_base.fbp b/eeschema/dialogs/dialog_bom_editor_base.fbp
index 97630c8..d78537d 100644
--- a/eeschema/dialogs/dialog_bom_editor_base.fbp
+++ b/eeschema/dialogs/dialog_bom_editor_base.fbp
@@ -45,7 +45,7 @@
<property name="name">DIALOG_BOM_EDITOR_BASE</property>
<property name="pos"></property>
<property name="size">775,654</property>
- <property name="style">wxDEFAULT_DIALOG_STYLE|wxRESIZE_BORDER</property>
+ <property name="style">wxDEFAULT_DIALOG_STYLE|wxMAXIMIZE_BOX|wxRESIZE_BORDER</property>
<property name="subclass">DIALOG_SHIM; dialog_shim.h</property>
<property name="title">BOM editor</property>
<property name="tooltip"></property>
@@ -1024,7 +1024,7 @@
<property name="font"></property>
<property name="gripper">0</property>
<property name="hidden">0</property>
- <property name="id">ID_BUTTON_CLOSE</property>
+ <property name="id">wxID_CANCEL</property>
<property name="label">Close</property>
<property name="max_size"></property>
<property name="maximize_button">0</property>
diff --git a/eeschema/dialogs/dialog_bom_editor_base.h b/eeschema/dialogs/dialog_bom_editor_base.h
index bfb9086..0086ae0 100644
--- a/eeschema/dialogs/dialog_bom_editor_base.h
+++ b/eeschema/dialogs/dialog_bom_editor_base.h
@@ -34,7 +34,6 @@ class DIALOG_SHIM;
#define ID_BUTTON_REGROUP 1001
#define ID_BUTTON_APPLY 1002
#define ID_BUTTON_REVERT 1003
-#define ID_BUTTON_CLOSE 1004
///////////////////////////////////////////////////////////////////////////////
/// Class DIALOG_BOM_EDITOR_BASE
@@ -93,7 +92,7 @@ class DIALOG_BOM_EDITOR_BASE : public DIALOG_SHIM
public:
- DIALOG_BOM_EDITOR_BASE( wxWindow* parent, wxWindowID id = wxID_ANY, const wxString& title = _("BOM editor"), const wxPoint& pos = wxDefaultPosition, const wxSize& size = wxSize( 775,654 ), long style = wxDEFAULT_DIALOG_STYLE|wxRESIZE_BORDER );
+ DIALOG_BOM_EDITOR_BASE( wxWindow* parent, wxWindowID id = wxID_ANY, const wxString& title = _("BOM editor"), const wxPoint& pos = wxDefaultPosition, const wxSize& size = wxSize( 775,654 ), long style = wxDEFAULT_DIALOG_STYLE|wxMAXIMIZE_BOX|wxRESIZE_BORDER );
~DIALOG_BOM_EDITOR_BASE();
void m_splitter1OnIdle( wxIdleEvent& )
--
2.7.4
Follow ups
References