diff --git a/CHANGELOG.md b/CHANGELOG.md index 998bd682..e80f6088 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,5 @@ +- Fixed diff panel appearing at wrong height (nearly invisible) after Reset Windows or on first use +- Fixed second diff in the same Claude session being invisible (stale card accumulation) - Fixed terminal scrolling to the beginning when a choice menu or diff panel appears (regression since 0.20.20) # 0.21.46 (2026-04-17) diff --git a/src/main/java/io/github/nbclaudecodegui/ui/ClaudeSessionTab.java b/src/main/java/io/github/nbclaudecodegui/ui/ClaudeSessionTab.java index 15060dbf..02b3b0a8 100644 --- a/src/main/java/io/github/nbclaudecodegui/ui/ClaudeSessionTab.java +++ b/src/main/java/io/github/nbclaudecodegui/ui/ClaudeSessionTab.java @@ -162,9 +162,10 @@ public class ClaudeSessionTab extends TopComponent private JediTermWidget terminalWidget; private JSplitPane splitPane; private JPanel errorPanel; - private JPanel southCard; - private CardLayout southCardLayout; - private String activeCard; + JPanel southCard; + CardLayout southCardLayout; + String activeCard; + JPanel currentDiffCard; private Runnable pendingDiffOnClose; // ------------------------------------------------------------------------- @@ -741,19 +742,10 @@ private void showChatLayout(JediTermWidget widget) { public void componentResized(ComponentEvent e) { int total = splitPane.getHeight(); if (total <= 0) return; - int bottom; - if (!savingEnabled[0]) { - bottom = savedBottomHeight > 0 ? savedBottomHeight - : southCard.getPreferredSize().height; - savingEnabled[0] = true; - } else { - bottom = NbPreferences.forModule(ClaudeSessionTab.class) - .getInt(getSavedCardKey(), southCard.getPreferredSize().height); - } - int divLoc = total - splitPane.getDividerSize() - bottom; - LOG.fine(sessionTag + "[splitPane resize] total=" + total - + " bottom=" + bottom + " divLoc=" + divLoc); - splitPane.setDividerLocation(divLoc); + LOG.fine(sessionTag + "[componentResized] activeCard=" + activeCard + + " total=" + total + " savedBottomHeight=" + savedBottomHeight); + applyDividerForCard(activeCard, total, savedBottomHeight); + savingEnabled[0] = true; } }); splitPane.addPropertyChangeListener(JSplitPane.DIVIDER_LOCATION_PROPERTY, e -> { @@ -893,11 +885,53 @@ private void showStartError(String command, String error, * @return preference key string */ private String getSavedCardKey() { - if (CARD_PROMPT.equals(activeCard) || activeCard == null) return "bottomHeight"; - return activeCard + "Height"; + return getSavedCardKeyFor(activeCard); + } + + /** + * Reads the saved divider height for {@code card}, falls back to sensible defaults, + * sets the divider location, and returns the bottom height used. + * {@code legacyPromptHeight} is the pre-migration "bottomHeight" value (pass -1 to skip). + */ + private int applyDividerForCard(String card, int total, int legacyPromptHeight) { + int saved = NbPreferences.forModule(ClaudeSessionTab.class) + .getInt(getSavedCardKeyFor(card), -1); + int bottom = computeBottomHeight(card, total, saved, legacyPromptHeight, + southCard.getPreferredSize().height); + int divLoc = total - splitPane.getDividerSize() - bottom; + LOG.fine(sessionTag + "[applyDivider] card=" + card + + " total=" + total + " saved=" + saved + + " legacy=" + legacyPromptHeight + + " preferred=" + southCard.getPreferredSize().height + + " bottom=" + bottom + " divLoc=" + divLoc); + if (divLoc >= 0) splitPane.setDividerLocation(divLoc); + return bottom; + } + + /** + * Pure function: computes the south-panel height for a given card. + * Extracted for unit testing — no Swing state access. + * + * @param card active card name (CARD_PROMPT or CARD_DIFF) + * @param total total splitPane height + * @param saved persisted height (negative means not saved) + * @param legacyPromptHeight "bottomHeight" pref value for backward-compat (negative to skip) + * @param preferredHeight southCard preferred height (fallback) + */ + static int computeBottomHeight(String card, int total, int saved, + int legacyPromptHeight, int preferredHeight) { + if (saved > 0) return saved; + if (CARD_DIFF.equals(card)) return total * 2 / 3; + if (CARD_PROMPT.equals(card) && legacyPromptHeight > 0) return legacyPromptHeight; + return preferredHeight; } - private void switchSouthCard(String card) { + String getSavedCardKeyFor(String card) { + if (CARD_PROMPT.equals(card) || card == null) return "bottomHeight"; + return card + "Height"; + } + + void switchSouthCard(String card) { if (southCard == null || southCardLayout == null) return; activeCard = card; southCardLayout.show(southCard, card); @@ -912,11 +946,14 @@ private void switchSouthCard(String card) { if (!CARD_CHOICE.equals(card)) { // restore divider for this card int total = splitPane.getHeight(); + LOG.fine(sessionTag + "[switchSouthCard] card=" + card + " total=" + total); if (total > 0) { - int bottom = NbPreferences.forModule(ClaudeSessionTab.class) - .getInt(getSavedCardKey(), DEFAULT_DIFF_HEIGHT); - int divLoc = total - splitPane.getDividerSize() - bottom; - splitPane.setDividerLocation(divLoc); + applyDividerForCard(card, total, -1); + } else { + SwingUtilities.invokeLater(() -> { + int t = splitPane != null ? splitPane.getHeight() : 0; + if (t > 0) applyDividerForCard(activeCard, t, -1); + }); } } else { // lock divider for choice menu @@ -1058,6 +1095,7 @@ private void cleanupChatLayout() { southCard = null; southCardLayout = null; activeCard = null; + currentDiffCard = null; } if (errorPanel != null) { remove(errorPanel); @@ -1124,6 +1162,10 @@ public boolean showEmbeddedDiff(boolean outsideProject, String outsideWarning, permPanel.showWarning(outsideWarning); } + if (currentDiffCard != null) { + southCard.remove(currentDiffCard); + currentDiffCard = null; + } JPanel diffCard = new JPanel(new BorderLayout()); if (toolbar != null) { diffCard.add(toolbar, BorderLayout.NORTH); @@ -1131,7 +1173,8 @@ public boolean showEmbeddedDiff(boolean outsideProject, String outsideWarning, diffCard.add(diffComponent, BorderLayout.CENTER); diffCard.add(permPanel, BorderLayout.SOUTH); - southCard.add(diffCard, CARD_DIFF); + currentDiffCard = diffCard; + southCard.add(currentDiffCard, CARD_DIFF); pendingDiffOnClose = onTabClose; switchSouthCard(CARD_DIFF); @@ -1148,6 +1191,10 @@ public void hideEmbeddedDiff() { pendingDiffOnClose = null; if (southCard == null) return; switchSouthCard(CARD_PROMPT); + if (currentDiffCard != null) { + southCard.remove(currentDiffCard); + currentDiffCard = null; + } revalidate(); repaint(); } diff --git a/src/test/java/io/github/nbclaudecodegui/ui/ClaudeSessionTabDiffTest.java b/src/test/java/io/github/nbclaudecodegui/ui/ClaudeSessionTabDiffTest.java new file mode 100644 index 00000000..4dd2807d --- /dev/null +++ b/src/test/java/io/github/nbclaudecodegui/ui/ClaudeSessionTabDiffTest.java @@ -0,0 +1,175 @@ +package io.github.nbclaudecodegui.ui; + +import java.awt.CardLayout; +import java.awt.Component; +import javax.swing.JLabel; +import javax.swing.JPanel; +import javax.swing.JSplitPane; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * Tests for the two embedded-diff bugs fixed in 0.22.2: + *
    + *
  1. Stale {@code CARD_DIFF} accumulates in the CardLayout when + * {@link ClaudeSessionTab#showEmbeddedDiff} is called multiple times.
    2. + *
  2. Wrong divider position when the diff card first appears ({@link + * ClaudeSessionTab#computeBottomHeight} default logic).
    3. + *
+ */ +class ClaudeSessionTabDiffTest { + + // ------------------------------------------------------------------------- + // Bug 1 — computeBottomHeight default values + // ------------------------------------------------------------------------- + + @Test + void computeBottomHeight_savedValue_returnsSaved() { + assertEquals(123, ClaudeSessionTab.computeBottomHeight("diff", 900, 123, -1, 50)); + } + + @Test + void computeBottomHeight_diffCard_noSaved_returnsTwoThirdsTotal() { + int total = 900; + int expected = total * 2 / 3; // 600 + assertEquals(expected, ClaudeSessionTab.computeBottomHeight("diff", total, -1, -1, 50)); + } + + @Test + void computeBottomHeight_promptCard_noSaved_legacyPresent_returnsLegacy() { + assertEquals(200, ClaudeSessionTab.computeBottomHeight("prompt", 900, -1, 200, 50)); + } + + @Test + void computeBottomHeight_promptCard_noSaved_noLegacy_returnsPreferred() { + assertEquals(50, ClaudeSessionTab.computeBottomHeight("prompt", 900, -1, -1, 50)); + } + + @Test + void computeBottomHeight_diffCard_savedZeroIgnored_returnsTwoThirds() { + // saved == 0 is treated as "not saved" (condition is saved > 0) + int total = 600; + assertEquals(total * 2 / 3, ClaudeSessionTab.computeBottomHeight("diff", total, 0, -1, 50)); + } + + // ------------------------------------------------------------------------- + // getSavedCardKeyFor + // ------------------------------------------------------------------------- + + @Test + void savedCardKey_promptCard_returnsBackwardCompatKey() { + ClaudeSessionTab tab = new ClaudeSessionTab(); + assertEquals("bottomHeight", tab.getSavedCardKeyFor("prompt")); + } + + @Test + void savedCardKey_nullCard_returnsBackwardCompatKey() { + ClaudeSessionTab tab = new ClaudeSessionTab(); + assertEquals("bottomHeight", tab.getSavedCardKeyFor(null)); + } + + @Test + void savedCardKey_diffCard_returnsDiffHeight() { + ClaudeSessionTab tab = new ClaudeSessionTab(); + assertEquals("diffHeight", tab.getSavedCardKeyFor("diff")); + } + + // ------------------------------------------------------------------------- + // Bug 2 — stale CARD_DIFF accumulates in CardLayout + // ------------------------------------------------------------------------- + + /** + * Sets up a minimal south-card environment on a tab (without starting a PTY) + * so that {@link ClaudeSessionTab#showEmbeddedDiff} can be called. + * {@code switchSouthCard} is overridden to a no-op to avoid needing a full + * JediTerm widget and splitPane. + */ + private ClaudeSessionTab tabWithSouthCard() { + ClaudeSessionTab tab = new ClaudeSessionTab() { + @Override + void switchSouthCard(String card) { + // no-op: avoids splitPane / terminalWidget dependency in tests + activeCard = card; + } + }; + CardLayout layout = new CardLayout(); + JPanel south = new JPanel(layout); + tab.southCard = south; + tab.southCardLayout = layout; + tab.activeCard = "prompt"; + return tab; + } + + @Test + void showEmbeddedDiff_firstCall_addsDiffCard() { + ClaudeSessionTab tab = tabWithSouthCard(); + + boolean shown = tab.showEmbeddedDiff(false, null, null, + new JLabel("diff1"), + new FileDiffPermissionPanel(() -> {}, r -> {}, () -> {}, () -> {}, null), + null); + + assertTrue(shown, "showEmbeddedDiff must return true when southCard is set"); + assertNotNull(tab.currentDiffCard, "currentDiffCard must be set after first call"); + // southCard now contains exactly one CARD_DIFF panel + assertEquals(1, countComponents(tab.southCard)); + } + + @Test + void showEmbeddedDiff_secondCall_replacesOldCard() { + ClaudeSessionTab tab = tabWithSouthCard(); + + tab.showEmbeddedDiff(false, null, null, + new JLabel("diff1"), + new FileDiffPermissionPanel(() -> {}, r -> {}, () -> {}, () -> {}, null), + null); + JPanel firstCard = tab.currentDiffCard; + + tab.showEmbeddedDiff(false, null, null, + new JLabel("diff2"), + new FileDiffPermissionPanel(() -> {}, r -> {}, () -> {}, () -> {}, null), + null); + JPanel secondCard = tab.currentDiffCard; + + assertNotSame(firstCard, secondCard, "currentDiffCard must be replaced on second call"); + // Only the new card should remain in southCard — no stale accumulation + assertEquals(1, countComponents(tab.southCard), + "southCard must contain exactly one diff card after second showEmbeddedDiff"); + assertFalse(isChildOf(tab.southCard, firstCard), + "Old diff card must be removed from southCard"); + assertTrue(isChildOf(tab.southCard, secondCard), + "New diff card must be present in southCard"); + } + + @Test + void hideEmbeddedDiff_removesCardAndClearsField() { + ClaudeSessionTab tab = tabWithSouthCard(); + + tab.showEmbeddedDiff(false, null, null, + new JLabel("diff"), + new FileDiffPermissionPanel(() -> {}, r -> {}, () -> {}, () -> {}, null), + null); + + tab.hideEmbeddedDiff(); + + assertNull(tab.currentDiffCard, "currentDiffCard must be null after hide"); + assertEquals(0, countComponents(tab.southCard), + "southCard must have no diff card after hide"); + } + + // ------------------------------------------------------------------------- + // Helpers + // ------------------------------------------------------------------------- + + private static int countComponents(JPanel panel) { + return panel.getComponentCount(); + } + + private static boolean isChildOf(JPanel parent, Component child) { + for (int i = 0; i < parent.getComponentCount(); i++) { + if (parent.getComponent(i) == child) return true; + } + return false; + } +}