Walkthrough additions

jabgui/src/main/java/org/jabref/gui/walkthrough/WalkthroughAction.java

@@ -314,7 +314,7 @@ private Walkthrough createGroupWalkthrough() {
                 new WindowEffect(HighlightEffect.PING),
                 new WindowEffect(mainResolver, HighlightEffect.FULL_SCREEN_DARKEN)
         );
-        String groupName = Localization.lang("Research Papers");
+        String groupName = Localization.lang("Research");
         String addGroup = Localization.lang("Add group");
         String addSelectedEntries = Localization.lang("Add selected entries to this group");
 

jabgui/src/main/java/org/jabref/gui/walkthrough/WalkthroughOverlay.java

@@ -35,8 +35,8 @@ public class WalkthroughOverlay {
     private final Map<Window, WindowOverlay> overlays = new HashMap<>();
     private final Stage stage;
     private final Walkthrough walkthrough;
-    private final WalkthroughHighlighter walkthroughHighlighter;
-    private final WalkthroughReverter walkthroughReverter;
+    private final WalkthroughHighlighter highlighter;
+    private final WalkthroughReverter reverter;
     private final SideEffectExecutor sideEffectExecutor;
 
     private @Nullable WalkthroughScroller scroller;
@@ -48,10 +48,10 @@ public class WalkthroughOverlay {
     public WalkthroughOverlay(Stage stage, Walkthrough walkthrough) {
         this.stage = stage;
         this.walkthrough = walkthrough;
-        this.walkthroughHighlighter = new WalkthroughHighlighter();
+        this.highlighter = new WalkthroughHighlighter();
+        this.highlighter.setOnBackgroundClick(this::showQuitConfirmationAndQuit);
         this.sideEffectExecutor = new SideEffectExecutor();
-        this.walkthroughReverter = new WalkthroughReverter(walkthrough, stage, sideEffectExecutor);
-        this.walkthroughHighlighter.setOnBackgroundClick(this::showQuitConfirmationAndQuit);
+        this.reverter = new WalkthroughReverter(walkthrough, stage, sideEffectExecutor);
     }
 
     public void show(@NonNull WalkthroughStep step) {
@@ -67,7 +67,7 @@ case SideEffect(String title, WalkthroughSideEffect sideEffect) -> {
                 } else {
                     LOGGER.error("Failed to execute side effect: {}", sideEffect.description());
                     LOGGER.warn("Side effect failed for step: {}", title);
-                    revertToPreviousStep();
+                    reverter.findAndUndo();
                 }
             }
             case VisibleComponent component -> {
@@ -83,8 +83,8 @@ case SideEffect(String title, WalkthroughSideEffect sideEffect) -> {
 
     public void detachAll() {
         cleanUp();
-        walkthroughReverter.revertAll();
-        walkthroughHighlighter.detachAll();
+        reverter.revertAll();
+        highlighter.detachAll();
         overlays.values().forEach(WindowOverlay::detach);
         overlays.clear();
     }
@@ -114,7 +114,7 @@ private void handleResolutionResult(WalkthroughResult result) {
             displayWalkthroughStep(result);
         } else {
             LOGGER.error("Failed to resolve node for step '{}'. Reverting.", walkthrough.getCurrentStep().title());
-            revertToPreviousStep();
+            reverter.findAndUndo();
         }
         resolver = null;
     }
@@ -134,7 +134,7 @@ private void displayWalkthroughStep(WalkthroughResult result) {
             this.scroller = new WalkthroughScroller(resolvedNode);
         }
 
-        walkthroughHighlighter.applyHighlight(
+        highlighter.applyHighlight(
                 component.highlight().orElse(null),
                 resolvedWindow.getScene(),
                 resolvedNode);
@@ -146,11 +146,7 @@ private void displayWalkthroughStep(WalkthroughResult result) {
             case PanelStep panel -> overlay.showPanel(panel, resolvedNode, this::prepareForNavigation);
         }
 
-        walkthroughReverter.attach(resolvedWindow, resolvedNode);
-    }
-
-    private void revertToPreviousStep() {
-        walkthroughReverter.findAndUndo();
+        reverter.attach(resolvedWindow, resolvedNode);
     }
 
     private void cleanUp() {
@@ -160,7 +156,7 @@ private void cleanUp() {
             resolver = null;
         }
 
-        walkthroughReverter.detach();
+        reverter.detach();
         if (scroller != null) {
             scroller.cleanup();
             scroller = null;
@@ -189,7 +185,7 @@ private void cleanUp() {
     ///
     /// So the current hack is we will hide [WindowOverlay] so that they won't get hacked restored.
     private void prepareForNavigation() {
-        walkthroughReverter.detach();
+        reverter.detach();
         hide();
     }
 }

jabgui/src/main/java/org/jabref/gui/walkthrough/WalkthroughRenderer.java

@@ -41,11 +41,11 @@ public Node render(TooltipStep step, Walkthrough walkthrough, Runnable beforeNav
         titleContainer.getChildren().add(titleFlow);
 
         VBox contentContainer = createContent(step, walkthrough, beforeNavigate);
-        contentContainer.getStyleClass().add("walkthrough-tooltip-content");
+        contentContainer.getStyleClass().add("walkthrough-content");
         VBox.setVgrow(contentContainer, Priority.ALWAYS);
 
         HBox actionsContainer = createActions(step, walkthrough, beforeNavigate);
-        actionsContainer.getStyleClass().add("walkthrough-tooltip-actions");
+        actionsContainer.getStyleClass().add("walkthrough-actions");
 
         step.maxHeight().ifPresent(tooltip::setMaxHeight);
         step.maxWidth().ifPresent(tooltip::setMaxWidth);

jabgui/src/main/java/org/jabref/gui/walkthrough/WindowOverlay.java

@@ -267,7 +267,8 @@ public void detach() {
     private PopOver createPopover(TooltipStep step, Runnable beforeNavigate) {
         Node content = renderer.render(step, walkthrough, beforeNavigate);
         PopOver popover = new PopOver();
-        popover.getScene().getStylesheets().setAll(window.getScene().getStylesheets());
+        popover.getStyleClass().add("walkthrough-tooltip");
+        popover.getScene().getStylesheets().setAll(window.getScene().getStylesheets()); // NOTE: Hack because ThemeManager cannot consistently apply themes
         popover.setContentNode(content);
         popover.setDetachable(false);
         popover.setCloseButtonEnabled(false);

jabgui/src/main/java/org/jabref/gui/walkthrough/declarative/NodeResolver.java

@@ -6,7 +6,6 @@
 import javafx.scene.Node;
 import javafx.scene.Parent;
 import javafx.scene.Scene;
-import javafx.scene.control.Button;
 import javafx.scene.control.ButtonBase;
 import javafx.scene.control.ButtonType;
 import javafx.scene.control.ContextMenu;
@@ -19,6 +18,7 @@
 import org.jabref.logic.l10n.Localization;
 
 import com.google.common.collect.Streams;
+import com.sun.javafx.scene.NodeHelper;
 import com.sun.javafx.scene.control.LabeledText;
 import org.jspecify.annotations.NonNull;
 import org.jspecify.annotations.Nullable;
@@ -33,55 +33,87 @@ public interface NodeResolver {
     /// @return an optional containing the found node, or empty if not found
     Optional<Node> resolve(@NonNull Scene scene);
 
-    /// Creates a resolver that finds a node by CSS selector.
+    /// Creates a resolver that finds a node by CSS selector. The returned node is
+    /// guaranteed to be visible.
     ///
     /// @param selector the CSS selector to find the node
     /// @return a resolver that finds the node by selector
     static NodeResolver selector(@NonNull String selector) {
-        return scene -> Optional.ofNullable(scene.lookup(selector));
+        return scene -> scene.getRoot().lookupAll(selector).stream().filter(NodeHelper::isTreeVisible).findFirst();
     }
 
-    /// Creates a resolver that finds a node by its fx:id.
+    /// Creates a resolver that finds a node by its fx:id. The returned node is
+    /// guaranteed to be visible.
     ///
     /// @param fxId the fx:id of the node
     /// @return a resolver that finds the node by fx:id
     static NodeResolver fxId(@NonNull String fxId) {
-        return scene -> Optional.ofNullable(scene.lookup("#" + fxId));
+        return selector("#" + fxId);
     }
 
-    /// Creates a resolver that finds a button by its graphic.
+    /// Creates a resolver that finds a button by its graphic. The returned button is
+    /// guaranteed to be visible.
     ///
     /// @param glyph the graphic of the button
     /// @return a resolver that finds the button by graphic
     static NodeResolver buttonWithGraphic(IconTheme.JabRefIcons glyph) {
-        // .icon-button, .button selector is not used, because lookupAll doesn't support multiple selectors
-        return scene -> Streams.concat(scene.getRoot().lookupAll(".button").stream(),
-                                       scene.getRoot().lookupAll(".icon-button").stream())
-                               .filter(node -> {
-                                   if (!(node instanceof ButtonBase button)) {
-                                       return false;
-                                   }
-                                   Node graphic = button.getGraphic();
-                                   return (graphic instanceof JabRefIconView jabRefIconView) && jabRefIconView.getGlyph() == glyph ||
-                                           (graphic instanceof FontIcon fontIcon) && fontIcon.getIconCode() == glyph.getIkon();
-                               })
-                               .findFirst();
+        return scene -> Streams
+                // .icon-button, .button selector is not used, because lookupAll doesn't support multiple selectors
+                .concat(scene.getRoot().lookupAll(".button").stream(),
+                        scene.getRoot().lookupAll(".icon-button").stream())
+                .filter(node -> {
+                    if (!(node instanceof ButtonBase button) || !NodeHelper.isTreeVisible(button)) {
+                        return false;
+                    }
+                    Node graphic = button.getGraphic();
+                    return (graphic instanceof JabRefIconView jabRefIconView) && jabRefIconView.getGlyph() == glyph ||
+                            (graphic instanceof FontIcon fontIcon) && fontIcon.getIconCode() == glyph.getIkon();
+                })
+                .findFirst();
     }
 
-    /// Creates a resolver that finds a node by a predicate.
+    /// Creates a resolver that finds a node by a predicate. The returned node is
+    /// guaranteed to be visible.
     ///
     /// @param predicate the predicate to match the node
     /// @return a resolver that finds the node matching the predicate
     static NodeResolver predicate(@NonNull Predicate<Node> predicate) {
-        return scene -> Optional.ofNullable(findNode(scene.getRoot(), predicate));
+        return scene -> Optional.ofNullable(findNode(scene.getRoot(),
+                node -> NodeHelper.isTreeVisible(node) && predicate.test(node)));
     }
 
-    /// Creates a resolver that finds a button by its StandardAction.
+    /// Creates a resolver that finds a button by its StandardAction. The button is
+    /// matched by its tooltip text or button text. The returned button is guaranteed to
+    /// be visible.
     ///
     /// @param action the StandardAction associated with the button
     /// @return a resolver that finds the button by action
     static NodeResolver action(@NonNull StandardActions action) {
-        return scene -> Optional.ofNullable(findNodeByAction(scene, action));
+        return scene -> Optional.ofNullable(findNode(scene.getRoot(), node -> {
+            if (!(node instanceof ButtonBase button) || !NodeHelper.isTreeVisible(button)) {
+                return false;
+            }
+
+            if (button.getTooltip() != null) {
+                String tooltipText = button.getTooltip().getText();
+                if (tooltipText != null && tooltipText.equals(action.getText())) {
+                    return true;
+                }
+            }
+
+            if (button.getStyleClass().contains("icon-button")) {
+                String actionText = action.getText();
+                if (button.getTooltip() != null && button.getTooltip().getText() != null) {
+                    String tooltipText = button.getTooltip().getText();
+                    if (tooltipText.startsWith(actionText) || tooltipText.contains(actionText)) {
+                        return true;
+                    }
+                }
+                return button.getText() != null && button.getText().equals(actionText);
+            }
+
+            return false;
+        }));
     }
 
     /// Creates a resolver that finds a button by its button type, assuming the node
@@ -95,14 +127,20 @@ static NodeResolver buttonType(@NonNull ButtonType buttonType) {
                 .map(node -> node instanceof DialogPane pane ? pane.lookupButton(buttonType) : null);
     }
 
-    /// Creates a resolver that finds a node by selector first, then predicate.
+    /// Creates a resolver that finds a node by selector first, then matches text
+    /// content in the node and the node's LabeledText children. The returned node is
+    /// guaranteed to be visible.
     ///
     /// @param selector    the style class to match
     /// @param textMatcher predicate to match text content in LabeledText children
     /// @return a resolver that finds the node by style class and text content
     static NodeResolver selectorWithText(@NonNull String selector, @NonNull Predicate<String> textMatcher) {
-        return scene -> scene.getRoot().lookupAll(selector).stream().filter(
-                node -> textMatcher.test(node.toString()) || node.lookupAll(".text").stream().anyMatch(child -> {
+        return scene -> scene
+                .getRoot()
+                .lookupAll(selector)
+                .stream()
+                .filter(NodeHelper::isTreeVisible)
+                .filter(node -> textMatcher.test(node.toString()) || node.lookupAll(".text").stream().anyMatch(child -> {
                             if (child instanceof LabeledText text) {
                                 String textContent = text.getText();
                                 return textContent != null && textMatcher.test(textContent);
@@ -127,6 +165,7 @@ static NodeResolver menuItem(@NonNull String key) {
             }
 
             return menu.getItems().stream()
+                       .filter(item -> NodeHelper.isTreeVisible(item.getStyleableNode()))
                        .filter(item -> Optional
                                .ofNullable(item.getText())
                                .map(str -> str.contains(Localization.lang(key)))
@@ -135,33 +174,6 @@ static NodeResolver menuItem(@NonNull String key) {
         };
     }
 
-    @Nullable
-    private static Node findNodeByAction(@NonNull Scene scene, @NonNull StandardActions action) {
-        return findNode(scene.getRoot(), node -> {
-            if (node instanceof Button button) {
-                if (button.getTooltip() != null) {
-                    String tooltipText = button.getTooltip().getText();
-                    if (tooltipText != null && tooltipText.equals(action.getText())) {
-                        return true;
-                    }
-                }
-
-                if (button.getStyleClass().contains("icon-button")) {
-                    String actionText = action.getText();
-                    if (button.getTooltip() != null && button.getTooltip().getText() != null) {
-                        String tooltipText = button.getTooltip().getText();
-                        if (tooltipText.startsWith(actionText) || tooltipText.contains(actionText)) {
-                            return true;
-                        }
-                    }
-
-                    return button.getText() != null && button.getText().equals(actionText);
-                }
-            }
-            return false;
-        });
-    }
-
     @Nullable
     private static Node findNode(@NonNull Node root, @NonNull Predicate<Node> predicate) {
         if (predicate.test(root)) {