From f3dcccb14408056c78de3f96b1abc23cb9d54ac0 Mon Sep 17 00:00:00 2001 From: James Crook Date: Sat, 19 Aug 2017 16:48:45 +0100 Subject: [PATCH] Bug 1165 - Enh: Keyboard preference key bindings can be way over to right Fixed by putting key bindings on the left in all three views. I also elevated EXPERIMENTAL_KEY_VIEW to no longer experimental, by excising the old code which we don't need any more. --- src/prefs/KeyConfigPrefs.cpp | 530 ----------------------------------- src/prefs/KeyConfigPrefs.h | 59 ---- src/widgets/KeyView.cpp | 38 +-- 3 files changed, 22 insertions(+), 605 deletions(-) diff --git a/src/prefs/KeyConfigPrefs.cpp b/src/prefs/KeyConfigPrefs.cpp index b15c1c5a5..add621713 100644 --- a/src/prefs/KeyConfigPrefs.cpp +++ b/src/prefs/KeyConfigPrefs.cpp @@ -41,8 +41,6 @@ KeyConfigPrefs and MousePrefs use. #include "../FileNames.h" -#if defined(EXPERIMENTAL_KEY_VIEW) - #include "../widgets/KeyView.h" // @@ -742,534 +740,6 @@ wxString KeyConfigPrefs::HelpPageName() return "Keyboard_Preferences"; } -#else -// -// KeyConfigPrefs -// -#define AssignDefaultsButtonID 17001 -#define CurrentComboID 17002 -#define SetButtonID 17003 -#define ClearButtonID 17004 -#define CommandsListID 17005 -#define ExportButtonID 17006 -#define ImportButtonID 17007 -#define CategoryID 17008 - -// The numbers of the columns of the mList. -enum -{ - CommandColumn, - KeyComboColumn -}; - -BEGIN_EVENT_TABLE(KeyConfigPrefs, PrefsPanel) - EVT_BUTTON(AssignDefaultsButtonID, KeyConfigPrefs::OnDefaults) - EVT_BUTTON(SetButtonID, KeyConfigPrefs::OnSet) - EVT_BUTTON(ClearButtonID, KeyConfigPrefs::OnClear) - EVT_BUTTON(ExportButtonID, KeyConfigPrefs::OnExport) - EVT_BUTTON(ImportButtonID, KeyConfigPrefs::OnImport) - EVT_CHOICE(CategoryID, KeyConfigPrefs::OnCategory) - EVT_LIST_ITEM_SELECTED(CommandsListID, KeyConfigPrefs::OnItemSelected) - EVT_LIST_KEY_DOWN(CommandsListID, KeyConfigPrefs::OnKeyDown) -END_EVENT_TABLE() - -KeyConfigPrefs::KeyConfigPrefs(wxWindow * parent) -: PrefsPanel(parent, _("Keyboard")), - mKey(NULL) -{ - Populate(); -} - -KeyConfigPrefs::~KeyConfigPrefs() -{ - if (mKey) - { - mKey->Disconnect(wxEVT_KEY_DOWN, - wxKeyEventHandler(KeyConfigPrefs::OnCaptureKeyDown)); - mKey->Disconnect(wxEVT_CHAR, - wxKeyEventHandler(KeyConfigPrefs::OnCaptureChar)); - } -} - -void KeyConfigPrefs::Populate() -{ - ShuttleGui S(this, eIsCreatingFromPrefs); - AudacityProject *project = GetActiveProject(); - - if (!project) { - S.StartVerticalLay(true); - { - S.StartStatic(wxEmptyString, true); - { - S.AddTitle(_("Keyboard preferences currently unavailable.")); - S.AddTitle(_("Open a new project to modify keyboard shortcuts.")); - } - S.EndStatic(); - } - S.EndVerticalLay(); - - return; - } - - mManager = project->GetCommandManager(); - mManager->GetCategories(mCats); - mCats.Insert(_("All"), 0); - - PopulateOrExchange(S); - - CreateList(); - mCommandSelected = -1; -} - -/// Normally in classes derived from PrefsPanel this function -/// is used both to populate the panel and to exchange data with it. -/// With KeyConfigPrefs all the exchanges are handled specially, -/// so this is only used in populating the panel. -void KeyConfigPrefs::PopulateOrExchange(ShuttleGui & S) -{ - S.SetBorder(2); - - S.StartStatic(_("Key Bindings"), 1); - { - S.StartHorizontalLay(wxALIGN_CENTRE, false); - { - S.Id(CategoryID); - mCat = S.AddChoice(_("C&ategory:"), - mCats[0], - &mCats); - } - S.EndHorizontalLay(); - - mList = S.Id(CommandsListID).AddListControlReportMode(); - mList->SetName(_("Key Bindings")); - - S.StartThreeColumn(); - { - if (!mKey) { - mKey = safenew wxTextCtrl(this, - CurrentComboID, - wxT(""), - wxDefaultPosition, -#if defined(__WXMAC__) - wxSize(300, -1)); -#else - wxSize(210, -1)); -#endif - mKey->Connect(wxEVT_KEY_DOWN, - wxKeyEventHandler(KeyConfigPrefs::OnCaptureKeyDown)); - mKey->Connect(wxEVT_CHAR, - wxKeyEventHandler(KeyConfigPrefs::OnCaptureChar)); - } - S.AddWindow(mKey); - - /* i18n-hint: (verb)*/ - S.Id(SetButtonID).AddButton(_("Set")); - S.Id(ClearButtonID).AddButton(_("Cl&ear")); - } - S.EndThreeColumn(); - -#if defined(__WXMAC__) - S.AddFixedText(_("Note: Pressing Cmd+Q will quit. All other keys are valid.")); -#endif - - S.StartThreeColumn(); - { - S.Id(ImportButtonID).AddButton(_("&Import...")); - S.Id(ExportButtonID).AddButton(_("&Export...")); - S.Id(AssignDefaultsButtonID).AddButton(_("&Defaults")); - } - S.EndThreeColumn(); - } - S.EndStatic(); -} - -/// Sets up mList with the right number of columns, titles, -/// fills the contents and sets column widths. -void KeyConfigPrefs::CreateList() -{ - mList->InsertColumn(CommandColumn, _("Command"), wxLIST_FORMAT_LEFT); - mList->InsertColumn(KeyComboColumn, _("Key Combination"), wxLIST_FORMAT_LEFT); - - RepopulateBindingsList(); - - mList->SetColumnWidth(CommandColumn, wxLIST_AUTOSIZE); - mList->SetColumnWidth(KeyComboColumn, 250); -} - -static int wxCALLBACK SortCallback(long item1, long item2, long sortData) -{ - wxArrayString *names = (wxArrayString *) sortData; - - if (names->Item(item1) < names->Item(item2)) { - return -1; - } - - if (names->Item(item1) > names->Item(item2)) { - return 1; - } - - return 0; -} - -void KeyConfigPrefs::RepopulateBindingsList() -{ - wxString cat = mCat->GetStringSelection(); - - mList->DeleteAllItems(); // Delete contents, but not the column headers. - mNames.Clear(); - mDefaultKeys.Clear(); - wxArrayString Keys,Labels,Categories; - - mManager->GetAllCommandData( - mNames, - Keys, - mDefaultKeys, - Labels, - Categories, -// True to include effects (list items), false otherwise. - true - ); - - bool save = (mKeys.GetCount() == 0); - - size_t ndx = 0; - int color = 0; - for (size_t i = 0; i < mNames.GetCount(); i++) { - wxString name = mNames[i]; - wxString key = KeyStringDisplay(Keys[i]); - - // Save the original key value to support canceling - if (save) { - mKeys.Add(key); - // mNewKeys is what mKeys will change to. - mNewKeys.Add(key); - } - else - mNewKeys[i] = key; // Make sure mNewKeys is updated. - -// if (cat != _("All") && ! Categories[i].StartsWith(cat)) { - if (cat != _("All") && ! (Categories[i]== cat)) { - continue; - } - wxString label; - - // Labels for undo and redo change according to the last command - // which can be undone/redone, so give them a special check in order - // not to confuse users - if (name == wxT("Undo")) { - label = _("Undo"); - } - else if (name == wxT("Redo")) { - label = _("Redo"); - } - else { - label = mManager->GetPrefixedLabelFromName(name); - } - - label = wxMenuItem::GetLabelFromText(label.BeforeFirst(wxT('\t'))); - - mList->InsertItem(ndx, label); - mList->SetItem(ndx, KeyComboColumn, key); - mList->SetItemData(ndx, i); - mList->SetItemBackgroundColour(ndx, color ? wxColour(240, 240, 240) : *wxWHITE); - color = 1 - color; - - ndx++; - } - -// mList->SortItems(SortCallback, (long) &mNames); -} - -void KeyConfigPrefs::OnImport(wxCommandEvent & WXUNUSED(event)) -{ - wxString file = wxT("Audacity-keys.xml"); - - file = FileNames::SelectFile(FileNames::Operation::Open, - _("Select an XML file containing Audacity keyboard shortcuts..."), - wxEmptyString, - file, - wxT(""), - _("XML files (*.xml)|*.xml|All files|*"), - wxRESIZE_BORDER, - this); - - if (!file) { - return; - } - - XMLFileReader reader; - if (!reader.Parse(mManager, file)) { - wxMessageBox(reader.GetErrorStr(), - _("Error Importing Keyboard Shortcuts"), - wxOK | wxCENTRE, this); - } - - RepopulateBindingsList(); -} - -void KeyConfigPrefs::OnExport(wxCommandEvent & WXUNUSED(event)) -{ - wxString file = wxT("Audacity-keys.xml"); - - file = FileNames::SelectFile(FileNames::Operation::Export, - _("Export Keyboard Shortcuts As:"), - path, - file, - wxT("xml"), - _("XML files (*.xml)|*.xml|All files|*"), - wxFD_SAVE | wxFD_OVERWRITE_PROMPT | wxRESIZE_BORDER, - this); - - if (!file) { - return; - } - - GuardedCall< void >( [&] { - XMLFileWriter prefFile{ file, _("Error Exporting Keyboard Shortcuts") }; - mManager->WriteXML(prefFile); - prefFile.Commit(); - } ); -} - -void KeyConfigPrefs::OnDefaults(wxCommandEvent & WXUNUSED(event)) -{ - for (size_t i = 0; i < mNames.GetCount(); i++) { - mManager->SetKeyFromIndex(i,mDefaultKeys[i]); - mNewKeys[i]=mDefaultKeys[i]; - } - RepopulateBindingsList(); -} - -void KeyConfigPrefs::OnCaptureKeyDown(wxKeyEvent & e) -{ - wxTextCtrl *t = (wxTextCtrl *)e.GetEventObject(); - -#if defined(__WXMAC__) || defined(__WXGTK__) - if (e.GetKeyCode() == WXK_TAB) { - wxNavigationKeyEvent nevent; - nevent.SetWindowChange(e.ControlDown()); - nevent.SetDirection(!e.ShiftDown()); - nevent.SetEventObject(t); - nevent.SetCurrentFocus(t); - t->GetParent()->ProcessEvent(nevent); - - return; - } -#endif - - t->SetValue(KeyStringDisplay(KeyEventToKeyString(e))); -} - -void KeyConfigPrefs::OnCaptureChar(wxKeyEvent & WXUNUSED(event)) -{ -} - -// Given a hotkey combination, returns the name (description) of the -// corresponding command, or the empty string if none is found. -wxString KeyConfigPrefs::NameFromKey( const wxString & key ) -{ - int i; - i=mNewKeys.Index( key ); - if( i== wxNOT_FOUND ) - return wxT(""); - return mNames[i]; -} - -// Sets the selected command to have this key -// This is not yet a committed change, which will happen on a save. -void KeyConfigPrefs::SetKeyForSelected( const wxString & key ) -{ - int i = mList->GetItemData(mCommandSelected); - wxString name = mNames[i]; - - mList->SetItem(mCommandSelected, KeyComboColumn, key); - mManager->SetKeyFromIndex(i, key); - -#if 0 - int i=mNames.Index( name ); - if( i!=wxNOT_FOUND ) - mNewKeys[i]=key; -#endif - - mNewKeys[i]=key; - -} - - -void KeyConfigPrefs::OnSet(wxCommandEvent & WXUNUSED(event)) -{ - if ( mCommandSelected >= (int)mNames.GetCount()) - return; - - wxString newKey = mKey->GetValue(); - wxString alreadyAssignedName = NameFromKey( newKey ); - - // Prevent same hotkey combination being used twice. - if( !alreadyAssignedName.IsEmpty() ) { - wxMessageBox( - wxString::Format( - _("The keyboard shortcut '%s' is already assigned to:\n\n'%s'"), - newKey.c_str(), - alreadyAssignedName.c_str()), - _("Error"), wxICON_STOP | wxCENTRE, this); - return; - } - - SetKeyForSelected( newKey ); -} - -void KeyConfigPrefs::OnClear(wxCommandEvent& WXUNUSED(event)) -{ - mKey->Clear(); - - if (mCommandSelected < 0 || mCommandSelected >= (int)mNames.GetCount()) { - return; - } - SetKeyForSelected( wxT("") ); -} - -void KeyConfigPrefs::OnKeyDown(wxListEvent & e) -{ -// the code in this function allows the user to seek to the next -// command which begins with the letter that is pressed -#ifdef __WXMAC__ - // I (Ed) have no way of telling what code will work on - // the Mac but the following code does not - return; -#endif - -#ifdef __WXMSW__ - // Windows seems to have this built-in - // and does not need the following code - return; - -#else - // The following code seems to work well on at least some versions of Linux - int keycode = e.GetKeyCode(); - int selected = mList->GetNextItem(-1, wxLIST_NEXT_ALL, wxLIST_STATE_SELECTED); - int cnt = mList->GetItemCount(); - wxListItem item; - bool found = false; - - item.SetColumn(CommandColumn); - item.SetMask(wxLIST_MASK_TEXT); - - for (int i = selected + 1; i < cnt; i++) - { - item.SetId(i); - - mList->GetItem(item); - - if (item.GetText().Left(1).IsSameAs(keycode, false)) { - mList->SetItemState(e.GetIndex(), - 0, - wxLIST_STATE_FOCUSED | wxLIST_STATE_SELECTED); - - mList->SetItemState(i, - wxLIST_STATE_FOCUSED | wxLIST_STATE_SELECTED, - wxLIST_STATE_FOCUSED | wxLIST_STATE_SELECTED); - - mList->EnsureVisible(i); - - found = true; - - break; - } - } - - if (!found) { - for (int i = 0; i < selected; i++) - { - item.SetId(i); - - mList->GetItem(item); - - if (item.GetText().Left(1).IsSameAs(keycode, false)) { - mList->SetItemState(e.GetIndex(), - 0, - wxLIST_STATE_FOCUSED | wxLIST_STATE_SELECTED); - - mList->SetItemState(i, - wxLIST_STATE_FOCUSED | wxLIST_STATE_SELECTED, - wxLIST_STATE_FOCUSED | wxLIST_STATE_SELECTED); - - mList->EnsureVisible(i); - break; - } - } - } -#endif -} - -void KeyConfigPrefs::OnCategory(wxCommandEvent & WXUNUSED(event)) -{ - RepopulateBindingsList(); -} - -void KeyConfigPrefs::OnItemSelected(wxListEvent & e) -{ - mCommandSelected = e.GetIndex(); - - if (mCommandSelected < 0 || mCommandSelected >= (int)mNames.GetCount()) { - mKey->SetLabel(wxT("")); - return; - } - - wxListItem item; - item.SetColumn(KeyComboColumn); - item.SetMask(wxLIST_MASK_TEXT); - item.SetId(mCommandSelected); - mList->GetItem(item); - - mKey->Clear(); - mKey->AppendText(item.GetText()); -} - -bool KeyConfigPrefs::Commit() -{ - for (size_t i = 0; i < mNames.GetCount(); i++) { -// wxString dkey = KeyStringNormalize(mManager->GetDefaultKeyFromName(mNames[i])); - wxString dkey = KeyStringNormalize(mDefaultKeys[i]); - wxString name = wxT("/NewKeys/") + mNames[i]; -// wxString key = KeyStringNormalize(mManager->GetKeyFromName(mNames[i])); - wxString key = KeyStringNormalize(mNewKeys[i]); - - if (gPrefs->HasEntry(name)) { - if (key != KeyStringNormalize(gPrefs->Read(name, key))) { - gPrefs->Write(name, key); - } - if (key == dkey) { - gPrefs->DeleteEntry(name); - } - } - else { - if (key != dkey) { - gPrefs->Write(name, key); - } - } - gPrefs->Flush(); - } - - return gPrefs->Flush(); -} - -void KeyConfigPrefs::Cancel() -{ - // Restore original key values - for (size_t i = 0; i < mNames.GetCount(); i++) { - mManager->SetKeyFromIndex(i, mKeys[i]); - } - - return; -} - -wxString KeyConfigPrefs::HelpPageName() -{ - return "Keyboard_Preferences"; -} - -#endif - PrefsPanel *KeyConfigPrefsFactory::Create(wxWindow *parent) { wxASSERT(parent); // to justify safenew diff --git a/src/prefs/KeyConfigPrefs.h b/src/prefs/KeyConfigPrefs.h index 1edd84468..6dfbb7476 100644 --- a/src/prefs/KeyConfigPrefs.h +++ b/src/prefs/KeyConfigPrefs.h @@ -16,8 +16,6 @@ class ShuttleGui; -#if defined(EXPERIMENTAL_KEY_VIEW) - #include #include #include @@ -95,63 +93,6 @@ private: DECLARE_EVENT_TABLE() }; -#else - -#include -#include -#include -#include - -#include "../commands/CommandManager.h" - -#include "PrefsPanel.h" - -class KeyConfigPrefs final : public PrefsPanel -{ - public: - KeyConfigPrefs(wxWindow * parent); - ~KeyConfigPrefs(); - bool Commit() override; - void Cancel() override; - wxString HelpPageName() override; - - private: - void Populate(); - void PopulateOrExchange(ShuttleGui & S); - void CreateList(); - void RepopulateBindingsList(); - wxString NameFromKey( const wxString & key ); - void SetKeyForSelected( const wxString & key ); - - void OnDefaults(wxCommandEvent & e); - void OnImport(wxCommandEvent & e); - void OnExport(wxCommandEvent & e); - void OnSet(wxCommandEvent & e); - void OnClear(wxCommandEvent & e); - void OnCategory(wxCommandEvent & e); - void OnItemSelected(wxListEvent & e); - void OnKeyDown(wxListEvent & e); - - void OnCaptureKeyDown(wxKeyEvent & e); - void OnCaptureChar(wxKeyEvent & e); - - wxChoice *mCat; - wxTextCtrl *mKey; - wxListCtrl *mList; - - CommandManager *mManager; - int mCommandSelected; - - wxArrayString mCats; - wxArrayString mNames; - wxArrayString mDefaultKeys; - wxArrayString mKeys; - wxArrayString mNewKeys; // Used for work in progress. - - DECLARE_EVENT_TABLE() -}; - -#endif class KeyConfigPrefsFactory final : public PrefsPanelFactory { diff --git a/src/widgets/KeyView.cpp b/src/widgets/KeyView.cpp index 8d35dcbdf..c4889a726 100644 --- a/src/widgets/KeyView.cpp +++ b/src/widgets/KeyView.cpp @@ -500,9 +500,9 @@ KeyView::UpdateHScroll() // Calculate the full line width mWidth = KV_LEFT_MARGIN + - mCommandWidth + - KV_COLUMN_SPACER + mKeyWidth + + KV_COLUMN_SPACER + + mCommandWidth + KV_VSCROLL_WIDTH; // Retrieve the current horizontal scroll amount @@ -1056,23 +1056,26 @@ KeyView::OnDrawBackground(wxDC & dc, const wxRect & rect, size_t line) const { const KeyNode *node = mLines[line]; wxRect r = rect; + wxRect r2 = rect; // for just the key shortcut. wxCoord indent = 0; // When in tree view mode, each younger branch gets indented by the // width of the open/close bitmaps if (mViewType == ViewByTree) { - indent += node->depth * KV_BITMAP_SIZE; + indent += mKeyWidth + KV_COLUMN_SPACER + node->depth * KV_BITMAP_SIZE; } // Offset left side by the indentation (if any) and scroll amounts r.x = indent - mScrollX; + r2.x = -mScrollX; // If the line width is less than the client width, then we want to // extend the background to the right edge of the client view. Otherwise, // go all the way to the end of the line width...this will draw past the // right edge, but that's what we want. r.width = wxMax(mWidth, r.width); + r2.width = mKeyWidth; // Selected lines get a solid background if (IsSelected(line)) @@ -1083,13 +1086,18 @@ KeyView::OnDrawBackground(wxDC & dc, const wxRect & rect, size_t line) const dc.SetPen(*wxTRANSPARENT_PEN); dc.SetBrush(wxBrush(wxSystemSettings::GetColour(wxSYS_COLOUR_HIGHLIGHT))); dc.DrawRectangle(r); - + // and they also get a dotted focus rect. This could just be left out. // The focus rect does very little for us, as it is the same size as the // rectangle itself. Consequently for themes that have black text it // disappears. But on HiContrast you do get a dotted green border which // may have some utility. AColor::DrawFocus(dc, r); + + if (mViewType == ViewByTree){ + dc.DrawRectangle(r2); + AColor::DrawFocus(dc, r2); + } } else { @@ -1097,6 +1105,8 @@ KeyView::OnDrawBackground(wxDC & dc, const wxRect & rect, size_t line) const dc.SetPen(*wxTRANSPARENT_PEN); dc.SetBrush(wxBrush(wxSystemSettings::GetColour(wxSYS_COLOUR_BTNFACE))); dc.DrawRectangle(r); + if (mViewType == ViewByTree) + dc.DrawRectangle(r2); } } else @@ -1104,6 +1114,8 @@ KeyView::OnDrawBackground(wxDC & dc, const wxRect & rect, size_t line) const // Non-selected lines get a thin bottom border dc.SetPen(wxColour(240, 240, 240)); dc.DrawLine(r.GetLeft(), r.GetBottom(), r.GetRight(), r.GetBottom()); + if (mViewType == ViewByTree ) + dc.DrawLine(r2.GetLeft(), r2.GetBottom(), r2.GetRight(), r2.GetBottom()); } } @@ -1139,7 +1151,7 @@ KeyView::OnDrawItem(wxDC & dc, const wxRect & rect, size_t line) const if (node->iscat || node->ispfx) { - wxCoord bx = x; + wxCoord bx = x + mKeyWidth + KV_COLUMN_SPACER; wxCoord by = rect.y; if (node->ispfx) @@ -1164,9 +1176,9 @@ KeyView::OnDrawItem(wxDC & dc, const wxRect & rect, size_t line) const // Indent text x += KV_LEFT_MARGIN; - // Draw the command and key columns - dc.DrawText(label, x + node->depth * KV_BITMAP_SIZE, rect.y); - dc.DrawText(node->key, x + mCommandWidth + KV_COLUMN_SPACER, rect.y); + // Draw the key and command columns + dc.DrawText(node->key, x , rect.y); + dc.DrawText(label, x + mKeyWidth + KV_COLUMN_SPACER + node->depth * KV_BITMAP_SIZE, rect.y); } else { @@ -1179,14 +1191,8 @@ KeyView::OnDrawItem(wxDC & dc, const wxRect & rect, size_t line) const label = node->prefix + wxT(" - ") + label; } - // Swap the columns based on view type - if(mViewType == ViewByName) - { - // Draw command column and then key column - dc.DrawText(label, x, rect.y); - dc.DrawText(node->key, x + mCommandWidth + KV_COLUMN_SPACER, rect.y); - } - else if(mViewType == ViewByKey) + // don't swap the columns based on view type + if((mViewType == ViewByName) || (mViewType == ViewByKey)) { // Draw key columnd and then command column dc.DrawText(node->key, x, rect.y);