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

Added "Add example entry" and "Import existing PDFs" when a library is empty #12741

Open
wants to merge 24 commits into
base: main
Choose a base branch
from

Conversation

sanjyot242
Copy link

@sanjyot242 sanjyot242 commented Mar 14, 2025

  • Added "Add example entry" and "Import existing PDFs" when a library is empty
  • Add listeners to track database and table entry changes to show buttons only when database is empty

Snip1
snip2
snip3
snip4-filter
snip5-open library properties
snip5-warning directory not configured
snip6 directory exists

Closes #12662

Mandatory checks

  • I own the copyright of the code submitted and I licence 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 (for UI changes)
  • 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.

-Ensures button is hidden when entries exist but are filtered
-Add listeners to track database and table entry changes
@koppor koppor mentioned this pull request Mar 14, 2025
7 tasks
@koppor koppor mentioned this pull request Mar 16, 2025
6 tasks
-used localization
-added styles for button
…e-12662

# Conflicts:
#	src/main/java/org/jabref/gui/maintable/MainTable.java
-checks if directory exists
-triggers search for unlinked files ( if directory exists )
-Shows notification and open library properties ( if directory doesn't exist)
Comment on lines 614 to 615
dialogService.showWarningDialogAndWait(
Localization.lang("File directory is not set or does not exist!"),
Copy link
Author

Choose a reason for hiding this comment

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

Used Key which was already defined in properties , Should i update ?

@sanjyot242 sanjyot242 changed the title Add placeholder with "Add Example Entry" for empty library Added "Add example entry" and "Import existing PDFs" when a library is empty Mar 18, 2025
@sanjyot242 sanjyot242 marked this pull request as ready for review March 18, 2025 06:54
@sanjyot242 sanjyot242 requested a review from koppor March 20, 2025 12:13
Comment on lines 614 to 615
dialogService.showWarningDialogAndWait(
Localization.lang("File directory is not set or does not exist!"),
Copy link
Member

Choose a reason for hiding this comment

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

Yes, but please think whether it could be critical error, then leave the !.

@koppor
Copy link
Member

koppor commented Mar 20, 2025

I need to try out the directory.

Global prefs:

image

  • If unsaved AND "search and store relative", then not possible to import PDFs.
  • If unsaved AND "Main file directory" exists, one can import PDFs.
  • If saved, import should be good to go

- Removed Unnecessary comments
- Used withField for BibEntry instead of setField
@sanjyot242
Copy link
Author

I need to try out the directory.

Global prefs:

image

  • If unsaved AND "search and store relative", then not possible to import PDFs.
  • If unsaved AND "Main file directory" exists, one can import PDFs.
  • If saved, import should be good to go

Hey ,
So i didnt understand the flow clearly yet , According to what was mentioned in the issue , the checks were with respect to fileDirectory .

Could you please explain in detail about the global prefs checks .

Thanks

@sanjyot242 sanjyot242 requested a review from koppor March 22, 2025 02:20
@koppor
Copy link
Member

koppor commented Mar 22, 2025

I need to try out the directory.

Global prefs:

image

  • If unsaved AND "search and store relative", then not possible to import PDFs.
  • If unsaved AND "Main file directory" exists, one can import PDFs.
  • If saved, import should be good to go

Hey ,
So i didnt understand the flow clearly yet , According to what was mentioned in the issue , the checks were with respect to fileDirectory .

Could you please explain in detail about the global prefs checks .

Thanks

Please. Think of a user perspective. Read on at https://docs.jabref.org/finding-sorting-and-cleaning-entries/filelinks#directories-for-files, then my specification makes sense?!

@koppor
Copy link
Member

koppor commented Mar 22, 2025

With

Check if org.jabref.model.database.BibDatabaseContext#getFileDirectories is non-empty

That wish should be fulfilled... Hopefully. Please check

@sanjyot242
Copy link
Author

sanjyot242 commented Mar 23, 2025

With

Check if org.jabref.model.database.BibDatabaseContext#getFileDirectories is non-empty

That wish should be fulfilled... Hopefully. Please check

Yes, I’m checking this condition:

List<Path> fileDirectories = database.getFileDirectories(filePreferences);

if (fileDirectories.isEmpty()) {
    // show warning and open library properties
} else {
    // find unlinked files
}

Just to confirm inside the else block, I should also check the global preferences, right?

@sanjyot242 sanjyot242 closed this Mar 23, 2025
@sanjyot242 sanjyot242 reopened this Mar 23, 2025
libraryTab.showAndEdit(entry);
});

Button importPdfsButton = new Button(Localization.lang("Import existing PDFs"));
Copy link
Member

Choose a reason for hiding this comment

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

    // TODO: activate only if directories available
    // enable if !databaseContext.getFileDirectories(filePreferences).isEmpty()

Copy link
Member

Choose a reason for hiding this comment

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

Add hover if disabled: "Please configure a file directory"

Copy link
Author

@sanjyot242 sanjyot242 Mar 23, 2025

Choose a reason for hiding this comment

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

with respect to initial issue - Show a notification that no directory exist and then open the library properties.

what about open library properties ? are we doing that ?

Copy link
Member

Choose a reason for hiding this comment

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

When writing the issue, I forgot about the "store files next to bib file"

OK, maybe, I had the more right idea back then.

Then "just" modifiy to use "diaglosService.notify" and not some dialog -- reason: less clicks.

Can you also try to focus the field for the directory please?

grafik

Copy link
Author

Choose a reason for hiding this comment

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

Done

Comment on lines 608 to 613
dialogService.showWarningDialogAndWait(
Localization.lang("File directory is not set or does not exist."),
Localization.lang("Please configure a file directory"));

LibraryPropertiesAction libraryPropertiesAction = new LibraryPropertiesAction(stateManager);
libraryPropertiesAction.execute();
Copy link
Member

Choose a reason for hiding this comment

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

Should never happen

-Added focus for library-specific file directory
-Removed Unused Localization keys
-Updated to lambda for better readability
@sanjyot242 sanjyot242 requested a review from koppor March 27, 2025 07:24

updatePlaceholder(placeholderBox);

database.getDatabase().getEntries().addListener((ListChangeListener<BibEntry>) change -> updatePlaceholder(placeholderBox));
Copy link
Member

Choose a reason for hiding this comment

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

Does following work, too? - I don't see any usage of the variable.

Suggested change
database.getDatabase().getEntries().addListener((ListChangeListener<BibEntry>) change -> updatePlaceholder(placeholderBox));
database.getDatabase().getEntries().addListener(_ -> updatePlaceholder(placeholderBox));

Copy link
Author

Choose a reason for hiding this comment

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

I'm sticking with the original code since the lambda with an underscore parameter creates ambiguity error.


database.getDatabase().getEntries().addListener((ListChangeListener<BibEntry>) change -> updatePlaceholder(placeholderBox));

this.getItems().addListener((ListChangeListener<BibEntryTableViewModel>) change -> updatePlaceholder(placeholderBox));
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
this.getItems().addListener((ListChangeListener<BibEntryTableViewModel>) change -> updatePlaceholder(placeholderBox));
this.getItems().addListener(_ -> updatePlaceholder(placeholderBox));

Copy link
Author

Choose a reason for hiding this comment

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

I'm sticking with the original code since the lambda with an underscore parameter creates ambiguity error.

Comment on lines 586 to 596
BibEntry exampleEntry = new BibEntry(StandardEntryType.Article);
exampleEntry.withField(StandardField.AUTHOR, "Oliver Kopp and Carl Christian Snethlage and Christoph Schwentker");
exampleEntry.withField(StandardField.TITLE, "JabRef: BibTeX-based literature management software");
exampleEntry.withField(StandardField.JOURNAL, "TUGboat");
exampleEntry.withField(StandardField.VOLUME, "44");
exampleEntry.withField(StandardField.NUMBER, "3");
exampleEntry.withField(StandardField.PAGES, "441--447");
exampleEntry.withField(StandardField.DOI, "10.47397/tb/44-3/tb138kopp-jabref");
exampleEntry.withField(StandardField.ISSN, "0896-3207");
exampleEntry.withField(StandardField.ISSUE, "138");
exampleEntry.withField(StandardField.YEAR, "2023");
Copy link
Member

Choose a reason for hiding this comment

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

Chain these statemetns

Suggested change
BibEntry exampleEntry = new BibEntry(StandardEntryType.Article);
exampleEntry.withField(StandardField.AUTHOR, "Oliver Kopp and Carl Christian Snethlage and Christoph Schwentker");
exampleEntry.withField(StandardField.TITLE, "JabRef: BibTeX-based literature management software");
exampleEntry.withField(StandardField.JOURNAL, "TUGboat");
exampleEntry.withField(StandardField.VOLUME, "44");
exampleEntry.withField(StandardField.NUMBER, "3");
exampleEntry.withField(StandardField.PAGES, "441--447");
exampleEntry.withField(StandardField.DOI, "10.47397/tb/44-3/tb138kopp-jabref");
exampleEntry.withField(StandardField.ISSN, "0896-3207");
exampleEntry.withField(StandardField.ISSUE, "138");
exampleEntry.withField(StandardField.YEAR, "2023");
BibEntry exampleEntry = new BibEntry(StandardEntryType.Article)
.withField(StandardField.AUTHOR, "Oliver Kopp and Carl Christian Snethlage and Christoph Schwentker")
.withField(StandardField.TITLE, "JabRef: BibTeX-based literature management software")
.withField(StandardField.JOURNAL, "TUGboat")
.withField(StandardField.VOLUME, "44")
.withField(StandardField.NUMBER, "3")
.withField(StandardField.PAGES, "441--447")
.withField(StandardField.DOI, "10.47397/tb/44-3/tb138kopp-jabref")
.withField(StandardField.ISSN, "0896-3207")
.withField(StandardField.ISSUE, "138")
.withField(StandardField.YEAR, "2023");

Comment on lines 613 to 614
FindUnlinkedFilesAction findUnlinkedFilesAction = new FindUnlinkedFilesAction(dialogService, stateManager);
findUnlinkedFilesAction.execute();
Copy link
Member

Choose a reason for hiding this comment

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

Indent is off

Comment on lines 617 to 618


Copy link
Member

Choose a reason for hiding this comment

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

Remove empty lines at end

@koppor
Copy link
Member

koppor commented Mar 28, 2025

Tried out. Works.

image

Is it possible to increase the size of the "heading"?

Maybe to the headings of the Welcome Tab?

image

@koppor koppor added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Mar 28, 2025
Copy link

trag-bot bot commented Mar 29, 2025

@trag-bot didn't find any issues in the code! ✅✨

@sanjyot242 sanjyot242 requested a review from koppor March 29, 2025 00:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: maintable status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Empty library should offer two actions
2 participants