From 55cf4ea95d88c8fe63f3db1d80390ffe0b75d008 Mon Sep 17 00:00:00 2001 From: James Crook Date: Mon, 22 Mar 2021 17:45:54 +0000 Subject: [PATCH] Bug 2524 - Macros "Cancel" button is confusing - doesn't do "what it says on the tin" --- src/BatchProcessDialog.cpp | 68 +++++++++++++++++++++++++++++++++----- src/BatchProcessDialog.h | 3 ++ 2 files changed, 62 insertions(+), 9 deletions(-) diff --git a/src/BatchProcessDialog.cpp b/src/BatchProcessDialog.cpp index 9e3d729ee..51e34cca5 100644 --- a/src/BatchProcessDialog.cpp +++ b/src/BatchProcessDialog.cpp @@ -69,6 +69,9 @@ #define MacrosPaletteTitle XO("Macros Palette") #define ManageMacrosTitle XO("Manage Macros") + +// Separate numerical range from the additional buttons +// in the expanded view (which start at 10,000). #define MacrosListID 7001 #define CommandsListID 7002 #define ApplyToProjectID 7003 @@ -80,6 +83,7 @@ BEGIN_EVENT_TABLE(ApplyMacroDialog, wxDialogWrapper) EVT_BUTTON(ApplyToProjectID, ApplyMacroDialog::OnApplyToProject) EVT_BUTTON(ApplyToFilesID, ApplyMacroDialog::OnApplyToFiles) EVT_BUTTON(wxID_CANCEL, ApplyMacroDialog::OnCancel) + EVT_BUTTON(wxID_CLOSE, ApplyMacroDialog::OnCancel) EVT_BUTTON(wxID_HELP, ApplyMacroDialog::OnHelp) END_EVENT_TABLE() @@ -174,7 +178,7 @@ void ApplyMacroDialog::PopulateOrExchange(ShuttleGui &S) /* i18n-hint: The Expand button makes the dialog bigger, with more in it */ mResize = S.Id(ExpandID).AddButton(XXO("&Expand")); S.AddSpace( 10,10,1 ); - S.AddStandardButtons( eCancelButton | eHelpButton); + S.AddStandardButtons( eCloseButton | eHelpButton); } S.EndHorizontalLay(); } @@ -495,16 +499,20 @@ void ApplyMacroDialog::OnCancel(wxCommandEvent & WXUNUSED(event)) enum { AddButtonID = 10000, RemoveButtonID, + RenameButtonID, + RestoreButtonID, ImportButtonID, ExportButtonID, + SaveButtonID, + DefaultsButtonID, + InsertButtonID, EditButtonID, DeleteButtonID, UpButtonID, DownButtonID, - RenameButtonID, - RestoreButtonID, + // MacrosListID 7005 // CommandsListID, 7002 // Re-Use IDs from ApplyMacroDialog. @@ -523,9 +531,10 @@ BEGIN_EVENT_TABLE(MacrosWindow, ApplyMacroDialog) EVT_BUTTON(RestoreButtonID, MacrosWindow::OnRestore) EVT_BUTTON(ImportButtonID, MacrosWindow::OnImport) EVT_BUTTON(ExportButtonID, MacrosWindow::OnExport) + EVT_BUTTON(SaveButtonID, MacrosWindow::OnSave) + EVT_BUTTON(ExpandID, MacrosWindow::OnExpand) EVT_BUTTON(ShrinkID, MacrosWindow::OnShrink) - EVT_SIZE(MacrosWindow::OnSize) EVT_LIST_ITEM_ACTIVATED(CommandsListID, MacrosWindow::OnCommandActivated) @@ -537,6 +546,7 @@ BEGIN_EVENT_TABLE(MacrosWindow, ApplyMacroDialog) EVT_BUTTON(wxID_OK, MacrosWindow::OnOK) EVT_BUTTON(wxID_CANCEL, MacrosWindow::OnCancel) + EVT_BUTTON(wxID_CLOSE, MacrosWindow::OnCancel) EVT_KEY_DOWN(MacrosWindow::OnKeyDown) END_EVENT_TABLE() @@ -628,6 +638,8 @@ void MacrosWindow::PopulateOrExchange(ShuttleGui & S) mRestore = S.Id(RestoreButtonID).AddButton(XXO("Re&store")); mImport = S.Id(ImportButtonID).AddButton(XO("I&mport...")); mExport = S.Id(ExportButtonID).AddButton(XO("E&xport...")); + mSave = S.Id(SaveButtonID).AddButton(XO("&Save")); + mSave->Enable( mChanged ); } S.EndVerticalLay(); } @@ -693,11 +705,12 @@ void MacrosWindow::PopulateOrExchange(ShuttleGui & S) // OnOK saves without prompting. // That difference is too slight to merit a button, and with the OK // button, people might expect the dialog to apply the macro too. - S.AddStandardButtons( /*eOkButton |*/ eCancelButton | eHelpButton); + S.AddStandardButtons( /*eOkButton |*/ eCloseButton | eHelpButton); } + S.EndHorizontalLay(); - + return; } @@ -761,14 +774,22 @@ void MacrosWindow::UpdateMenus() void MacrosWindow::UpdateDisplay( bool bExpanded ) { + // If we failed to save changes, we abandon the attempt to + // change the expand/shrink state of the GUI. if( !SaveChanges() ) return; mbExpanded = bExpanded; + + mChanged = false; + // if we try to access the about to be destroyed mSave button + // inappropriately, we need to crash rather than (sometimes) silently + // succeed. + mSave = nullptr; + DestroyChildren(); SetSizer( nullptr ); - - mChanged = false; + mSelectedCommand = 0; SetMinSize( wxSize( 200,200 )); @@ -795,7 +816,10 @@ void MacrosWindow::OnExpand(wxCommandEvent &WXUNUSED(event)) { UpdateDisplay( true );} void MacrosWindow::OnShrink(wxCommandEvent &WXUNUSED(event)) -{ UpdateDisplay( false );} +{ + if( ChangeOK() ) + UpdateDisplay( false ); +} bool MacrosWindow::ChangeOK() @@ -821,6 +845,7 @@ bool MacrosWindow::ChangeOK() } mChanged = false; + mSave->Enable( mChanged ); } return true; @@ -1044,6 +1069,7 @@ void MacrosWindow::OnRemove(wxCommandEvent & WXUNUSED(event)) // changed. Since we've just deleted the macro, we should // forget about that change. mChanged = false; + mSave->Enable( mChanged ); mActiveMacro = mMacros->GetItemText(item); PopulateMacros(); @@ -1070,6 +1096,7 @@ void MacrosWindow::OnRestore(wxCommandEvent & WXUNUSED(event)) mMacroCommands.RestoreMacro(mActiveMacro); mChanged = true; + mSave->Enable( mChanged ); PopulateList(); } @@ -1114,6 +1141,12 @@ void MacrosWindow::OnExport(wxCommandEvent & WXUNUSED(event)) mMacroCommands.WriteMacro(mMacros->GetItemText(item), this); } +void MacrosWindow::OnSave(wxCommandEvent & WXUNUSED(event)) +{ + SaveChanges(); +} + + /// An item in the list has been selected. /// Bring up a dialog to allow its parameters to be edited. void MacrosWindow::OnCommandActivated(wxListEvent & WXUNUSED(event)) @@ -1154,6 +1187,8 @@ void MacrosWindow::InsertCommandAt(int item) d.mSelectedParameters, item); mChanged = true; + mSave->Enable( mChanged ); + mSelectedCommand = item + 1; PopulateList(); } @@ -1191,7 +1226,10 @@ void MacrosWindow::OnEditCommandParams(wxCommandEvent & WXUNUSED(event)) mMacroCommands.AddToMacro(command, params, item); + mChanged = true; + mSave->Enable( mChanged ); + mSelectedCommand = item; PopulateList(); } @@ -1207,7 +1245,9 @@ void MacrosWindow::OnDelete(wxCommandEvent & WXUNUSED(event)) } mMacroCommands.DeleteFromMacro(item); + mChanged = true; + mSave->Enable( mChanged ); if (item >= (mList->GetItemCount() - 2) && item >= 0) { item--; @@ -1230,7 +1270,10 @@ void MacrosWindow::OnUp(wxCommandEvent & WXUNUSED(event)) mMacroCommands.GetParams(item), item - 1); mMacroCommands.DeleteFromMacro(item + 1); + mChanged = true; + mSave->Enable( mChanged ); + mSelectedCommand = item - 1; PopulateList(); } @@ -1249,7 +1292,10 @@ void MacrosWindow::OnDown(wxCommandEvent & WXUNUSED(event)) mMacroCommands.GetParams(item), item + 2); mMacroCommands.DeleteFromMacro(item); + mChanged = true; + mSave->Enable( mChanged ); + mSelectedCommand = item + 1; PopulateList(); } @@ -1277,7 +1323,11 @@ bool MacrosWindow::SaveChanges(){ return false; } } + mChanged = false; + if( mSave ) + mSave->Enable( mChanged ); + return true; } diff --git a/src/BatchProcessDialog.h b/src/BatchProcessDialog.h index 6909cf0ac..d1eb45478 100644 --- a/src/BatchProcessDialog.h +++ b/src/BatchProcessDialog.h @@ -107,6 +107,8 @@ private: void OnRestore(wxCommandEvent &event); void OnImport(wxCommandEvent &event); void OnExport(wxCommandEvent &event); + void OnSave(wxCommandEvent &event); + void OnExpand(wxCommandEvent &event); void OnShrink(wxCommandEvent &event); void OnSize(wxSizeEvent &event); @@ -136,6 +138,7 @@ private: wxButton *mRestore; wxButton *mImport; wxButton *mExport; + wxButton *mSave; int mSelectedCommand; bool mChanged;