From 7cea4b23f16210ae40df8c19c0918cff77d9edaf Mon Sep 17 00:00:00 2001 From: Seth Hillbrand Date: Mon, 6 Jul 2020 21:51:12 -0700 Subject: [PATCH] ADDED: Expand selection in context menu Break out the context menu with heuristically limited choices being appended to the main list rather than re-selecting Fixes https://gitlab.com/kicad/code/kicad/issues/4799 --- eeschema/tools/ee_selection_tool.cpp | 217 ++++++++++-------- include/collector.h | 48 +++- include/tool/action_menu.h | 3 + pagelayout_editor/tools/pl_selection_tool.cpp | 6 +- pcbnew/tools/selection_tool.cpp | 190 ++++++++------- 5 files changed, 281 insertions(+), 183 deletions(-) diff --git a/eeschema/tools/ee_selection_tool.cpp b/eeschema/tools/ee_selection_tool.cpp index 2739efa817..b33b2615f5 100644 --- a/eeschema/tools/ee_selection_tool.cpp +++ b/eeschema/tools/ee_selection_tool.cpp @@ -525,7 +525,7 @@ bool EE_SELECTION_TOOL::SelectPoint( const VECTOR2I& aWhere, const KICAD_T* aFil // If still more than one item we're going to have to ask the user. if( collector.GetCount() > 1 ) { - collector.m_MenuTitle = _( "Clarify Selection" ); + collector.m_MenuTitle = wxEmptyString; // Must call selectionMenu via RunAction() to avoid event-loop contention m_toolMgr->RunAction( EE_ACTIONS::selectionMenu, true, &collector ); @@ -603,7 +603,7 @@ void EE_SELECTION_TOOL::GuessSelectionCandidates( EE_COLLECTOR& collector, const EDA_ITEM* item = collector[ i ]; if( !item->HitTest( (wxPoint) aPos, 0 ) ) - collector.Remove( item ); + collector.Transfer( item ); } } @@ -614,7 +614,7 @@ void EE_SELECTION_TOOL::GuessSelectionCandidates( EE_COLLECTOR& collector, const EDA_ITEM* other = collector[ ( i + 1 ) % 2 ]; if( item->Type() != SCH_SHEET_T && other->Type() == SCH_SHEET_T ) - collector.Remove( other ); + collector.Transfer( other ); } // Prefer a symbol to a pin or the opposite, when both a symbol and a pin are selected @@ -631,9 +631,9 @@ void EE_SELECTION_TOOL::GuessSelectionCandidates( EE_COLLECTOR& collector, const if( item->Type() == SCH_COMPONENT_T && other->Type() == SCH_PIN_T ) { if( !m_isLibEdit && m_frame->eeconfig()->m_Selection.select_pin_selects_symbol ) - collector.Remove( other ); + collector.Transfer( other ); else - collector.Remove( item ); + collector.Transfer( item ); } } @@ -644,7 +644,7 @@ void EE_SELECTION_TOOL::GuessSelectionCandidates( EE_COLLECTOR& collector, const EDA_ITEM* other = collector[ ( i + 1 ) % 2 ]; if( item->Type() == SCH_FIELD_T && other->Type() == SCH_COMPONENT_T ) - collector.Remove( other ); + collector.Transfer( other ); } // No need for multiple wires at a single point; if there's a junction select that; @@ -665,9 +665,9 @@ void EE_SELECTION_TOOL::GuessSelectionCandidates( EE_COLLECTOR& collector, const for( int j = collector.GetCount() - 1; j >= 0; --j ) { if( junction && collector[ j ]->Type() != SCH_JUNCTION_T ) - collector.Remove( j ); + collector.Transfer( j ); else if( !junction && j > 0 ) - collector.Remove( j ); + collector.Transfer( j ); } } } @@ -1102,106 +1102,127 @@ int EE_SELECTION_TOOL::SelectionMenu( const TOOL_EVENT& aEvent ) bool EE_SELECTION_TOOL::doSelectionMenu( EE_COLLECTOR* aCollector ) { EDA_ITEM* current = nullptr; - ACTION_MENU menu( true ); bool selectAll = false; + bool expandSelection = false; - int limit = std::min( MAX_SELECT_ITEM_IDS, aCollector->GetCount() ); - - for( int i = 0; i < limit; ++i ) + do { - wxString text; - EDA_ITEM* item = ( *aCollector )[i]; - text = item->GetSelectMenuText( m_frame->GetUserUnits() ); + /// The user has requested the full, non-limited list of selection items + if( expandSelection ) + aCollector->Combine(); - wxString menuText = wxString::Format( "&%d. %s\t%d", i + 1, text, i + 1 ); - menu.Add( menuText, i + 1, item->GetMenuImage() ); - } + expandSelection = false; - menu.AppendSeparator(); - menu.Add( _( "Select &All\tA" ), limit + 1, net_highlight_schematic_xpm ); + int limit = std::min( 9, aCollector->GetCount() ); + ACTION_MENU menu( true ); - if( aCollector->m_MenuTitle.Length() ) - menu.SetTitle( aCollector->m_MenuTitle ); - - menu.SetIcon( info_xpm ); - menu.DisplayTitle( true ); - SetContextMenu( &menu, CMENU_NOW ); - - while( TOOL_EVENT* evt = Wait() ) - { - if( evt->Action() == TA_CHOICE_MENU_UPDATE ) + for( int i = 0; i < limit; ++i ) { - if( selectAll ) - { - for( int i = 0; i < aCollector->GetCount(); ++i ) - unhighlight( ( *aCollector )[i], BRIGHTENED ); - } - else if( current ) - { - unhighlight( current, BRIGHTENED ); - } + wxString text; + EDA_ITEM* item = ( *aCollector )[i]; + text = item->GetSelectMenuText( m_frame->GetUserUnits() ); - int id = *evt->GetCommandId(); - - // User has pointed an item, so show it in a different way - if( id > 0 && id <= limit ) - { - current = ( *aCollector )[id - 1]; - highlight( current, BRIGHTENED ); - } - else - { - current = nullptr; - } - - // User has pointed on the "Select All" option - if( id == limit + 1 ) - { - for( int i = 0; i < aCollector->GetCount(); ++i ) - highlight( ( *aCollector )[i], BRIGHTENED ); - selectAll = true; - } - else - { - selectAll = false; - } - } - else if( evt->Action() == TA_CHOICE_MENU_CHOICE ) - { - if( selectAll ) - { - for( int i = 0; i < aCollector->GetCount(); ++i ) - unhighlight( ( *aCollector )[i], BRIGHTENED ); - } - else if( current ) - unhighlight( current, BRIGHTENED ); - - OPT id = evt->GetCommandId(); - - // User has selected the "Select All" option - if( id == limit + 1 ) - { - selectAll = true; - current = nullptr; - } - // User has selected an item, so this one will be returned - else if( id && ( *id > 0 ) && ( *id <= limit ) ) - { - selectAll = false; - current = ( *aCollector )[*id - 1]; - } - else - { - selectAll = false; - current = nullptr; - } - - break; + wxString menuText = wxString::Format( "&%d. %s\t%d", i + 1, text, i + 1 ); + menu.Add( menuText, i + 1, item->GetMenuImage() ); } - getView()->UpdateItems(); - m_frame->GetCanvas()->Refresh(); - } + menu.AppendSeparator(); + menu.Add( _( "Select &All\tA" ), limit + 1, net_highlight_schematic_xpm ); + + if( !expandSelection && aCollector->HasAdditionalItems() ) + menu.Add( _( "&Expand Selection\tE" ), limit + 2, nullptr ); + + if( aCollector->m_MenuTitle.Length() ) + { + menu.SetTitle( aCollector->m_MenuTitle ); + menu.SetIcon( info_xpm ); + menu.DisplayTitle( true ); + } + else + { + menu.DisplayTitle( false ); + } + + SetContextMenu( &menu, CMENU_NOW ); + + while( TOOL_EVENT* evt = Wait() ) + { + if( evt->Action() == TA_CHOICE_MENU_UPDATE ) + { + if( selectAll ) + { + for( int i = 0; i < aCollector->GetCount(); ++i ) + unhighlight( ( *aCollector )[i], BRIGHTENED ); + } + else if( current ) + { + unhighlight( current, BRIGHTENED ); + } + + int id = *evt->GetCommandId(); + + // User has pointed an item, so show it in a different way + if( id > 0 && id <= limit ) + { + current = ( *aCollector )[id - 1]; + highlight( current, BRIGHTENED ); + } + else + { + current = nullptr; + } + + // User has pointed on the "Select All" option + if( id == limit + 1 ) + { + for( int i = 0; i < aCollector->GetCount(); ++i ) + highlight( ( *aCollector )[i], BRIGHTENED ); + selectAll = true; + } + else + { + selectAll = false; + } + } + else if( evt->Action() == TA_CHOICE_MENU_CHOICE ) + { + if( selectAll ) + { + for( int i = 0; i < aCollector->GetCount(); ++i ) + unhighlight( ( *aCollector )[i], BRIGHTENED ); + } + else if( current ) + unhighlight( current, BRIGHTENED ); + + OPT id = evt->GetCommandId(); + + // User has selected the "Select All" option + if( id == limit + 1 ) + { + selectAll = true; + current = nullptr; + } + // User has selected an item, so this one will be returned + else if( id && ( *id > 0 ) && ( *id <= limit ) ) + { + selectAll = false; + current = ( *aCollector )[*id - 1]; + } + else + { + selectAll = false; + current = nullptr; + } + } + else if( evt->Action() == TA_CHOICE_MENU_CLOSED ) + { + break; + } + + getView()->UpdateItems(); + m_frame->GetCanvas()->Refresh(); + } + } while( expandSelection ); if( selectAll ) return true; diff --git a/include/collector.h b/include/collector.h index 8ad43518b6..ac957103f9 100644 --- a/include/collector.h +++ b/include/collector.h @@ -55,7 +55,8 @@ class EDA_ITEM; class COLLECTOR { protected: - std::vector m_List; + std::vector m_List; // Primary list of most likely items + std::vector m_BackupList; // Secondary list with items removed by heuristics const KICAD_T* m_ScanTypes; INSPECTOR_FUNC m_inspector; @@ -148,6 +149,51 @@ public: } } + /** + * Test if the collector has heuristic backup items + * @return true if Combine() can run to bring secondary items into the list + */ + bool HasAdditionalItems() + { + return !m_BackupList.empty(); + } + + /** + * Re-combines the backup list into the main list of the collector + */ + void Combine() + { + std::copy( m_BackupList.begin(), m_BackupList.end(), std::back_inserter( m_List ) ); + m_BackupList.clear(); + } + + /** + * Moves the item at \a aIndex (first position is 0) to the backup list + * @param aIndex The index into the list. + */ + void Transfer( int aIndex ) + { + m_BackupList.push_back( m_List[aIndex] ); + m_List.erase( m_List.begin() + aIndex ); + } + + /** + * Moves the item aItem (if exists in the collector) to the backup list + * @param aItem the item to be moved. + */ + void Transfer( EDA_ITEM* aItem ) + { + for( size_t i = 0; i < m_List.size(); i++ ) + { + if( m_List[i] == aItem ) + { + m_List.erase( m_List.begin() + i ); + m_BackupList.push_back( aItem ); + return; + } + } + } + /** * Function operator[int] * is used for read only access and returns the object at \a aIndex. diff --git a/include/tool/action_menu.h b/include/tool/action_menu.h index dc7634e4ca..168dc4894b 100644 --- a/include/tool/action_menu.h +++ b/include/tool/action_menu.h @@ -210,6 +210,9 @@ protected: ///> Checks if any of submenus contains a TOOL_ACTION with a specific ID. OPT_TOOL_EVENT findToolAction( int aId ); + bool m_isForcedPosition; + wxPoint m_forcedPosition; + bool m_dirty; // Menu requires update before display bool m_titleDisplayed; diff --git a/pagelayout_editor/tools/pl_selection_tool.cpp b/pagelayout_editor/tools/pl_selection_tool.cpp index d216ca3eec..0d4f79afea 100644 --- a/pagelayout_editor/tools/pl_selection_tool.cpp +++ b/pagelayout_editor/tools/pl_selection_tool.cpp @@ -280,7 +280,7 @@ void PL_SELECTION_TOOL::guessSelectionCandidates( COLLECTOR& collector, const VE EDA_ITEM* other = collector[ ( i + 1 ) % 2 ]; if( item->HitTest( (wxPoint) aPos, 0 ) && !other->HitTest( (wxPoint) aPos, 0 ) ) - collector.Remove( other ); + collector.Transfer( other ); } } @@ -585,7 +585,9 @@ bool PL_SELECTION_TOOL::doSelectionMenu( COLLECTOR* aCollector ) current = ( *aCollector )[*id - 1]; else current = NULL; - + } + else if( evt->Action() == TA_CHOICE_MENU_CLOSED ) + { break; } diff --git a/pcbnew/tools/selection_tool.cpp b/pcbnew/tools/selection_tool.cpp index a8061cd4aa..a63d667d02 100644 --- a/pcbnew/tools/selection_tool.cpp +++ b/pcbnew/tools/selection_tool.cpp @@ -461,7 +461,7 @@ bool SELECTION_TOOL::selectPoint( const VECTOR2I& aWhere, bool aOnDrag, if( aOnDrag ) Wait( TOOL_EVENT( TC_ANY, TA_MOUSE_UP, BUT_LEFT ) ); - if( !doSelectionMenu( &collector, _( "Clarify Selection" ) ) ) + if( !doSelectionMenu( &collector, wxEmptyString ) ) { if( aSelectionCancelledFlag ) *aSelectionCancelledFlag = true; @@ -1487,100 +1487,126 @@ bool SELECTION_TOOL::doSelectionMenu( GENERAL_COLLECTOR* aCollector, const wxStr { BOARD_ITEM* current = nullptr; PCBNEW_SELECTION highlightGroup; - ACTION_MENU menu( true ); bool selectAll = false; + bool expandSelection = false; highlightGroup.SetLayer( LAYER_SELECT_OVERLAY ); getView()->Add( &highlightGroup ); - int limit = std::min( 9, aCollector->GetCount() ); - - for( int i = 0; i < limit; ++i ) + do { - wxString text; - BOARD_ITEM* item = ( *aCollector )[i]; - text = item->GetSelectMenuText( m_frame->GetUserUnits() ); + /// The user has requested the full, non-limited list of selection items + if( expandSelection ) + aCollector->Combine(); - wxString menuText = wxString::Format( "&%d. %s\t%d", i + 1, text, i + 1 ); - menu.Add( menuText, i + 1, item->GetMenuImage() ); - } + expandSelection = false; - menu.AppendSeparator(); - menu.Add( _( "Select &All\tA" ), limit + 1, net_highlight_xpm ); + int limit = std::min( 9, aCollector->GetCount() ); + ACTION_MENU menu( true ); - if( aTitle.Length() ) - menu.SetTitle( aTitle ); - - menu.SetIcon( info_xpm ); - menu.DisplayTitle( true ); - SetContextMenu( &menu, CMENU_NOW ); - - while( TOOL_EVENT* evt = Wait() ) - { - if( evt->Action() == TA_CHOICE_MENU_UPDATE ) + for( int i = 0; i < limit; ++i ) { - if( selectAll ) - { - for( int i = 0; i < aCollector->GetCount(); ++i ) - unhighlight( ( *aCollector )[i], BRIGHTENED, &highlightGroup ); - } - else if( current ) - unhighlight( current, BRIGHTENED, &highlightGroup ); + wxString text; + BOARD_ITEM* item = ( *aCollector )[i]; + text = item->GetSelectMenuText( m_frame->GetUserUnits() ); - int id = *evt->GetCommandId(); - - // User has pointed an item, so show it in a different way - if( id > 0 && id <= limit ) - { - current = ( *aCollector )[id - 1]; - highlight( current, BRIGHTENED, &highlightGroup ); - } - else - current = nullptr; - - // User has pointed on the "Select All" option - if( id == limit + 1 ) - { - for( int i = 0; i < aCollector->GetCount(); ++i ) - highlight( ( *aCollector )[i], BRIGHTENED, &highlightGroup ); - selectAll = true; - } - else - selectAll = false; + wxString menuText = wxString::Format( "&%d. %s\t%d", i + 1, text, i + 1 ); + menu.Add( menuText, i + 1, item->GetMenuImage() ); } - else if( evt->Action() == TA_CHOICE_MENU_CHOICE ) + + menu.AppendSeparator(); + menu.Add( _( "Select &All\tA" ), limit + 1, net_highlight_xpm ); + + if( !expandSelection && aCollector->HasAdditionalItems() ) + menu.Add( _( "&Expand Selection\tE" ), limit + 2, nullptr ); + + if( aTitle.Length() ) { - if( selectAll ) - { - for( int i = 0; i < aCollector->GetCount(); ++i ) - unhighlight( ( *aCollector )[i], BRIGHTENED, &highlightGroup ); - } - else if( current ) - unhighlight( current, BRIGHTENED, &highlightGroup ); - - OPT id = evt->GetCommandId(); - - // User has selected the "Select All" option - if( id == limit + 1 ) - { - selectAll = true; - current = nullptr; - } - // User has selected an item, so this one will be returned - else if( id && ( *id > 0 ) && ( *id <= limit ) ) - { - selectAll = false; - current = ( *aCollector )[*id - 1]; - } - else - { - selectAll = false; - current = nullptr; - } - - break; + menu.SetTitle( aTitle ); + menu.SetIcon( info_xpm ); + menu.DisplayTitle( true ); } - } + else + menu.DisplayTitle( false ); + + SetContextMenu( &menu, CMENU_NOW ); + + while( TOOL_EVENT* evt = Wait() ) + { + if( evt->Action() == TA_CHOICE_MENU_UPDATE ) + { + if( selectAll ) + { + for( int i = 0; i < aCollector->GetCount(); ++i ) + unhighlight( ( *aCollector )[i], BRIGHTENED, &highlightGroup ); + } + else if( current ) + unhighlight( current, BRIGHTENED, &highlightGroup ); + + int id = *evt->GetCommandId(); + + // User has pointed an item, so show it in a different way + if( id > 0 && id <= limit ) + { + current = ( *aCollector )[id - 1]; + highlight( current, BRIGHTENED, &highlightGroup ); + } + else + current = nullptr; + + // User has pointed on the "Select All" option + if( id == limit + 1 ) + { + for( int i = 0; i < aCollector->GetCount(); ++i ) + highlight( ( *aCollector )[i], BRIGHTENED, &highlightGroup ); + selectAll = true; + } + else + selectAll = false; + } + else if( evt->Action() == TA_CHOICE_MENU_CHOICE ) + { + if( selectAll ) + { + for( int i = 0; i < aCollector->GetCount(); ++i ) + unhighlight( ( *aCollector )[i], BRIGHTENED, &highlightGroup ); + } + else if( current ) + unhighlight( current, BRIGHTENED, &highlightGroup ); + + OPT id = evt->GetCommandId(); + + // User has selected the "Select All" option + if( id == limit + 1 ) + { + selectAll = true; + current = nullptr; + } + else if( id == limit + 2 ) + { + expandSelection = true; + selectAll = false; + current = nullptr; + } + // User has selected an item, so this one will be returned + else if( id && ( *id > 0 ) && ( *id <= limit ) ) + { + selectAll = false; + current = ( *aCollector )[*id - 1]; + } + else + { + selectAll = false; + current = nullptr; + } + } + else if( evt->Action() == TA_CHOICE_MENU_CLOSED ) + { + break; + } + } + } while( expandSelection ); + getView()->Remove( &highlightGroup ); if( selectAll ) @@ -2420,7 +2446,7 @@ void SELECTION_TOOL::GuessSelectionCandidates( GENERAL_COLLECTOR& aCollector, { for( BOARD_ITEM* item : rejected ) { - aCollector.Remove( item ); + aCollector.Transfer( item ); } } }