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.

Sign up for GitHub

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

minor refactor to JabRefDialogService #11767

Merged
merged 5 commits into from
Sep 18, 2024
Merged

minor refactor to JabRefDialogService #11767

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c6d62bc
minor refactor to JabRefDialogService
leaf-soba Sep 16, 2024
fd3b7f0
Update JabRefDialogService
leaf-soba Sep 17, 2024
53d046c
fix missing Localization.lang
leaf-soba Sep 17, 2024
15ec6de
fix missing string
leaf-soba Sep 17, 2024
a9fcc67
fix Localization.lang issue
leaf-soba Sep 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 28 additions & 30 deletions src/main/java/org/jabref/gui/JabRefDialogService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import java.util.List;
import java.util.Optional;
import java.util.function.Consumer;
import java.util.stream.Collectors;

import javafx.concurrent.Task;
import javafx.geometry.Pos;
Expand Down Expand Up @@ -227,14 +226,15 @@ public void showErrorDialogAndWait(FetcherException fetcherException) {
Optional<SimpleHttpResponse> httpResponse = fetcherException.getHttpResponse();
if (httpResponse.isPresent()) {
int statusCode = httpResponse.get().statusCode();
if (statusCode == 401) {
this.showInformationDialogAndWait(failedTitle, Localization.lang("Access denied. You are not authorized to access this resource. Please check your credentials and try again. If you believe you should have access, please contact the administrator for assistance.") + "\n\n" + localizedMessage);
} else if (statusCode == 403) {
this.showInformationDialogAndWait(failedTitle, Localization.lang("Access denied. You do not have permission to access this resource. Please contact the administrator for assistance or try a different action.") + "\n\n" + localizedMessage);
} else if (statusCode == 404) {
this.showInformationDialogAndWait(failedTitle, Localization.lang("The requested resource could not be found. It seems that the file you are trying to download is not available or has been moved. Please verify the URL and try again. If you believe this is an error, please contact the administrator for further assistance.") + "\n\n" + localizedMessage);
} else {
this.showErrorDialogAndWait(failedTitle, Localization.lang("Something is wrong on JabRef side. Please check the URL and try again.") + "\n\n" + localizedMessage);
switch (statusCode) {
case 401 ->
this.showInformationDialogAndWait(failedTitle, Localization.lang("Access denied. You are not authorized to access this resource. Please check your credentials and try again. If you believe you should have access, please contact the administrator for assistance.") + "\n\n" + localizedMessage);
case 403 ->
this.showInformationDialogAndWait(failedTitle, Localization.lang("Access denied. You do not have permission to access this resource. Please contact the administrator for assistance or try a different action.") + "\n\n" + localizedMessage);
case 404 ->
this.showInformationDialogAndWait(failedTitle, Localization.lang("The requested resource could not be found. It seems that the file you are trying to download is not available or has been moved. Please verify the URL and try again. If you believe this is an error, please contact the administrator for further assistance.") + "\n\n" + localizedMessage);
default ->
this.showErrorDialogAndWait(failedTitle, Localization.lang("Something is wrong on JabRef side. Please check the URL and try again.") + "\n\n" + localizedMessage);
}
Copy link
Member

Choose a reason for hiding this comment

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

This can be refactored even more?

content = switch (statusCode) { 401 -> Localization.lang"..");

 this.showInformationDialogAndWait(failedTitle, content);

I would just not care about info and error. I think "info" is OK even in the default case.

} else if (fetcherException instanceof FetcherClientException) {
this.showErrorDialogAndWait(failedTitle, Localization.lang("Something is wrong on JabRef side. Please check the URL and try again.") + "\n\n" + localizedMessage);
Expand Down Expand Up @@ -388,7 +388,7 @@ public <V> void showProgressDialogAndWait(String title, String content, Task<V>
}

@Override
public <V> Optional<ButtonType> showBackgroundProgressDialogAndWait(String title, String content, StateManager stateManager) {
public Optional<ButtonType> showBackgroundProgressDialogAndWait(String title, String content, StateManager stateManager) {
TaskProgressView<Task<?>> taskProgressView = new TaskProgressView<>();
EasyBind.bindContent(taskProgressView.getTasks(), stateManager.getRunningBackgroundTasks());
taskProgressView.setRetainTasks(false);
Expand Down Expand Up @@ -424,25 +424,23 @@ public void notify(String message) {
// The event log is not that user friendly (different purpose).
LOGGER.info(message);

UiTaskExecutor.runInJavaFXThread(() -> {
Notifications.create()
.text(message)
.position(Pos.BOTTOM_CENTER)
.hideAfter(TOAST_MESSAGE_DISPLAY_TIME)
.owner(mainWindow)
.threshold(5,
Notifications.create()
.title(Localization.lang("Last notification"))
.text(
"(" + Localization.lang("Check the event log to see all notifications") + ")"
+ "\n\n" + message)
.onAction(e -> {
ErrorConsoleAction ec = new ErrorConsoleAction();
ec.execute();
}))
.hideCloseButton()
.show();
});
UiTaskExecutor.runInJavaFXThread(() -> Notifications.create()
.text(message)
.position(Pos.BOTTOM_CENTER)
.hideAfter(TOAST_MESSAGE_DISPLAY_TIME)
.owner(mainWindow)
.threshold(5,
Notifications.create()
Copy link
Member

Choose a reason for hiding this comment

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

This new indent is a bit strange - maybe keep the old indent - on line 428 it also "felt" better, because it is indented 4 spaces, not 30 or so ^^

.title(Localization.lang("Last notification"))
.text(
"(" + Localization.lang("Check the event log to see all notifications") + ")"
+ "\n\n" + message)
.onAction(e -> {
ErrorConsoleAction ec = new ErrorConsoleAction();
ec.execute();
}))
.hideCloseButton()
.show());
}

@Override
Expand Down Expand Up @@ -472,7 +470,7 @@ public Optional<Path> showDirectorySelectionDialog(DirectoryDialogConfiguration
public List<Path> showFileOpenDialogAndGetMultipleFiles(FileDialogConfiguration fileDialogConfiguration) {
FileChooser chooser = getConfiguredFileChooser(fileDialogConfiguration);
List<File> files = chooser.showOpenMultipleDialog(mainWindow);
return files != null ? files.stream().map(File::toPath).collect(Collectors.toList()) : Collections.emptyList();
return files != null ? files.stream().map(File::toPath).toList() : Collections.emptyList();
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return files != null ? files.stream().map(File::toPath).toList() : Collections.emptyList();
return files != null ? files.stream().map(File::toPath).toList() : List.of();

}

private DirectoryChooser getConfiguredDirectoryChooser(DirectoryDialogConfiguration directoryDialogConfiguration) {
Expand Down
Loading