Skip to content

Commit f00faf6

Browse files
HeikoKlareCopilot
andcommitted
Find/Replace overlay: use handler enablement for button enabled state
The enablement of the buttons for search options of the find/replace overlay are currently derived from the availability of the underlying search option. However, there are actions attached to the buttons (changing activation of the search option) and the button enablement should actually reflect if that action is available or not. Though these are effectively equal, mapping the button enablement to the action availability is the conceptually more correct representation. With this change, FindReplaceOverlayAction extends AbstractHandler, making the action itself a handler and providing and enablement state. Enablement of the buttons for the search options is not kept in sync with enablement of the action/handler via an according handler listener. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 3cc77fd commit f00faf6

3 files changed

Lines changed: 28 additions & 8 deletions

File tree

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

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,16 @@
1818
import org.eclipse.swt.widgets.ToolBar;
1919
import org.eclipse.swt.widgets.ToolItem;
2020

21+
import org.eclipse.core.commands.HandlerEvent;
22+
import org.eclipse.core.commands.IHandlerListener;
23+
2124
import org.eclipse.jface.layout.GridDataFactory;
2225

2326
class AccessibleToolItem {
2427
private final ToolItem toolItem;
2528

2629
private String baseToolTipText;
2730

28-
2931
AccessibleToolItem(Composite parent, int styleBits) {
3032
ToolBar toolbar = new ToolBar(parent, SWT.FLAT | SWT.HORIZONTAL);
3133
GridDataFactory.fillDefaults().grab(true, true).align(SWT.CENTER, SWT.CENTER).applyTo(toolbar);
@@ -51,6 +53,15 @@ void setToolTipText(String text) {
5153

5254
void setAction(FindReplaceOverlayAction action) {
5355
setToolTipText(toolItem.getToolTipText());
56+
toolItem.setEnabled(action.isEnabled());
57+
action.addHandlerListener(new IHandlerListener() {
58+
@Override
59+
public void handlerChanged(HandlerEvent event) {
60+
if (event.isEnabledChanged() && !toolItem.isDisposed()) {
61+
toolItem.setEnabled(action.isEnabled());
62+
}
63+
}
64+
});
5465
toolItem.addSelectionListener(SelectionListener.widgetSelectedAdapter(__ -> action.execute()));
5566
action.addShortcutHintListener(hint -> {
5667
if (!toolItem.isDisposed()) {

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

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -104,12 +104,8 @@ public ToolItem build() {
104104
toolItem.setSelection(invertSearchOption ? !state : state);
105105
}
106106
});
107-
toolItem.setEnabled(findReplaceLogic.isAvailable(searchOption));
108-
findReplaceLogic.addSearchOptionAvailabilityChangedListener(searchOption, state -> {
109-
if (!toolItem.isDisposed()) {
110-
toolItem.setEnabled(state);
111-
}
112-
});
107+
searchOptionAction.setAvailable(findReplaceLogic.isAvailable(searchOption));
108+
findReplaceLogic.addSearchOptionAvailabilityChangedListener(searchOption, searchOptionAction::setAvailable);
113109
return toolItem;
114110
}
115111
}

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

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,12 @@
1515
import java.util.List;
1616
import java.util.function.Consumer;
1717

18+
import org.eclipse.core.commands.AbstractHandler;
19+
import org.eclipse.core.commands.ExecutionEvent;
20+
1821
import org.eclipse.jface.bindings.keys.KeyStroke;
1922

20-
class FindReplaceOverlayAction {
23+
class FindReplaceOverlayAction extends AbstractHandler {
2124
private final Runnable operation;
2225

2326
private final List<Runnable> executionListeners = new ArrayList<>();
@@ -34,11 +37,21 @@ void addShortcuts(List<KeyStroke> shortcutsToAdd) {
3437
this.shortcuts.addAll(shortcutsToAdd);
3538
}
3639

40+
@Override
41+
public Object execute(ExecutionEvent event) {
42+
execute();
43+
return null;
44+
}
45+
3746
void execute() {
3847
operation.run();
3948
notifyExecutionListeners();
4049
}
4150

51+
void setAvailable(boolean available) {
52+
setBaseEnabled(available);
53+
}
54+
4255
List<KeyStroke> getShortcuts() {
4356
return Collections.unmodifiableList(shortcuts);
4457
}

0 commit comments

Comments
 (0)