From 4c9233a1083799eea941119756b0be3c7c290f1b Mon Sep 17 00:00:00 2001 From: Abraham Wolk Date: Tue, 8 Aug 2023 14:53:08 +0200 Subject: [PATCH 1/6] CSSTUDIO-1997 Select most recently focused tab when closing a tab that has the focus. --- .../java/org/phoebus/ui/docking/DockItem.java | 12 ++++++++++++ .../java/org/phoebus/ui/docking/DockPane.java | 15 ++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/core/ui/src/main/java/org/phoebus/ui/docking/DockItem.java b/core/ui/src/main/java/org/phoebus/ui/docking/DockItem.java index cdca334c52..946aa452cb 100644 --- a/core/ui/src/main/java/org/phoebus/ui/docking/DockItem.java +++ b/core/ui/src/main/java/org/phoebus/ui/docking/DockItem.java @@ -176,6 +176,18 @@ public DockItem(final String label) createContextMenu(); setOnClosed(event -> handleClosed()); + setOnCloseRequest(event -> { + // Select the previously selected tab: + var dockPane = getDockPane(); + var recentlyOpenedTabs = dockPane.tabsInOrderOfFocus; + recentlyOpenedTabs.remove(this); + + if (recentlyOpenedTabs.size() > 0) { + var tab = recentlyOpenedTabs.getFirst(); + var selectionModel = dockPane.getSelectionModel(); + selectionModel.select(tab); + } + }); } /** This tab should be in a DockPane, not a plain TabPane diff --git a/core/ui/src/main/java/org/phoebus/ui/docking/DockPane.java b/core/ui/src/main/java/org/phoebus/ui/docking/DockPane.java index bf064e304a..7191d08a1a 100644 --- a/core/ui/src/main/java/org/phoebus/ui/docking/DockPane.java +++ b/core/ui/src/main/java/org/phoebus/ui/docking/DockPane.java @@ -9,7 +9,11 @@ import java.lang.ref.WeakReference; import java.text.MessageFormat; -import java.util.*; +import java.util.ArrayList; +import java.util.Deque; +import java.util.LinkedList; +import java.util.List; +import java.util.Objects; import java.util.concurrent.CopyOnWriteArrayList; import java.util.function.Consumer; import java.util.logging.Level; @@ -219,8 +223,17 @@ public static void alwaysShowTabs(final boolean do_show_single_tabs) getTabs().addListener((InvalidationListener) change -> handleTabChanges()); setOnContextMenuRequested(this::showContextMenu); + + getSelectionModel().selectedItemProperty().addListener((observable, previous_item, new_item) -> { + // Keep track of the order of focus of tabs: + if (new_item != null) { + tabsInOrderOfFocus.remove(new_item); + tabsInOrderOfFocus.push((DockItem) new_item); + } + }); } + protected LinkedList tabsInOrderOfFocus = new LinkedList<>(); private void showContextMenu(final ContextMenuEvent event) { From d3e109aa646b4d7e845b273dfbffdf1bc89dff9c Mon Sep 17 00:00:00 2001 From: Abraham Wolk Date: Tue, 8 Aug 2023 15:33:29 +0200 Subject: [PATCH 2/6] CSSTUDIO-1997 Add configuration option. --- .../main/java/org/phoebus/ui/Preferences.java | 1 + .../java/org/phoebus/ui/docking/DockItem.java | 27 ++++++++++--------- .../java/org/phoebus/ui/docking/DockPane.java | 17 +++++++----- .../phoebus_ui_preferences.properties | 8 ++++++ 4 files changed, 34 insertions(+), 19 deletions(-) diff --git a/core/ui/src/main/java/org/phoebus/ui/Preferences.java b/core/ui/src/main/java/org/phoebus/ui/Preferences.java index 98ba4605ad..ec089653bf 100644 --- a/core/ui/src/main/java/org/phoebus/ui/Preferences.java +++ b/core/ui/src/main/java/org/phoebus/ui/Preferences.java @@ -53,6 +53,7 @@ public class Preferences @Preference public static int[] alarm_area_panel_invalid_severity_background_color; @Preference public static int[] alarm_area_panel_undefined_severity_background_color; @Preference public static String cache_hint_for_picture_and_symbol_widgets; + @Preference public static boolean open_previous_tab_when_closing_tab; static { diff --git a/core/ui/src/main/java/org/phoebus/ui/docking/DockItem.java b/core/ui/src/main/java/org/phoebus/ui/docking/DockItem.java index 946aa452cb..5a00cedbcd 100644 --- a/core/ui/src/main/java/org/phoebus/ui/docking/DockItem.java +++ b/core/ui/src/main/java/org/phoebus/ui/docking/DockItem.java @@ -28,6 +28,7 @@ import org.phoebus.framework.spi.AppDescriptor; import org.phoebus.framework.spi.AppInstance; import org.phoebus.security.authorization.AuthorizationService; +import org.phoebus.ui.Preferences; import org.phoebus.ui.application.Messages; import org.phoebus.ui.application.SaveLayoutHelper; import org.phoebus.ui.dialog.DialogHelper; @@ -176,18 +177,20 @@ public DockItem(final String label) createContextMenu(); setOnClosed(event -> handleClosed()); - setOnCloseRequest(event -> { - // Select the previously selected tab: - var dockPane = getDockPane(); - var recentlyOpenedTabs = dockPane.tabsInOrderOfFocus; - recentlyOpenedTabs.remove(this); - - if (recentlyOpenedTabs.size() > 0) { - var tab = recentlyOpenedTabs.getFirst(); - var selectionModel = dockPane.getSelectionModel(); - selectionModel.select(tab); - } - }); + if (Preferences.open_previous_tab_when_closing_tab) { + setOnCloseRequest(event -> { + // Select the previously selected tab: + var dockPane = getDockPane(); + var recentlyOpenedTabs = dockPane.tabsInOrderOfFocus; + recentlyOpenedTabs.remove(this); + + if (recentlyOpenedTabs.size() > 0) { + var tab = recentlyOpenedTabs.getFirst(); + var selectionModel = dockPane.getSelectionModel(); + selectionModel.select(tab); + } + }); + } } /** This tab should be in a DockPane, not a plain TabPane diff --git a/core/ui/src/main/java/org/phoebus/ui/docking/DockPane.java b/core/ui/src/main/java/org/phoebus/ui/docking/DockPane.java index 7191d08a1a..d32a8fb8a7 100644 --- a/core/ui/src/main/java/org/phoebus/ui/docking/DockPane.java +++ b/core/ui/src/main/java/org/phoebus/ui/docking/DockPane.java @@ -23,6 +23,7 @@ import javafx.beans.value.ChangeListener; import javafx.beans.value.ObservableValue; import org.phoebus.framework.jobs.JobManager; +import org.phoebus.ui.Preferences; import org.phoebus.ui.application.Messages; import org.phoebus.ui.dialog.DialogHelper; import org.phoebus.ui.javafx.ImageCache; @@ -224,13 +225,15 @@ public static void alwaysShowTabs(final boolean do_show_single_tabs) setOnContextMenuRequested(this::showContextMenu); - getSelectionModel().selectedItemProperty().addListener((observable, previous_item, new_item) -> { - // Keep track of the order of focus of tabs: - if (new_item != null) { - tabsInOrderOfFocus.remove(new_item); - tabsInOrderOfFocus.push((DockItem) new_item); - } - }); + if (Preferences.open_previous_tab_when_closing_tab) { + getSelectionModel().selectedItemProperty().addListener((observable, previous_item, new_item) -> { + // Keep track of the order of focus of tabs: + if (new_item != null) { + tabsInOrderOfFocus.remove(new_item); + tabsInOrderOfFocus.push((DockItem) new_item); + } + }); + } } protected LinkedList tabsInOrderOfFocus = new LinkedList<>(); diff --git a/core/ui/src/main/resources/phoebus_ui_preferences.properties b/core/ui/src/main/resources/phoebus_ui_preferences.properties index cb5180f426..3222bb92fe 100644 --- a/core/ui/src/main/resources/phoebus_ui_preferences.properties +++ b/core/ui/src/main/resources/phoebus_ui_preferences.properties @@ -128,3 +128,11 @@ alarm_area_panel_undefined_severity_background_color=200,0,200,200 # default caching behavior is used (i.e., caching is DISABLED, # and the cache hint is set to "CacheHint.DEFAULT"). cache_hint_for_picture_and_symbol_widgets= + +# When 'open_previous_tab_when_closing_tab' is set to 'false', the tab to the left +# of the tab that is closed receives the focus when a tab is closed (e.g., by +# clicking the 'X'-icon on the tab). When set to 'true', the most recently focused +# tab receives the focus (if a tab is closed that doesn't have the focus, the +# focus remains unchanged since the most recently focused tab is the one that has +# the focus). +open_previous_tab_when_closing_tab=false From 16f20ce2e63b970f3016660b366038c9840e1daf Mon Sep 17 00:00:00 2001 From: Abraham Wolk Date: Tue, 8 Aug 2023 15:56:24 +0200 Subject: [PATCH 3/6] Revert "CSSTUDIO-1997 Add configuration option." This reverts commit d3e109aa646b4d7e845b273dfbffdf1bc89dff9c. --- .../main/java/org/phoebus/ui/Preferences.java | 1 - .../java/org/phoebus/ui/docking/DockItem.java | 27 +++++++++---------- .../java/org/phoebus/ui/docking/DockPane.java | 17 +++++------- .../phoebus_ui_preferences.properties | 8 ------ 4 files changed, 19 insertions(+), 34 deletions(-) diff --git a/core/ui/src/main/java/org/phoebus/ui/Preferences.java b/core/ui/src/main/java/org/phoebus/ui/Preferences.java index ec089653bf..98ba4605ad 100644 --- a/core/ui/src/main/java/org/phoebus/ui/Preferences.java +++ b/core/ui/src/main/java/org/phoebus/ui/Preferences.java @@ -53,7 +53,6 @@ public class Preferences @Preference public static int[] alarm_area_panel_invalid_severity_background_color; @Preference public static int[] alarm_area_panel_undefined_severity_background_color; @Preference public static String cache_hint_for_picture_and_symbol_widgets; - @Preference public static boolean open_previous_tab_when_closing_tab; static { diff --git a/core/ui/src/main/java/org/phoebus/ui/docking/DockItem.java b/core/ui/src/main/java/org/phoebus/ui/docking/DockItem.java index 5a00cedbcd..946aa452cb 100644 --- a/core/ui/src/main/java/org/phoebus/ui/docking/DockItem.java +++ b/core/ui/src/main/java/org/phoebus/ui/docking/DockItem.java @@ -28,7 +28,6 @@ import org.phoebus.framework.spi.AppDescriptor; import org.phoebus.framework.spi.AppInstance; import org.phoebus.security.authorization.AuthorizationService; -import org.phoebus.ui.Preferences; import org.phoebus.ui.application.Messages; import org.phoebus.ui.application.SaveLayoutHelper; import org.phoebus.ui.dialog.DialogHelper; @@ -177,20 +176,18 @@ public DockItem(final String label) createContextMenu(); setOnClosed(event -> handleClosed()); - if (Preferences.open_previous_tab_when_closing_tab) { - setOnCloseRequest(event -> { - // Select the previously selected tab: - var dockPane = getDockPane(); - var recentlyOpenedTabs = dockPane.tabsInOrderOfFocus; - recentlyOpenedTabs.remove(this); - - if (recentlyOpenedTabs.size() > 0) { - var tab = recentlyOpenedTabs.getFirst(); - var selectionModel = dockPane.getSelectionModel(); - selectionModel.select(tab); - } - }); - } + setOnCloseRequest(event -> { + // Select the previously selected tab: + var dockPane = getDockPane(); + var recentlyOpenedTabs = dockPane.tabsInOrderOfFocus; + recentlyOpenedTabs.remove(this); + + if (recentlyOpenedTabs.size() > 0) { + var tab = recentlyOpenedTabs.getFirst(); + var selectionModel = dockPane.getSelectionModel(); + selectionModel.select(tab); + } + }); } /** This tab should be in a DockPane, not a plain TabPane diff --git a/core/ui/src/main/java/org/phoebus/ui/docking/DockPane.java b/core/ui/src/main/java/org/phoebus/ui/docking/DockPane.java index d32a8fb8a7..7191d08a1a 100644 --- a/core/ui/src/main/java/org/phoebus/ui/docking/DockPane.java +++ b/core/ui/src/main/java/org/phoebus/ui/docking/DockPane.java @@ -23,7 +23,6 @@ import javafx.beans.value.ChangeListener; import javafx.beans.value.ObservableValue; import org.phoebus.framework.jobs.JobManager; -import org.phoebus.ui.Preferences; import org.phoebus.ui.application.Messages; import org.phoebus.ui.dialog.DialogHelper; import org.phoebus.ui.javafx.ImageCache; @@ -225,15 +224,13 @@ public static void alwaysShowTabs(final boolean do_show_single_tabs) setOnContextMenuRequested(this::showContextMenu); - if (Preferences.open_previous_tab_when_closing_tab) { - getSelectionModel().selectedItemProperty().addListener((observable, previous_item, new_item) -> { - // Keep track of the order of focus of tabs: - if (new_item != null) { - tabsInOrderOfFocus.remove(new_item); - tabsInOrderOfFocus.push((DockItem) new_item); - } - }); - } + getSelectionModel().selectedItemProperty().addListener((observable, previous_item, new_item) -> { + // Keep track of the order of focus of tabs: + if (new_item != null) { + tabsInOrderOfFocus.remove(new_item); + tabsInOrderOfFocus.push((DockItem) new_item); + } + }); } protected LinkedList tabsInOrderOfFocus = new LinkedList<>(); diff --git a/core/ui/src/main/resources/phoebus_ui_preferences.properties b/core/ui/src/main/resources/phoebus_ui_preferences.properties index 3222bb92fe..cb5180f426 100644 --- a/core/ui/src/main/resources/phoebus_ui_preferences.properties +++ b/core/ui/src/main/resources/phoebus_ui_preferences.properties @@ -128,11 +128,3 @@ alarm_area_panel_undefined_severity_background_color=200,0,200,200 # default caching behavior is used (i.e., caching is DISABLED, # and the cache hint is set to "CacheHint.DEFAULT"). cache_hint_for_picture_and_symbol_widgets= - -# When 'open_previous_tab_when_closing_tab' is set to 'false', the tab to the left -# of the tab that is closed receives the focus when a tab is closed (e.g., by -# clicking the 'X'-icon on the tab). When set to 'true', the most recently focused -# tab receives the focus (if a tab is closed that doesn't have the focus, the -# focus remains unchanged since the most recently focused tab is the one that has -# the focus). -open_previous_tab_when_closing_tab=false From 497bb1d47864a1f018e63a0a339095f93fba6af2 Mon Sep 17 00:00:00 2001 From: Abraham Wolk Date: Tue, 8 Aug 2023 15:59:30 +0200 Subject: [PATCH 4/6] CSSTUDIO-1997 Remove temporary variable and improve naming. --- .../src/main/java/org/phoebus/ui/docking/DockItem.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/core/ui/src/main/java/org/phoebus/ui/docking/DockItem.java b/core/ui/src/main/java/org/phoebus/ui/docking/DockItem.java index 946aa452cb..e35602b1f0 100644 --- a/core/ui/src/main/java/org/phoebus/ui/docking/DockItem.java +++ b/core/ui/src/main/java/org/phoebus/ui/docking/DockItem.java @@ -179,13 +179,12 @@ public DockItem(final String label) setOnCloseRequest(event -> { // Select the previously selected tab: var dockPane = getDockPane(); - var recentlyOpenedTabs = dockPane.tabsInOrderOfFocus; - recentlyOpenedTabs.remove(this); + dockPane.tabsInOrderOfFocus.remove(this); - if (recentlyOpenedTabs.size() > 0) { - var tab = recentlyOpenedTabs.getFirst(); + if (dockPane.tabsInOrderOfFocus.size() > 0) { + var tabToSelect = dockPane.tabsInOrderOfFocus.getFirst(); var selectionModel = dockPane.getSelectionModel(); - selectionModel.select(tab); + selectionModel.select(tabToSelect); } }); } From 9555f3b67e7f6a3046572bee7942f936b1927d0b Mon Sep 17 00:00:00 2001 From: Abraham Wolk Date: Wed, 9 Aug 2023 09:08:07 +0200 Subject: [PATCH 5/6] CSSTUDIO-1997 Add removal of DockItem from `dockPane.tabsInOrderOfFocus` in OnClose event handler of `DockItem`. --- core/ui/src/main/java/org/phoebus/ui/docking/DockItem.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/ui/src/main/java/org/phoebus/ui/docking/DockItem.java b/core/ui/src/main/java/org/phoebus/ui/docking/DockItem.java index e35602b1f0..de48ec238f 100644 --- a/core/ui/src/main/java/org/phoebus/ui/docking/DockItem.java +++ b/core/ui/src/main/java/org/phoebus/ui/docking/DockItem.java @@ -702,6 +702,10 @@ protected void handleClosed() setContent(null); // Remove "application" entry which otherwise holds on to application data model getProperties().remove(KEY_APPLICATION); + var dockPane = getDockPane(); + if (dockPane != null) { + dockPane.tabsInOrderOfFocus.remove(this); + } } /** Programmatically close this tab From 657aab9bf34160d701a6c592093fd80c5a65ebc1 Mon Sep 17 00:00:00 2001 From: Abraham Wolk Date: Wed, 9 Aug 2023 09:18:02 +0200 Subject: [PATCH 6/6] CSSTUDIO-1997 Combine OnCloseRequest event-handlers. --- .../java/org/phoebus/ui/docking/DockItem.java | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/core/ui/src/main/java/org/phoebus/ui/docking/DockItem.java b/core/ui/src/main/java/org/phoebus/ui/docking/DockItem.java index de48ec238f..0945cc79d2 100644 --- a/core/ui/src/main/java/org/phoebus/ui/docking/DockItem.java +++ b/core/ui/src/main/java/org/phoebus/ui/docking/DockItem.java @@ -604,20 +604,24 @@ public void select() */ public void addCloseCheck(final Supplier> ok_to_close) { - if (getOnCloseRequest() == null) - setOnCloseRequest(event -> - { - // For now, prevent closing - event.consume(); + var alreadyExistingEventHandler = getOnCloseRequest(); - // Invoke all the ok-to-close checks in background threads - // since those that save files might take time. - JobManager.schedule("Close " + getLabel(), monitor -> - { - if (prepareToClose()) - Platform.runLater(() -> close()); - }); + setOnCloseRequest(event -> { + // For now, prevent closing + event.consume(); + + // Invoke all the ok-to-close checks in background threads + // since those that save files might take time. + JobManager.schedule("Close " + getLabel(), monitor -> + { + if (prepareToClose()) { + if (alreadyExistingEventHandler != null) { + alreadyExistingEventHandler.handle(event); + } + Platform.runLater(() -> close()); + } }); + }); close_check.add(ok_to_close); }