Skip to content

Commit b9109c8

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 e642f04 commit b9109c8

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,15 +523,15 @@ 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)
@@ -547,7 +540,7 @@ private void createSearchTools() {
547540

548541
FindReplaceOverlayAction selectAllAction = new FindReplaceOverlayAction(this::performSelectAll);
549542
selectAllAction.addShortcuts(KeyboardShortcuts.SEARCH_ALL);
550-
searchActions.add(selectAllAction);
543+
commandSupport.registerSearchAction(selectAllAction);
551544
selectAllButton = new AccessibleToolItemBuilder(searchTools).withStyleBits(SWT.PUSH)
552545
.withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_SEARCH_ALL))
553546
.withToolTipText(FindReplaceMessages.FindReplaceOverlay_searchAllButton_toolTip)
@@ -560,7 +553,7 @@ private void createCloseTools() {
560553

561554
FindReplaceOverlayAction closeAction = new FindReplaceOverlayAction(this::close);
562555
closeAction.addShortcuts(KeyboardShortcuts.CLOSE);
563-
commonActions.add(closeAction);
556+
commandSupport.registerCommonAction(closeAction);
564557

565558
// Close button
566559
new AccessibleToolItemBuilder(closeTools).withStyleBits(SWT.PUSH)
@@ -573,7 +566,7 @@ private void createAreaSearchButton() {
573566
FindReplaceOverlaySearchOptionAction searchInSelectionAction = new FindReplaceOverlaySearchOptionAction(SearchOptions.GLOBAL, findReplaceLogic);
574567
searchInSelectionAction.addExecutionListener(this::updateIncrementalSearch);
575568
searchInSelectionAction.addShortcuts(KeyboardShortcuts.OPTION_SEARCH_IN_SELECTION);
576-
commonActions.add(searchInSelectionAction);
569+
commandSupport.registerCommonAction(searchInSelectionAction);
577570
searchInSelectionButton = new AccessibleToolItemBuilder(searchTools).withStyleBits(SWT.CHECK)
578571
.withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_SEARCH_IN_AREA))
579572
.withToolTipText(FindReplaceMessages.FindReplaceOverlay_searchInSelectionButton_toolTip)
@@ -585,7 +578,7 @@ private void createRegexSearchButton() {
585578
findReplaceLogic);
586579
regexAction.addExecutionListener(this::updateIncrementalSearch);
587580
regexAction.addShortcuts(KeyboardShortcuts.OPTION_REGEX);
588-
commonActions.add(regexAction);
581+
commandSupport.registerCommonAction(regexAction);
589582
regexSearchButton = new AccessibleToolItemBuilder(searchTools).withStyleBits(SWT.CHECK)
590583
.withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_FIND_REGEX))
591584
.withToolTipText(FindReplaceMessages.FindReplaceOverlay_regexSearchButton_toolTip)
@@ -601,7 +594,7 @@ private void createCaseSensitiveButton() {
601594
SearchOptions.CASE_SENSITIVE, findReplaceLogic);
602595
caseSensitiveAction.addExecutionListener(this::updateIncrementalSearch);
603596
caseSensitiveAction.addShortcuts(KeyboardShortcuts.OPTION_CASE_SENSITIVE);
604-
commonActions.add(caseSensitiveAction);
597+
commandSupport.registerCommonAction(caseSensitiveAction);
605598
caseSensitiveSearchButton = new AccessibleToolItemBuilder(searchTools).withStyleBits(SWT.CHECK)
606599
.withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_CASE_SENSITIVE))
607600
.withToolTipText(FindReplaceMessages.FindReplaceOverlay_caseSensitiveButton_toolTip)
@@ -613,7 +606,7 @@ private void createWholeWordsButton() {
613606
SearchOptions.WHOLE_WORD, findReplaceLogic);
614607
wholeWordAction.addExecutionListener(this::updateIncrementalSearch);
615608
wholeWordAction.addShortcuts(KeyboardShortcuts.OPTION_WHOLE_WORD);
616-
commonActions.add(wholeWordAction);
609+
commandSupport.registerCommonAction(wholeWordAction);
617610
wholeWordSearchButton = new AccessibleToolItemBuilder(searchTools).withStyleBits(SWT.CHECK)
618611
.withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_WHOLE_WORD))
619612
.withToolTipText(FindReplaceMessages.FindReplaceOverlay_wholeWordsButton_toolTip)
@@ -634,7 +627,7 @@ private void createReplaceTools() {
634627
performSingleReplace();
635628
});
636629
replaceAction.addShortcuts(KeyboardShortcuts.SEARCH_FORWARD);
637-
replaceActions.add(replaceAction);
630+
commandSupport.registerReplaceAction(replaceAction);
638631
replaceButton = new AccessibleToolItemBuilder(replaceTools).withStyleBits(SWT.PUSH)
639632
.withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_REPLACE))
640633
.withToolTipText(FindReplaceMessages.FindReplaceOverlay_replaceButton_toolTip)
@@ -648,7 +641,7 @@ private void createReplaceTools() {
648641
performReplaceAll();
649642
});
650643
replaceAllAction.addShortcuts(KeyboardShortcuts.SEARCH_ALL);
651-
replaceActions.add(replaceAllAction);
644+
commandSupport.registerReplaceAction(replaceAllAction);
652645
replaceAllButton = new AccessibleToolItemBuilder(replaceTools).withStyleBits(SWT.PUSH)
653646
.withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_REPLACE_ALL))
654647
.withToolTipText(FindReplaceMessages.FindReplaceOverlay_replaceAllButton_toolTip)
@@ -754,6 +747,7 @@ private void hideReplace() {
754747
return;
755748
}
756749
customFocusOrder.dispose();
750+
commandSupport.unregisterReplaceActions();
757751
searchBar.forceFocus();
758752
contentAssistReplaceField = null;
759753
replaceBarOpen = false;
@@ -776,8 +770,8 @@ private void createReplaceDialog() {
776770
}
777771

778772
private void initializeReplaceShortcutHandlers() {
779-
registerActionShortcutsAtControl(commonActions, replaceBar);
780-
registerActionShortcutsAtControl(replaceActions, replaceBar);
773+
commandSupport.registerCommonActionShortcutsAtControl(replaceBar);
774+
commandSupport.registerReplaceActionShortcutsAtControl(replaceBar);
781775
}
782776

783777
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 List<FindReplaceOverlayAction> commonActions = new ArrayList<>();
44+
private List<FindReplaceOverlayAction> searchActions = new ArrayList<>();
45+
private 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)