Skip to content

Commit

Permalink
Find/Replace: always show overlay when target is visible #2075 #2046
Browse files Browse the repository at this point in the history 
The FindReplaceOverlay is currently only supposed to be shown when the
target editor is visible. Sometimes it is even hidden unintendedly when
switching focus to the underlying editor, which is not a single editor
but encapsulated in a MultiPageEditorPart, which has different pages.
Switching between the pages of that multi-page part, even to pages
without a text editor, does not recalculate the visibility of the
overlay. The reason for this behavior is a faulty identification of
whether the target part is displayed.

This change reimplements the visibility handling for the
FindReplaceOverlay based on the target part's visibility. It's goal is
to show the overlay if, and only if, the target part is visible, no
matter whether it is encapsulated into a multi-editor part or not.
- It encapsulates all target-visility-related functionality into a
nested class. This class tracks and exposes the visibility state.
- It adds an IPageChangedListener to get notified about page changes in
MultiPageEditorParts.
- It reduces the recalculation of the visibility state to part/page
change cases in which the target part is involved.

Fixes #2075
Fixes #2046
  • Loading branch information
@HeikoKlare
HeikoKlare committed Jul 16, 2024
1 parent ec4db98 commit a5689de
Showing 1 changed file with 75 additions and 46 deletions.
121 changes: 75 additions & 46 deletions ...nch.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlay.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.function.Supplier;

import org.osgi.framework.FrameworkUtil;

Expand Down Expand Up @@ -51,6 +52,8 @@
import org.eclipse.jface.bindings.keys.KeyStroke;
import org.eclipse.jface.dialogs.Dialog;
import org.eclipse.jface.dialogs.IDialogSettings;
import org.eclipse.jface.dialogs.IPageChangedListener;
import org.eclipse.jface.dialogs.PageChangedEvent;
import org.eclipse.jface.layout.GridDataFactory;
import org.eclipse.jface.layout.GridLayoutFactory;
import org.eclipse.jface.resource.JFaceColors;
Expand All @@ -61,15 +64,16 @@
import org.eclipse.jface.text.IFindReplaceTargetExtension;
import org.eclipse.jface.text.ITextViewer;

import org.eclipse.ui.IPartListener;
import org.eclipse.ui.IWorkbenchPage;
import org.eclipse.ui.IPartListener2;
import org.eclipse.ui.IWorkbenchPart;
import org.eclipse.ui.IWorkbenchPartReference;
import org.eclipse.ui.PlatformUI;
import org.eclipse.ui.internal.findandreplace.FindReplaceLogic;
import org.eclipse.ui.internal.findandreplace.FindReplaceMessages;
import org.eclipse.ui.internal.findandreplace.HistoryStore;
import org.eclipse.ui.internal.findandreplace.SearchOptions;
import org.eclipse.ui.internal.findandreplace.status.IFindReplaceStatus;
import org.eclipse.ui.part.MultiPageEditorSite;

import org.eclipse.ui.texteditor.IAbstractTextEditorHelpContextIds;
import org.eclipse.ui.texteditor.StatusTextEditor;
Expand Down Expand Up @@ -108,7 +112,7 @@ private final class KeyboardShortcuts {
private final Map<KeyStroke, Runnable> replaceKeyStrokeHandlers = new HashMap<>();

private FindReplaceLogic findReplaceLogic;
private IWorkbenchPart targetPart;
private final IWorkbenchPart targetPart;
private boolean overlayOpen;
private boolean replaceBarOpen;

Expand Down Expand Up @@ -141,7 +145,7 @@ private final class KeyboardShortcuts {
private Color backgroundToUse;
private Color normalTextForegroundColor;
private boolean positionAtTop = true;
private boolean isTargetVisible = true;
private final TargetPartVisibilityHandler targetPartVisibilityHandler;

public FindReplaceOverlay(Shell parent, IWorkbenchPart part, IFindReplaceTarget target) {
super(parent);
Expand All @@ -150,6 +154,8 @@ public FindReplaceOverlay(Shell parent, IWorkbenchPart part, IFindReplaceTarget
setShellStyle(SWT.MODELESS);
setBlockOnOpen(false);
targetPart = part;
targetPartVisibilityHandler = new TargetPartVisibilityHandler(targetPart, this::getShell, this::close,
this::updatePlacementAndVisibility);
}

@Override
Expand Down Expand Up @@ -219,81 +225,104 @@ public void shellDeactivated(ShellEvent e) {

private PaintListener widgetMovementListener = __ -> updatePlacementAndVisibility();

private IPartListener partListener = new IPartListener() {
private static class TargetPartVisibilityHandler implements IPartListener2, IPageChangedListener {
private final IWorkbenchPart targetPart;
private final IWorkbenchPart topLevelPart;
private final Supplier<Shell> shellProvider;
private final Runnable closeCallback;
private final Runnable placementUpdateCallback;

private boolean isTopLevelVisible = true;
private boolean isNestedLevelVisible = true;

TargetPartVisibilityHandler(IWorkbenchPart targetPart, Supplier<Shell> shellProvider, Runnable closeCallback,
Runnable placementUpdateCallback) {
this.targetPart = targetPart;
this.shellProvider = shellProvider;
this.closeCallback = closeCallback;
this.placementUpdateCallback = placementUpdateCallback;
if (targetPart != null && targetPart.getSite() instanceof MultiPageEditorSite multiEditorSite) {
topLevelPart = multiEditorSite.getMultiPageEditor();
} else {
topLevelPart = targetPart;
}
}

@Override
public void partActivated(IWorkbenchPart part) {
getShell().getDisplay().asyncExec(this::adaptToPartActivationChange);
public void partBroughtToTop(IWorkbenchPartReference partRef) {
if (partRef.getPart(false) == topLevelPart && !isTopLevelVisible) {
this.isTopLevelVisible = true;
shellProvider.get().getDisplay().asyncExec(this::adaptToPartActivationChange);
}
}

@Override
public void partDeactivated(IWorkbenchPart part) {
getShell().getDisplay().asyncExec(this::adaptToPartActivationChange);
public void partVisible(IWorkbenchPartReference partRef) {
if (partRef.getPart(false) == topLevelPart && !isTopLevelVisible) {
this.isTopLevelVisible = true;
shellProvider.get().getDisplay().asyncExec(this::adaptToPartActivationChange);
}
}

@Override
public void partBroughtToTop(IWorkbenchPart part) {
getShell().getDisplay().asyncExec(this::adaptToPartActivationChange);
public void partHidden(IWorkbenchPartReference partRef) {
if (partRef.getPart(false) == topLevelPart && isTopLevelVisible) {
this.isTopLevelVisible = false;
shellProvider.get().getDisplay().asyncExec(this::adaptToPartActivationChange);
}
}

@Override
public void partClosed(IWorkbenchPart part) {
close();
public void partClosed(IWorkbenchPartReference partRef) {
if (partRef.getPart(false) == topLevelPart) {
closeCallback.run();
}
}

@Override
public void partOpened(IWorkbenchPart part) {
// Do nothing
public void pageChanged(PageChangedEvent event) {
if (event.getSource() == topLevelPart) {
boolean isPageVisible = event.getSelectedPage() == targetPart;
if (isNestedLevelVisible != isPageVisible) {
this.isNestedLevelVisible = isPageVisible;
shellProvider.get().getDisplay().asyncExec(this::adaptToPartActivationChange);
}
}
}

private void adaptToPartActivationChange() {
if (getShell() == null || targetPart.getSite().getPart() == null) {
if (shellProvider.get() == null || targetPart.getSite().getPart() == null) {
return;
}
isTargetVisible = isPartCurrentlyDisplayedInPartSash();
updatePlacementAndVisibility();
placementUpdateCallback.run();

if (isTargetVisible) {
if (!isTargetVisible()) {
targetPart.getSite().getShell().setActive();
targetPart.setFocus();
getShell().getDisplay().asyncExec(this::focusTargetWidget);
shellProvider.get().getDisplay().asyncExec(this::focusTargetWidget);
}
}

private void focusTargetWidget() {
if (getShell() == null || targetPart.getSite().getPart() == null) {
if (shellProvider.get() == null || targetPart.getSite().getPart() == null) {
return;
}
if (!(targetPart instanceof StatusTextEditor)) {
return;
if (targetPart instanceof StatusTextEditor textEditor) {
textEditor.getAdapter(ITextViewer.class).getTextWidget().forceFocus();
}
StatusTextEditor textEditor = (StatusTextEditor) targetPart;
Control targetWidget = textEditor.getAdapter(ITextViewer.class).getTextWidget();
}

targetWidget.forceFocus();
public boolean isTargetVisible() {
return isTopLevelVisible && isNestedLevelVisible;
}
};
}

private KeyListener closeOnTargetEscapeListener = KeyListener.keyPressedAdapter(c -> {
if (c.keyCode == SWT.ESC) {
this.close();
}
});

private boolean isPartCurrentlyDisplayedInPartSash() {
IWorkbenchPage activePage = PlatformUI.getWorkbench().getActiveWorkbenchWindow().getActivePage();

// Check if the targetPart is currently displayed on the active page
boolean isPartDisplayed = false;

if (activePage != null) {
IWorkbenchPart activePart = activePage.getActivePart();
if (activePart != null && activePart == targetPart) {
isPartDisplayed = true;
}
}

return isPartDisplayed;
}

/**
* Returns the dialog settings object used to share state between several
* find/replace overlays.
Expand Down Expand Up @@ -386,7 +415,7 @@ private void unbindListeners() {
targetWidget.getShell().removeControlListener(shellMovementListener);
targetWidget.removePaintListener(widgetMovementListener);
targetWidget.removeKeyListener(closeOnTargetEscapeListener);
targetPart.getSite().getPage().removePartListener(partListener);
targetPart.getSite().getPage().removePartListener(targetPartVisibilityHandler);
}
}
}
Expand All @@ -399,7 +428,7 @@ private void bindListeners() {
targetWidget.getShell().addControlListener(shellMovementListener);
targetWidget.addPaintListener(widgetMovementListener);
targetWidget.addKeyListener(closeOnTargetEscapeListener);
targetPart.getSite().getPage().addPartListener(partListener);
targetPart.getSite().getPage().addPartListener(targetPartVisibilityHandler);
}
}

Expand Down Expand Up @@ -849,7 +878,7 @@ private void repositionTextSelection() {
}

private void updatePlacementAndVisibility() {
if (!isTargetVisible) {
if (!targetPartVisibilityHandler.isTargetVisible()) {
getShell().setVisible(false);
return;
}
Expand Down

0 comments on commit a5689de

Please sign in to comment.