Hide tabs without restart #12958

koppor · 2025-04-17T23:15:39Z

Follow-up to #12919

Hides tabs without reload.

Mandatory checks

I own the copyright of the code submitted and I license it under the MIT license
[/] Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)
[/] Tests created for changes (if applicable)
Manually tested changed features in running JabRef (always required)
[/] Screenshots added in PR description (if change is visible to the user)
[/] Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
[/] Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

trag-bot · 2025-04-17T23:16:47Z

src/main/java/org/jabref/gui/ai/components/aichat/AiChatGuardedComponent.java

                                  TaskExecutor taskExecutor
    ) {
-        super(aiService, aiPreferences, externalApplicationsPreferences, dialogService);
+        super(aiService, aiPreferences, externalApplicationsPreferences, dialogService, adaptVisibleTabs);


The constructor call in the superclass has been modified to include 'adaptVisibleTabs', but the JavaDoc for the constructor has not been updated to reflect this change.

trag-bot · 2025-04-17T23:17:26Z

src/main/java/org/jabref/gui/ai/components/privacynotice/AiPrivacyNoticeGuardedComponent.java


-    public AiPrivacyNoticeGuardedComponent(AiPreferences aiPreferences, ExternalApplicationsPreferences externalApplicationsPreferences, DialogService dialogService) {
+    public AiPrivacyNoticeGuardedComponent(AiPreferences aiPreferences, ExternalApplicationsPreferences externalApplicationsPreferences, DialogService dialogService, AdaptVisibleTabs adaptVisibleTabs) {


The JavaDoc for the constructor should be updated to include the new parameter 'AdaptVisibleTabs adaptVisibleTabs' as per the special instructions.

trag-bot · 2025-04-17T23:18:08Z

src/main/java/org/jabref/gui/ai/components/summary/SummaryComponent.java

    ) {
-        super(aiPreferences, externalApplicationsPreferences, dialogService);
+        super(aiPreferences, externalApplicationsPreferences, dialogService, adaptVisibleTabs);


The constructor call now includes an additional parameter 'adaptVisibleTabs', but there is no JavaDoc update reflecting this change. JavaDoc should be updated to include this new parameter.

trag-bot · 2025-04-17T23:18:32Z

src/main/java/org/jabref/gui/ai/components/util/EmbeddingModelGuardedComponent.java

    ) {
-        super(aiPreferences, externalApplicationsPreferences, dialogService);
+        super(aiPreferences, externalApplicationsPreferences, dialogService, adaptVisibleTabs);


The constructor call to the superclass now includes a new parameter 'adaptVisibleTabs', but the JavaDoc for the constructor has not been updated to reflect this change.

trag-bot · 2025-04-17T23:19:43Z

src/main/java/org/jabref/gui/entryeditor/AiSummaryTab.java

@@ -18,6 +18,7 @@ public class AiSummaryTab extends EntryEditorTab {
    private final AiService aiService;
    private final DialogService dialogService;
    private final StateManager stateManager;
+    private final AdaptVisibleTabs adaptVisibleTabs;


The field 'adaptVisibleTabs' is introduced but not documented in the JavaDoc of the constructor. This violates the requirement to update JavaDoc when method arguments change.

trag-bot · 2025-04-17T23:19:58Z

src/main/java/org/jabref/gui/entryeditor/EntryEditor.java

@@ -87,7 +87,7 @@
 * <p>
 * The editors for fields are created via {@link org.jabref.gui.fieldeditors.FieldEditors}.
 */
-public class EntryEditor extends BorderPane implements PreviewControls {
+public class EntryEditor extends BorderPane implements PreviewControls, AdaptVisibleTabs {


The class now implements AdaptVisibleTabs, but the JavaDoc for the class does not reflect this change. The JavaDoc should be updated to include information about the new interface implementation.

trag-bot · 2025-04-17T23:20:23Z

src/main/java/org/jabref/gui/frame/JabRefFrame.java

+                // Actions are recreated here since this avoids passing more parameters and the amount of additional memory consumption is neglegtable.
+                new UndoAction(this::getCurrentLibraryTab, undoManager, dialogService, stateManager),
+                new RedoAction(this::getCurrentLibraryTab, undoManager, dialogService, stateManager));
+        Injector.setModelOrService(EntryEditor.class, entryEditor);


The Injector.setModelOrService call is duplicated in the patch. This could lead to unnecessary redundancy and potential maintenance issues.

trag-bot · 2025-04-17T23:21:13Z

src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java

                              TaskExecutor taskExecutor,
                              CustomLocalDragboard localDragboard
    ) {
        this.stateManager = Objects.requireNonNull(stateManager);
        this.dialogService = Objects.requireNonNull(dialogService);
        this.aiService = Objects.requireNonNull(aiService);
        this.preferences = Objects.requireNonNull(preferences);
+        this.adaptVisibleTabs = adaptVisibleTabs;


The use of Objects.requireNonNull should be replaced with @nonnull annotation to ensure the adaptVisibleTabs parameter is not null, aligning with the project's guidelines.

trag-bot · 2025-04-17T23:21:59Z

src/main/java/org/jabref/gui/sidepane/SidePaneContentFactory.java

@@ -50,6 +53,7 @@ public SidePaneContentFactory(LibraryTabContainer tabContainer,
        this.dialogService = dialogService;
        this.aiService = aiService;
        this.stateManager = stateManager;
+        this.adaptVisibleTabs = adaptVisibleTabs;


The assignment of adaptVisibleTabs in the constructor lacks a null check, which violates the fail-fast principle by not immediately handling invalid states.

github-actions · 2025-04-17T23:26:25Z

The build of this PR is available at https://builds.jabref.org/pull/12958/merge.

calixtus · 2025-04-18T18:13:47Z

I believe to correct future approach would be to observe an observable list of visible tabs, A Pane in the ai package would implement an interface of the EntryEditor interface and that could be added to or removed from the list.

koppor · 2025-04-18T18:30:10Z

I believe to correct future approach would be to observe an observable list of visible tabs, A Pane in the ai package would implement an interface of the EntryEditor interface and that could be added to or removed from the list.

Follow-up?

Still not clear how to access this list from "somewhere".

I was about to add an event bus for the general (cause sender and receiver are decoupled), but did not dare 😅

calixtus · 2025-04-18T18:44:17Z

Yes follow up eventually. When we rewrite the entry editor. List would sit in the StateManager.

Siedlerchr · 2025-04-21T11:39:54Z

Yeah, looks a bit weird now. I would have expected an observabelist of tabs in the state manager

koppor · 2025-04-21T13:28:37Z

Yeah, looks a bit weird now. I would have expected an observabelist of tabs in the state manager

That would have been much more effort for me 😅.

I did not use any dependency injection; instead explicitly passing the dependency down.

Proposal: Either someone improves the code significantly the next two weeks or we merge and do follow-up PRs.

Currently, restarting only because of hiding is bad UX IMHO.

calixtus · 2025-04-21T14:14:04Z

Im ok with follow-up. We need to push forward here. Will also increase pressure on us to finally work on the issues with the entry editor.

calixtus · 2025-04-21T14:14:32Z

Maybe put a fixme or todo in the code as a bad conscience reminder

calixtus · 2025-04-21T19:51:49Z

This strategy might also solve #12865

koppor added 2 commits April 18, 2025 00:54

Fix link

6543af0

Introduce interface AdaptVisibleTabs to avoid reload

d5b3ecd

koppor added the component: entry-editor label Apr 17, 2025

koppor mentioned this pull request Apr 17, 2025

Reorder settings in the Entry editor tab in preferences and buttons to hide AI tabs #12919

Merged

7 tasks

trag-bot bot reviewed Apr 17, 2025

View reviewed changes

koppor added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Apr 18, 2025

koppor requested review from Siedlerchr and calixtus April 18, 2025 18:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Hide tabs without restart #12958

Hide tabs without restart #12958

koppor commented Apr 17, 2025

trag-bot bot Apr 17, 2025

trag-bot bot Apr 17, 2025

trag-bot bot Apr 17, 2025

trag-bot bot Apr 17, 2025

trag-bot bot Apr 17, 2025

trag-bot bot Apr 17, 2025

trag-bot bot Apr 17, 2025

trag-bot bot Apr 17, 2025

trag-bot bot Apr 17, 2025

github-actions bot commented Apr 17, 2025

calixtus commented Apr 18, 2025

koppor commented Apr 18, 2025

calixtus commented Apr 18, 2025

Siedlerchr commented Apr 21, 2025

koppor commented Apr 21, 2025

calixtus commented Apr 21, 2025

calixtus commented Apr 21, 2025

calixtus commented Apr 21, 2025


		public AiPrivacyNoticeGuardedComponent(AiPreferences aiPreferences, ExternalApplicationsPreferences externalApplicationsPreferences, DialogService dialogService) {
		public AiPrivacyNoticeGuardedComponent(AiPreferences aiPreferences, ExternalApplicationsPreferences externalApplicationsPreferences, DialogService dialogService, AdaptVisibleTabs adaptVisibleTabs) {

Hide tabs without restart #12958

Are you sure you want to change the base?

Hide tabs without restart #12958

Conversation

koppor commented Apr 17, 2025

Mandatory checks

trag-bot bot Apr 17, 2025

Choose a reason for hiding this comment

trag-bot bot Apr 17, 2025

Choose a reason for hiding this comment

trag-bot bot Apr 17, 2025

Choose a reason for hiding this comment

trag-bot bot Apr 17, 2025

Choose a reason for hiding this comment

trag-bot bot Apr 17, 2025

Choose a reason for hiding this comment

trag-bot bot Apr 17, 2025

Choose a reason for hiding this comment

trag-bot bot Apr 17, 2025

Choose a reason for hiding this comment

trag-bot bot Apr 17, 2025

Choose a reason for hiding this comment

trag-bot bot Apr 17, 2025

Choose a reason for hiding this comment

github-actions bot commented Apr 17, 2025

calixtus commented Apr 18, 2025

koppor commented Apr 18, 2025

calixtus commented Apr 18, 2025

Siedlerchr commented Apr 21, 2025

koppor commented Apr 21, 2025

calixtus commented Apr 21, 2025

calixtus commented Apr 21, 2025

calixtus commented Apr 21, 2025