Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: ADD Refresh library menu item #391

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

luca-domenichini
Copy link
Contributor

Implemenation of #390

image

Pressing the menù item updates the library from scratch.

@abhinayagarwal abhinayagarwal added the enhancement New feature or request label Aug 31, 2021
@abhinayagarwal abhinayagarwal added this to the 18 milestone Sep 1, 2021
@luca-domenichini luca-domenichini changed the title Implementation of #390 feat: ADD Refresh library menu item Feb 8, 2022
@luca-domenichini
Copy link
Contributor Author

@abhinayagarwal any news on this?

Copy link
Collaborator

@Oliver-Loeffler Oliver-Loeffler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @luca-domenichini,
I've reviewed the code in a very basic way yet. Still playing around with it but I like the idea.
Those license headers require updates (unfortunately) and please consider replacing the ReturningRunnable with a standard Java Supplier (actually supplier does the job and we won't have to maintain the code).

Nevertheless, at a first view this looks good but I'll run a 2nd review to see if we can hook in some tests.
Sorry for the long delay!

package com.oracle.javafx.scenebuilder.kit.util;

@FunctionalInterface
public interface ReturningRunnable<T> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be okay to replace ReturningRunnable with java.util.function.Supplier?

The reason I can imagine here is just to lower maintenance efforts. Supplier is pretty much standard and we won't have to care for license and other stuff.

@@ -74,6 +74,7 @@
<RadioMenuItem fx:id="libraryViewAsSections" mnemonicParsing="false" onAction="#onLibraryViewAsSections" text="%library.panel.menu.view.sections" toggleGroup="$libraryDisplayOptionTG" />
<SeparatorMenuItem mnemonicParsing="false" />
<MenuItem mnemonicParsing="false" onAction="#onManageJarFxml" text="%library.panel.menu.manage.jar.fxml" />
<MenuItem mnemonicParsing="false" onAction="#onLibraryRefresh" text="%library.panel.menu.refresh" />
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update the license header in line 4 from Copyright (c) 2016, 2019, Gluon and/or its affiliates. to Copyright (c) 2016, 2022, Gluon and/or its affiliates..

@@ -91,12 +93,13 @@ public class LibraryDialogController extends AbstractFxmlWindowController {

private ObservableList<DialogListItem> listItems;

private Runnable onAddJar;
private Runnable onAddFolder;
private ReturningRunnable<Boolean> onAddJar;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please consider replacing the ReturningRunnable with JDKs default java.util.function.Supplier interface.

@@ -61,9 +61,11 @@
import com.oracle.javafx.scenebuilder.kit.preferences.MavenPreferences;
import com.oracle.javafx.scenebuilder.kit.preferences.PreferencesControllerBase;
import com.oracle.javafx.scenebuilder.kit.preferences.PreferencesRecordArtifact;
import com.oracle.javafx.scenebuilder.kit.util.ReturningRunnable;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change line 2 from * Copyright (c) 2016, 2021, Gluon and/or its affiliates. into * Copyright (c) 2016, 2022, Gluon and/or its affiliates..

@@ -109,6 +109,7 @@ public class MavenDialogController extends AbstractFxmlWindowController {
};

private final PreferencesControllerBase preferencesControllerBase;
private boolean confirmed;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update line 2 from * Copyright (c) 2016, 2017 Gluon and/or its affiliates. into * Copyright (c) 2016, 2022 Gluon and/or its affiliates..

@@ -31,15 +31,6 @@
*/
package com.oracle.javafx.scenebuilder.kit.editor.panel.library.maven.search;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update license header here in line 2 from * Copyright (c) 2016, 2017 Gluon and/or its affiliates. to * Copyright (c) 2016, 2022 Gluon and/or its affiliates..

@@ -43,10 +43,6 @@
import java.nio.file.Files;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update license header here in line 2 from * Copyright (c) 2017, 2021 Gluon and/or its affiliates. to * Copyright (c) 2017, 2022 Gluon and/or its affiliates..

@@ -75,7 +75,7 @@
*/
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update license header here from * Copyright (c) 2017, Gluon and/or its affiliates. to * Copyright (c) 2017, 2022, Gluon and/or its affiliates..

@AlmasB AlmasB removed this from the 18 milestone Feb 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants