Skip to content

Commit c3e5d57

Browse files
HeikoKlareHeikoKlare
committed
Find/replace overlay: move action management to command class
The management of actions for the find/replace overlay, the registration of key shortcuts and the initialization of according tooltips with the shortcuts is currently part of the FindReplaceOverlay itself. In addition, the tooltip texts are static throughout the overlay lifecycle and show conflicting shortcuts, such as Enter for both the "search forward" and "replace" button, even though only either of them is registered, depending on which of the input fields has focus. With this change, the responsibility for the action management with the activation and deactivation of their key bindings is moved to the FindReplaceOverlayCommandSupport. This prepares for the provision of the actions as Eclipse handlers in order to allow rebinding shortcuts. Contributes to #2015
1 parent 622774c commit c3e5d57

4 files changed

Lines changed: 109 additions & 41 deletions

File tree

‎bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/AccessibleToolItem.java‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@
2323
class AccessibleToolItem {
2424
private final ToolItem toolItem;
2525

26-
private FindReplaceOverlayAction action;
26+
private String baseToolTipText;
27+
2728

2829
AccessibleToolItem(Composite parent, int styleBits) {
2930
ToolBar toolbar = new ToolBar(parent, SWT.FLAT | SWT.HORIZONTAL);
@@ -44,13 +45,22 @@ void setImage(Image image) {
4445
}
4546

4647
void setToolTipText(String text) {
47-
toolItem.setToolTipText(action != null ? action.addShortcutHintToTooltipText(text) : text);
48+
this.baseToolTipText = text;
49+
toolItem.setToolTipText(text);
4850
}
4951

50-
void setAction(FindReplaceOverlayAction newAction) {
51-
this.action = newAction;
52+
void setAction(FindReplaceOverlayAction action) {
5253
setToolTipText(toolItem.getToolTipText());
5354
toolItem.addSelectionListener(SelectionListener.widgetSelectedAdapter(__ -> action.execute()));
55+
action.addShortcutHintListener(hint -> {
56+
if (!toolItem.isDisposed()) {
57+
String tooltipWithHint = baseToolTipText;
58+
if (!hint.isEmpty()) {
59+
tooltipWithHint += " (" + hint + ")"; //$NON-NLS-1$//$NON-NLS-2$
60+
}
61+
toolItem.setToolTipText(tooltipWithHint);
62+
}
63+
});
5464
}
5565

5666
}

‎bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlay.java‎

Lines changed: 22 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
*******************************************************************************/
1414
package org.eclipse.ui.internal.findandreplace.overlay;
1515

16-
import java.util.ArrayList;
1716
import java.util.List;
1817
import java.util.concurrent.atomic.AtomicReference;
1918

@@ -136,10 +135,6 @@ private static final class KeyboardShortcuts {
136135
private ToolItem replaceButton;
137136
private ToolItem replaceAllButton;
138137

139-
private final List<FindReplaceOverlayAction> commonActions = new ArrayList<>();
140-
private final List<FindReplaceOverlayAction> searchActions = new ArrayList<>();
141-
private final List<FindReplaceOverlayAction> replaceActions = new ArrayList<>();
142-
143138
private Color widgetBackgroundColor;
144139
private Color overlayBackgroundColor;
145140
private Color normalTextForegroundColor;
@@ -154,12 +149,16 @@ private static final class KeyboardShortcuts {
154149
private final FocusListener targetActionActivationHandling = new FocusListener() {
155150
@Override
156151
public void focusGained(FocusEvent e) {
157-
commandSupport.overlayActivated();
152+
if (e.widget == searchBar.getTextBar()) {
153+
commandSupport.searchBarActivated();
154+
} else if (replaceBar != null && e.widget == replaceBar.getTextBar()) {
155+
commandSupport.replaceBarActivated();
156+
}
158157
}
159158

160159
@Override
161160
public void focusLost(FocusEvent e) {
162-
commandSupport.overlayDeactivated();
161+
commandSupport.searchOrReplaceBarDeactivated();
163162
}
164163
};
165164

@@ -415,14 +414,8 @@ private void createContainerAndSearchControls(Composite parent) {
415414
}
416415

417416
private void initializeSearchShortcutHandlers() {
418-
registerActionShortcutsAtControl(commonActions, searchBar);
419-
registerActionShortcutsAtControl(searchActions, searchBar);
420-
}
421-
422-
private void registerActionShortcutsAtControl(List<FindReplaceOverlayAction> actions, Control control) {
423-
for (FindReplaceOverlayAction action : actions) {
424-
FindReplaceShortcutUtil.registerActionShortcutsAtControl(action, control);
425-
}
417+
commandSupport.registerCommonActionShortcutsAtControl(searchBar);
418+
commandSupport.registerSearchActionShortcutsAtControl(searchBar);
426419
}
427420

428421
/**
@@ -498,7 +491,7 @@ private void createReplaceToggle() {
498491

499492
FindReplaceOverlayAction replaceToggleAction = new FindReplaceOverlayAction(() -> setReplaceVisible(!replaceBarOpen));
500493
replaceToggleAction.addShortcuts(KeyboardShortcuts.TOGGLE_REPLACE);
501-
commonActions.add(replaceToggleAction);
494+
commandSupport.registerCommonAction(replaceToggleAction);
502495

503496
replaceToggle = new AccessibleToolItemBuilder(replaceToggleTools)
504497
.withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_OPEN_REPLACE_AREA))
@@ -530,23 +523,23 @@ private void createSearchTools() {
530523

531524
FindReplaceOverlayAction searchBackwardAction = new FindReplaceOverlayAction(() -> performSearch(false));
532525
searchBackwardAction.addShortcuts(KeyboardShortcuts.SEARCH_BACKWARD);
533-
searchActions.add(searchBackwardAction);
526+
commandSupport.registerSearchAction(searchBackwardAction);
534527
searchBackwardButton = new AccessibleToolItemBuilder(searchTools).withStyleBits(SWT.PUSH)
535528
.withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_FIND_PREV))
536529
.withToolTipText(FindReplaceMessages.FindReplaceOverlay_upSearchButton_toolTip)
537530
.withAction(searchBackwardAction).build();
538531

539532
FindReplaceOverlayAction searchForwardAction = new FindReplaceOverlayAction(() -> performSearch(true));
540533
searchForwardAction.addShortcuts(KeyboardShortcuts.SEARCH_FORWARD);
541-
searchActions.add(searchForwardAction);
534+
commandSupport.registerSearchAction(searchForwardAction);
542535
searchForwardButton = new AccessibleToolItemBuilder(searchTools).withStyleBits(SWT.PUSH)
543536
.withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_FIND_NEXT))
544537
.withToolTipText(FindReplaceMessages.FindReplaceOverlay_downSearchButton_toolTip)
545538
.withAction(searchForwardAction).build();
546539

547540
FindReplaceOverlayAction selectAllAction = new FindReplaceOverlayAction(this::performSelectAll);
548541
selectAllAction.addShortcuts(KeyboardShortcuts.SEARCH_ALL);
549-
searchActions.add(selectAllAction);
542+
commandSupport.registerSearchAction(selectAllAction);
550543
selectAllButton = new AccessibleToolItemBuilder(searchTools).withStyleBits(SWT.PUSH)
551544
.withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_SEARCH_ALL))
552545
.withToolTipText(FindReplaceMessages.FindReplaceOverlay_searchAllButton_toolTip)
@@ -559,7 +552,7 @@ private void createCloseTools() {
559552

560553
FindReplaceOverlayAction closeAction = new FindReplaceOverlayAction(this::close);
561554
closeAction.addShortcuts(KeyboardShortcuts.CLOSE);
562-
commonActions.add(closeAction);
555+
commandSupport.registerCommonAction(closeAction);
563556

564557
// Close button
565558
new AccessibleToolItemBuilder(closeTools).withStyleBits(SWT.PUSH)
@@ -572,7 +565,7 @@ private void createAreaSearchButton() {
572565
FindReplaceOverlaySearchOptionAction searchInSelectionAction = new FindReplaceOverlaySearchOptionAction(SearchOptions.GLOBAL, findReplaceLogic);
573566
searchInSelectionAction.addExecutionListener(this::updateIncrementalSearch);
574567
searchInSelectionAction.addShortcuts(KeyboardShortcuts.OPTION_SEARCH_IN_SELECTION);
575-
commonActions.add(searchInSelectionAction);
568+
commandSupport.registerCommonAction(searchInSelectionAction);
576569
searchInSelectionButton = new AccessibleToolItemBuilder(searchTools).withStyleBits(SWT.CHECK)
577570
.withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_SEARCH_IN_AREA))
578571
.withToolTipText(FindReplaceMessages.FindReplaceOverlay_searchInSelectionButton_toolTip)
@@ -584,7 +577,7 @@ private void createRegexSearchButton() {
584577
findReplaceLogic);
585578
regexAction.addExecutionListener(this::updateIncrementalSearch);
586579
regexAction.addShortcuts(KeyboardShortcuts.OPTION_REGEX);
587-
commonActions.add(regexAction);
580+
commandSupport.registerCommonAction(regexAction);
588581
regexSearchButton = new AccessibleToolItemBuilder(searchTools).withStyleBits(SWT.CHECK)
589582
.withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_FIND_REGEX))
590583
.withToolTipText(FindReplaceMessages.FindReplaceOverlay_regexSearchButton_toolTip)
@@ -600,7 +593,7 @@ private void createCaseSensitiveButton() {
600593
SearchOptions.CASE_SENSITIVE, findReplaceLogic);
601594
caseSensitiveAction.addExecutionListener(this::updateIncrementalSearch);
602595
caseSensitiveAction.addShortcuts(KeyboardShortcuts.OPTION_CASE_SENSITIVE);
603-
commonActions.add(caseSensitiveAction);
596+
commandSupport.registerCommonAction(caseSensitiveAction);
604597
caseSensitiveSearchButton = new AccessibleToolItemBuilder(searchTools).withStyleBits(SWT.CHECK)
605598
.withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_CASE_SENSITIVE))
606599
.withToolTipText(FindReplaceMessages.FindReplaceOverlay_caseSensitiveButton_toolTip)
@@ -612,7 +605,7 @@ private void createWholeWordsButton() {
612605
SearchOptions.WHOLE_WORD, findReplaceLogic);
613606
wholeWordAction.addExecutionListener(this::updateIncrementalSearch);
614607
wholeWordAction.addShortcuts(KeyboardShortcuts.OPTION_WHOLE_WORD);
615-
commonActions.add(wholeWordAction);
608+
commandSupport.registerCommonAction(wholeWordAction);
616609
wholeWordSearchButton = new AccessibleToolItemBuilder(searchTools).withStyleBits(SWT.CHECK)
617610
.withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_WHOLE_WORD))
618611
.withToolTipText(FindReplaceMessages.FindReplaceOverlay_wholeWordsButton_toolTip)
@@ -633,7 +626,7 @@ private void createReplaceTools() {
633626
performSingleReplace();
634627
});
635628
replaceAction.addShortcuts(KeyboardShortcuts.SEARCH_FORWARD);
636-
replaceActions.add(replaceAction);
629+
commandSupport.registerReplaceAction(replaceAction);
637630
replaceButton = new AccessibleToolItemBuilder(replaceTools).withStyleBits(SWT.PUSH)
638631
.withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_REPLACE))
639632
.withToolTipText(FindReplaceMessages.FindReplaceOverlay_replaceButton_toolTip)
@@ -647,7 +640,7 @@ private void createReplaceTools() {
647640
performReplaceAll();
648641
});
649642
replaceAllAction.addShortcuts(KeyboardShortcuts.SEARCH_ALL);
650-
replaceActions.add(replaceAllAction);
643+
commandSupport.registerReplaceAction(replaceAllAction);
651644
replaceAllButton = new AccessibleToolItemBuilder(replaceTools).withStyleBits(SWT.PUSH)
652645
.withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_REPLACE_ALL))
653646
.withToolTipText(FindReplaceMessages.FindReplaceOverlay_replaceAllButton_toolTip)
@@ -753,6 +746,7 @@ private void hideReplace() {
753746
return;
754747
}
755748
customFocusOrder.dispose();
749+
commandSupport.unregisterReplaceActions();
756750
searchBar.forceFocus();
757751
contentAssistReplaceField = null;
758752
replaceBarOpen = false;
@@ -775,8 +769,8 @@ private void createReplaceDialog() {
775769
}
776770

777771
private void initializeReplaceShortcutHandlers() {
778-
registerActionShortcutsAtControl(commonActions, replaceBar);
779-
registerActionShortcutsAtControl(replaceActions, replaceBar);
772+
commandSupport.registerCommonActionShortcutsAtControl(replaceBar);
773+
commandSupport.registerReplaceActionShortcutsAtControl(replaceBar);
780774
}
781775

782776
private void enableSearchTools(boolean enable) {

‎bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayAction.java‎

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import java.util.ArrayList;
1414
import java.util.Collections;
1515
import java.util.List;
16+
import java.util.function.Consumer;
1617

1718
import org.eclipse.jface.bindings.keys.KeyStroke;
1819

@@ -21,6 +22,8 @@ class FindReplaceOverlayAction {
2122

2223
private final List<Runnable> executionListeners = new ArrayList<>();
2324

25+
private final List<Consumer<String>> shortcutHintListeners = new ArrayList<>();
26+
2427
private final List<KeyStroke> shortcuts = new ArrayList<>();
2528

2629
FindReplaceOverlayAction(Runnable operation) {
@@ -48,13 +51,6 @@ boolean executeIfMatching(KeyStroke keystroke) {
4851
return false;
4952
}
5053

51-
String addShortcutHintToTooltipText(String originalTooltipText) {
52-
if (shortcuts.isEmpty()) {
53-
return originalTooltipText;
54-
}
55-
return originalTooltipText + " (" + shortcuts.get(0).format() + ")"; //$NON-NLS-1$ //$NON-NLS-2$
56-
}
57-
5854
void addExecutionListener(Runnable listener) {
5955
executionListeners.add(listener);
6056
}
@@ -65,4 +61,23 @@ void notifyExecutionListeners() {
6561
}
6662
}
6763

64+
void addShortcutHintListener(Consumer<String> listener) {
65+
shortcutHintListeners.add(listener);
66+
}
67+
68+
void activateKeyBinding() {
69+
shortcutHintListeners.forEach(listener -> listener.accept(getShortcutHint()));
70+
}
71+
72+
private String getShortcutHint() {
73+
if (shortcuts.isEmpty()) {
74+
return ""; //$NON-NLS-1$
75+
}
76+
return shortcuts.get(0).format(); // $NON-NLS-1$
77+
}
78+
79+
void deactivateKeyBinding() {
80+
shortcutHintListeners.forEach(listener -> listener.accept("")); //$NON-NLS-1$
81+
}
82+
6883
}

‎bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayCommandSupport.java‎

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,13 @@
1111
package org.eclipse.ui.internal.findandreplace.overlay;
1212

1313
import java.lang.reflect.Method;
14+
import java.util.ArrayList;
1415
import java.util.HashMap;
1516
import java.util.List;
1617
import java.util.Map;
1718

19+
import org.eclipse.swt.widgets.Control;
20+
1821
import org.eclipse.core.runtime.ILog;
1922

2023
import org.eclipse.jface.action.IAction;
@@ -37,15 +40,61 @@ class FindReplaceOverlayCommandSupport {
3740
private final IWorkbenchPart targetPart;
3841
private DeactivateGlobalActionHandlers globalActionHandlerDeaction;
3942

43+
private final List<FindReplaceOverlayAction> commonActions = new ArrayList<>();
44+
private final List<FindReplaceOverlayAction> searchActions = new ArrayList<>();
45+
private final List<FindReplaceOverlayAction> replaceActions = new ArrayList<>();
46+
4047
FindReplaceOverlayCommandSupport(IWorkbenchPart targetPart) {
4148
this.targetPart = targetPart;
4249
}
4350

44-
void overlayActivated() {
51+
void registerCommonAction(FindReplaceOverlayAction action) {
52+
this.commonActions.add(action);
53+
}
54+
55+
void registerSearchAction(FindReplaceOverlayAction action) {
56+
this.searchActions.add(action);
57+
}
58+
59+
void registerReplaceAction(FindReplaceOverlayAction action) {
60+
this.replaceActions.add(action);
61+
}
62+
63+
void unregisterReplaceActions() {
64+
this.replaceActions.clear();
65+
}
66+
67+
void registerCommonActionShortcutsAtControl(Control control) {
68+
commonActions.forEach(action -> FindReplaceShortcutUtil.registerActionShortcutsAtControl(action, control));
69+
}
70+
71+
void registerSearchActionShortcutsAtControl(Control control) {
72+
searchActions.forEach(action -> FindReplaceShortcutUtil.registerActionShortcutsAtControl(action, control));
73+
}
74+
75+
void registerReplaceActionShortcutsAtControl(Control control) {
76+
replaceActions.forEach(action -> FindReplaceShortcutUtil.registerActionShortcutsAtControl(action, control));
77+
}
78+
79+
void searchBarActivated() {
80+
searchOrReplaceBarActivated();
81+
searchActions.forEach(FindReplaceOverlayAction::activateKeyBinding);
82+
}
83+
84+
void replaceBarActivated() {
85+
searchOrReplaceBarActivated();
86+
replaceActions.forEach(FindReplaceOverlayAction::activateKeyBinding);
87+
}
88+
89+
private void searchOrReplaceBarActivated() {
4590
setTextEditorActionsActivated(false);
91+
commonActions.forEach(FindReplaceOverlayAction::activateKeyBinding);
4692
}
4793

48-
void overlayDeactivated() {
94+
void searchOrReplaceBarDeactivated() {
95+
commonActions.forEach(FindReplaceOverlayAction::deactivateKeyBinding);
96+
searchActions.forEach(FindReplaceOverlayAction::deactivateKeyBinding);
97+
replaceActions.forEach(FindReplaceOverlayAction::deactivateKeyBinding);
4998
setTextEditorActionsActivated(true);
5099
}
51100

0 commit comments

Comments
 (0)