Find/Replace overlay: decouple toggle button state from action execution#4164
Open
HeikoKlare wants to merge 1 commit into
Open
Conversation
HeikoKlare
force-pushed
the
findreplace-overlay-search-option-observer
branch
from
b79b43a to
b399ccb
Compare
Contributor
HeikoKlare
force-pushed
the
findreplace-overlay-search-option-observer
branch
2 times, most recently
from
608e39e to
fc9b306
Compare
There was a problem hiding this comment.
Pull request overview
Refactors the Find/Replace overlay toggle buttons to decouple UI selection state from action execution by introducing a listener/observer mechanism in IFindReplaceLogic/FindReplaceLogic, and wiring tool items to update their selection state from model changes.
Changes:
- Added
addSearchOptionChangedListener(...)to the logic layer and notify observers on option activation/deactivation. - Updated overlay toggle button construction to bind ToolItem selection state to the model (including inverted mapping for “search in selection” vs
GLOBAL). - Added/updated UI and logic tests to cover observer notifications and the
GLOBALinversion behavior.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/OverlayAccess.java | Adjusts test UI access helpers to trigger option changes via selection events. |
| tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayTest.java | Adds an overlay-level test for GLOBAL vs “search in selection” inversion. |
| tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/FindReplaceLogicTest.java | Adds tests for search option change listener notifications. |
| bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlay.java | Converts toggle operations to pure model commands and binds ToolItems to model state. |
| bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/AccessibleToolItemBuilder.java | Adds ToolItem-to-model binding helpers (withSearchOption, inverted variant). |
| bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/AccessibleToolItem.java | Simplifies operation execution; removes prior Enter-key traverse handling. |
| bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/IFindReplaceLogic.java | Adds listener registration API for option state changes. |
| bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/FindReplaceLogic.java | Implements listener storage and notification on activate/deactivate. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
HeikoKlare
force-pushed
the
findreplace-overlay-search-option-observer
branch
2 times, most recently
from
95c7cdb to
3422358
Compare
Previously, checkbox tool items in the Find/Replace overlay managed their own visual state by reading or flipping the SWT ToolItem selection inside the action lambda. This coupled the UI state update to the execution path of the action, making the action responsible for two unrelated concerns: toggling a search option in the model and keeping a widget in sync with that state. FindReplaceLogic now exposes an addSearchOptionChangedListener mechanism that notifies observers whenever a search option is activated or deactivated. AccessibleToolItemBuilder gains withSearchOption and withInvertedSearchOption methods that bind a ToolItem's selection state directly to a SearchOptions value: the initial state is read from the logic at build time, and subsequent changes are pushed to the item automatically via the observer. The action operations for checkbox buttons are reduced to pure model commands and no longer touch widget state at all. Tests for the new addSearchOptionChangedListener API are added to FindReplaceLogicTest, and a dedicated overlay test verifies the correct inversion of the searchInSelection button relative to the GLOBAL option through both transitions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
HeikoKlare
force-pushed
the
findreplace-overlay-search-option-observer
branch
from
3422358 to
d4128ab
Compare
|
Will this rework also allow to fix #2651 ? This is: Find/Replace Overlay: "Content Assist Available" ctrl+space does not work in .java files |
Contributor
Author
I hope that fixing the mentioned issue will become possibly (more easily/cleanly) with the rework I plan to do, yes. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The Find/Replace overlay contains several toggle buttons for search options (case sensitivity, regex mode, whole word, search in selection). Previously, these buttons were responsible not only for triggering a model change but also for updating their own visual selection state from within the operation that was executed when clicked. This meant the operation had to be aware of the widget it belonged to, creating a tight coupling between the view and the action logic.
This change introduces a proper observer pattern: the logic layer now notifies registered observers whenever a search option is activated or deactivated. When the toggle buttons are constructed, they register themselves as observers for their respective options, so their visual state is updated automatically in response to model changes. The action operations for these buttons are thereby reduced to pure model commands with no knowledge of any UI element.
Beyond the cleaner architecture, this also fixes several an existing issue: the state update was applied inconsistently depending on whether the button was activated via mouse click or keyboard, because the two activation paths went through different code.
Tests for the observer notification mechanism are added to FindReplaceLogicTest.
This is the first in a series of changes improving the separation of concerns in the Find/Replace overlay, addressing #1912. The subsequent changes build on this foundation to ultimately replace the overlay's hard-coded SWT-level key processing with proper Eclipse command and key binding infrastructure, making all overlay shortcuts visible and configurable through the standard Preferences > Keys dialog.