← Back to team overview

kicad-developers team mailing list archive

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