diff --git a/src/main/java/com/example/workutils/system/ui/MainWindow.java b/src/main/java/com/example/workutils/system/ui/MainWindow.java index c206ed2..059a17a 100644 --- a/src/main/java/com/example/workutils/system/ui/MainWindow.java +++ b/src/main/java/com/example/workutils/system/ui/MainWindow.java @@ -54,6 +54,7 @@ public class MainWindow { } public void refreshNavItems(List newItems) { + embeddedCache.clear(); navItems = new ArrayList<>(newItems); navigationRail.setItems(navItems); showWelcomePage(); @@ -155,7 +156,10 @@ public class MainWindow { if (item.action != null) { item.action.run(); } - showEmbeddedFor(item); + if (!showEmbeddedFor(item)) { + navigationRail.clearSelection(); + showWelcomePage(); + } } private void showPluginToolbar(String title) { @@ -200,15 +204,17 @@ public class MainWindow { currentEmbedded = null; } - private void showEmbeddedFor(NavItem item) { + private boolean showEmbeddedFor(NavItem item) { if (item.pluginId == null) { hidePluginToolbar(); - return; + return false; } EmbeddedEntry entry = embeddedCache.get(item.pluginId); if (entry != null) { switchToEmbedded(item.pluginId, entry); + return true; } + return false; } public void embedPlugin(String pluginId, String ignoredIcon, String name, Parent content) { diff --git a/src/test/java/com/example/workutils/system/ui/LightThemeCssRuntimeTest.java b/src/test/java/com/example/workutils/system/ui/LightThemeCssRuntimeTest.java index 21ec4cb..7a6d482 100644 --- a/src/test/java/com/example/workutils/system/ui/LightThemeCssRuntimeTest.java +++ b/src/test/java/com/example/workutils/system/ui/LightThemeCssRuntimeTest.java @@ -1,5 +1,8 @@ package com.example.workutils.system.ui; +import javafx.beans.property.SimpleStringProperty; +import javafx.collections.ListChangeListener; +import javafx.collections.ObservableList; import javafx.css.CssParser; import javafx.scene.Scene; import javafx.scene.control.Button; @@ -19,35 +22,48 @@ import javafx.scene.control.TreeItem; import javafx.scene.control.TreeView; import javafx.scene.layout.Region; import javafx.scene.layout.VBox; -import javafx.beans.property.SimpleStringProperty; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import java.io.IOException; +import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; import java.util.List; +import java.util.concurrent.atomic.AtomicInteger; import static com.example.workutils.system.ui.FxTestSupport.fx; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertTrue; class LightThemeCssRuntimeTest { + @TempDir + Path tempDir; + + private final AtomicInteger stylesheetSequence = new AtomicInteger(); + @Test void realStylesheetAddsNoJavaFxParserErrors() throws Exception { fx(() -> { - int errorsBefore = CssParser.errorsProperty().size(); + ParserRun parserRun = observeParserErrors(this::createFixture); - createFixture(); - - List newErrors = CssParser.errorsProperty() - .subList(errorsBefore, CssParser.errorsProperty().size()); - assertTrue(newErrors.isEmpty(), () -> "JavaFX CSS parser errors: " + newErrors); + assertNoParserErrors(parserRun); + assertEquals(20, parserRun.value().pageTitle().getFont().getSize(), + "A computed style proves that the unique stylesheet URL was parsed"); }); } @Test void semanticClassesComputeApprovedTypographyAndDimensions() throws Exception { fx(() -> { - Fixture fixture = createFixture(); + ParserRun parserRun = observeParserErrors(this::createFixture); + Fixture fixture = parserRun.value(); + + assertNoParserErrors(parserRun); assertEquals(20, fixture.pageTitle().getFont().getSize()); assertEquals(15, fixture.sectionTitle().getFont().getSize()); @@ -73,7 +89,90 @@ class LightThemeCssRuntimeTest { }); } - private static Fixture createFixture() { + @Test + void repeatedUniqueLoadsPreserveAndRestoreParserErrorState() throws Exception { + fx(() -> { + ObservableList globalErrors = CssParser.errorsProperty(); + List errorsBefore = List.copyOf(globalErrors); + CssParser.ParseError preExistingError = new CssParser.ParseError("pre-existing test sentinel"); + globalErrors.add(preExistingError); + + try { + ParserRun firstLoad = observeParserErrors(this::createFixture); + + CssParser.ParseError capturedError = new CssParser.ParseError("captured test sentinel"); + ParserRun isolatedError = observeParserErrors(() -> { + globalErrors.add(capturedError); + return null; + }); + + ParserRun secondLoad = observeParserErrors(this::createFixture); + + assertNoParserErrors(firstLoad); + assertEquals(List.of(capturedError), isolatedError.errors()); + assertNoParserErrors(secondLoad); + assertNotEquals(firstLoad.value().stylesheetUrl(), secondLoad.value().stylesheetUrl(), + "Each run must use a unique URL so JavaFX cannot reuse a cached parse"); + assertEquals(20, firstLoad.value().pageTitle().getFont().getSize()); + assertEquals(20, secondLoad.value().pageTitle().getFont().getSize()); + + List expectedErrors = new ArrayList<>(errorsBefore); + expectedErrors.add(preExistingError); + assertEquals(expectedErrors, List.copyOf(globalErrors), + "The observer must preserve parser errors that predate a run"); + } finally { + removeByIdentity(globalErrors, List.of(preExistingError)); + assertEquals(errorsBefore, List.copyOf(globalErrors), + "The test must restore the parser's global error state"); + } + }); + } + + private ParserRun observeParserErrors(ThrowingSupplier action) throws Exception { + ObservableList globalErrors = CssParser.errorsProperty(); + List errorsBefore = List.copyOf(globalErrors); + List capturedErrors = new ArrayList<>(); + ListChangeListener listener = change -> { + while (change.next()) { + if (change.wasAdded()) { + capturedErrors.addAll(change.getAddedSubList()); + } + } + }; + + globalErrors.addListener(listener); + try { + return new ParserRun<>(action.get(), List.copyOf(capturedErrors)); + } finally { + globalErrors.removeListener(listener); + removeByIdentity(globalErrors, capturedErrors); + assertEquals(errorsBefore, List.copyOf(globalErrors), + "A stylesheet run must restore pre-existing JavaFX parser errors"); + } + } + + private static void removeByIdentity( + ObservableList globalErrors, + List errorsToRemove) { + for (int capturedIndex = errorsToRemove.size() - 1; capturedIndex >= 0; capturedIndex--) { + CssParser.ParseError capturedError = errorsToRemove.get(capturedIndex); + for (int globalIndex = globalErrors.size() - 1; globalIndex >= 0; globalIndex--) { + if (globalErrors.get(globalIndex) == capturedError) { + globalErrors.remove(globalIndex); + break; + } + } + } + } + + private static void assertNoParserErrors(ParserRun parserRun) { + assertTrue(parserRun.errors().isEmpty(), + () -> "JavaFX CSS parser errors: " + parserRun.errors()); + } + + private Fixture createFixture() throws IOException { + String stylesheetUrl = copyRealStylesheetToUniqueUrl(); + Label pageTitle = labelWithStyle(Theme.CSS_PAGE_TITLE); Label sectionTitle = labelWithStyle(Theme.CSS_SECTION_TITLE); Label bodyText = labelWithStyle(Theme.CSS_BODY_TEXT); @@ -116,13 +215,26 @@ class LightThemeCssRuntimeTest { logArea, table, tree, compactList, tabPane, defaultButton, textField, comboBox, header); Scene scene = new Scene(root, 900, 1000); - Theme.applyCustomCss(scene); + scene.getStylesheets().add(stylesheetUrl); root.applyCss(); root.layout(); return new Fixture(pageTitle, sectionTitle, bodyText, secondaryText, logArea, table, tree, compactList, tabPane, - defaultButton, textField, comboBox, header); + defaultButton, textField, comboBox, header, stylesheetUrl); + } + + private String copyRealStylesheetToUniqueUrl() throws IOException { + Path stylesheet = tempDir.resolve( + "light-theme-runtime-" + stylesheetSequence.incrementAndGet() + ".css"); + try (InputStream input = LightThemeCssRuntimeTest.class.getClassLoader() + .getResourceAsStream("css/light-theme.css")) { + if (input == null) { + throw new IOException("Missing classpath resource css/light-theme.css"); + } + Files.copy(input, stylesheet); + } + return stylesheet.toUri().toString(); } private static Label labelWithStyle(String styleClass) { @@ -168,6 +280,15 @@ class LightThemeCssRuntimeTest { Button defaultButton, TextField textField, ComboBox comboBox, - Region header) { + Region header, + String stylesheetUrl) { + } + + private record ParserRun(T value, List errors) { + } + + @FunctionalInterface + private interface ThrowingSupplier { + T get() throws Exception; } } diff --git a/src/test/java/com/example/workutils/system/ui/shell/NavigationRailTest.java b/src/test/java/com/example/workutils/system/ui/shell/NavigationRailTest.java index 5701bb1..2c6fe64 100644 --- a/src/test/java/com/example/workutils/system/ui/shell/NavigationRailTest.java +++ b/src/test/java/com/example/workutils/system/ui/shell/NavigationRailTest.java @@ -25,6 +25,7 @@ import org.kordamp.ikonli.javafx.FontIcon; import java.lang.reflect.Field; import java.util.ArrayList; import java.util.List; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; @@ -385,6 +386,66 @@ class NavigationRailTest { }); } + @Test + void refreshInvalidatesCachedPaneAndConfigurationFailureCanBeRetried() throws Exception { + fx(() -> { + AtomicReference windowRef = new AtomicReference<>(); + AtomicBoolean configValid = new AtomicBoolean(true); + AtomicInteger configChecks = new AtomicInteger(); + AtomicInteger paneBuilds = new AtomicInteger(); + MainWindow.NavItem item = new MainWindow.NavItem( + AppIcon.API_DOC, "API", "API item", () -> { + configChecks.incrementAndGet(); + if (!configValid.get()) { + return; + } + StackPane pane = new StackPane(); + pane.setId("api-pane-" + paneBuilds.incrementAndGet()); + windowRef.get().embedPlugin("api-doc", "legacy", "API", pane); + }, "api-doc"); + MainWindow window = new MainWindow( + List.of(item), new WorkUtilsProperties(), () -> { + }); + windowRef.set(window); + Parent root = stage(window).getScene().getRoot(); + ToggleButton initialButton = assertInstanceOf( + ToggleButton.class, root.lookup("#nav-api-doc")); + + initialButton.fire(); + + assertEquals(1, configChecks.get()); + assertEquals(1, paneBuilds.get()); + assertNotNull(root.lookup("#api-pane-1")); + + configValid.set(false); + window.refreshNavItems(List.of(item)); + + NavigationRail rail = assertInstanceOf( + NavigationRail.class, root.lookup(".navigation-rail")); + ToggleButton refreshedButton = assertInstanceOf( + ToggleButton.class, root.lookup("#nav-api-doc")); + assertNull(rail.selectedPluginId()); + assertNotNull(root.lookup(".welcome-page")); + + refreshedButton.fire(); + + assertEquals(2, configChecks.get()); + assertEquals(1, paneBuilds.get()); + assertNull(root.lookup("#api-pane-1")); + assertNotNull(root.lookup(".welcome-page")); + assertNull(rail.selectedPluginId()); + + configValid.set(true); + refreshedButton.fire(); + + assertEquals(3, configChecks.get()); + assertEquals(2, paneBuilds.get()); + assertEquals("api-doc", rail.selectedPluginId()); + assertNotNull(root.lookup("#api-pane-2")); + assertNull(root.lookup(".welcome-page")); + }); + } + @Test void collapsedRailUsesExactFixedWidthAndCollapsedPseudoClass() throws Exception { fx(() -> {