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

Jump to bottom

Refactored LibraryTab #10732

Merged
merged 21 commits into from
Jan 11, 2024
Merged

Refactored LibraryTab #10732

merged 21 commits into from
Jan 11, 2024

Conversation

calixtus
Copy link
Member

@calixtus calixtus commented Dec 31, 2023 •
edited
Loading

  • Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
  • 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.

@calixtus
Removed code duplication
9fbac6a
@calixtus
Moved LibraryTab to its own package
c6e5373
@calixtus
Moved call to LibraryTab::cleanup to libraryTab itself
30ddc7e
@calixtus
Refactored BasePanelMode to LibraryTab private
eab3616
@calixtus
Refactored closing library
727d03b
@calixtus
Extracted dnd methods
a3c911a
@calixtus
Merge remote-tracking branch 'upstream/main' into cleanup-librarytab
4b4e2ed
@calixtus
Merge remote-tracking branch 'upstream/main' into cleanup-librarytab
f32e475 
# Conflicts:
#	src/main/java/org/jabref/gui/JabRefFrame.java
#	src/main/java/org/jabref/gui/duplicationFinder/DuplicateSearch.java
#	src/main/java/org/jabref/gui/exporter/ExportCommand.java
#	src/main/java/org/jabref/gui/exporter/SaveAllAction.java
#	src/main/java/org/jabref/gui/importer/ImportCommand.java
#	src/main/java/org/jabref/gui/importer/actions/GUIPostOpenAction.java
#	src/main/java/org/jabref/gui/importer/actions/MergeReviewIntoCommentAction.java
#	src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java
#	src/main/java/org/jabref/gui/library/LibraryTab.java
#	src/main/java/org/jabref/gui/maintable/MainTable.java
#	src/main/java/org/jabref/gui/preferences/ShowPreferencesAction.java
#	src/main/java/org/jabref/gui/shared/SharedDatabaseLoginDialogViewModel.java
#	src/main/java/org/jabref/gui/shared/SharedDatabaseUIManager.java
#	src/main/java/org/jabref/gui/undo/UndoRedoAction.java
#	src/test/java/org/jabref/gui/exporter/SaveDatabaseActionTest.java
#	src/test/java/org/jabref/gui/importer/NewEntryActionTest.java
@calixtus
Fixed merge errors
119052d
@Siedlerchr
Copy link
Member

Needs to be tested on mac as well, I remember there were some weird issues

@calixtus calixtus changed the title Refactoring to window closing Refactored LibraryTab Jan 10, 2024
@calixtus calixtus added status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers type: code-quality Issues related to code or architecture decisions labels Jan 10, 2024
@calixtus calixtus marked this pull request as ready for review January 10, 2024 00:43
@Siedlerchr
Copy link
Member

should also be tested with remote SQL database

koppor
koppor reviewed Jan 10, 2024
View reviewed changes
Copy link
Member

@koppor koppor left a comment

Choose a reason for hiding this comment

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

First comments

src/main/resources/csl-styles Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Please do not commit changes in the submodule

Copy link
Member Author

Choose a reason for hiding this comment

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

oops, was an accident, now i have to figure out how to remove these changes

src/main/java/org/jabref/preferences/JabRefPreferences.java Show resolved Hide resolved
src/main/java/org/jabref/http/server/Server.java Show resolved Hide resolved
src/main/java/org/jabref/gui/LibraryTabContainer.java Show resolved Hide resolved
src/main/java/org/jabref/gui/LibraryTab.java
@@ -86,6 +92,11 @@

public class LibraryTab extends Tab {
Copy link
Member

Choose a reason for hiding this comment

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

Can you add design-ideas as JavaDoc?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope, at least not yet. Would be speculation. Right now its only to get some order into the chaos.
In fact, there are still other things I wanted to do here, but i stopped to keep this reviewable. But one future step would be to get the entry editor out of the library tab and into the frame, and to fix all the bindings stuff.

Copy link
Member Author

Choose a reason for hiding this comment

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

Another thing would be to break this class into a more classes and fixing the mvvm pattern with library tab.

src/main/java/org/jabref/gui/LibraryTab.java Outdated Show resolved Hide resolved
@calixtus
Copy link
Member Author

should also be tested with remote SQL database

Tested. Works.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 10, 2024 •
edited
Loading

The build for this PR is no longer available. Please visit https://builds.jabref.org/main/ for the latest build.

@Siedlerchr Siedlerchr added this pull request to the merge queue Jan 11, 2024
Merged via the queue into main with commit b521028 Jan 11, 2024
18 checks passed
@Siedlerchr Siedlerchr deleted the cleanup-librarytab branch January 11, 2024 11:41
Siedlerchr added a commit that referenced this pull request Jan 12, 2024
@Siedlerchr
Merge remote-tracking branch 'upstream/main' into allEntriesDuplicates
c17550d 
* upstream/main: (25 commits)
  Remove remainging "testLogging" (#10769)
  Reuse JUnit 5.7's @DisabledIfEnvironmentVariable (#10768)
  Refactored LibraryTab (#10732)
  Fix annotation (#10766)
  Add initial .git-blame-ignore-revs (#10765)
  Streamline test output (#10762)
  Fix NPE when opening and re-opening a library (#10763)
  Changed default status of 'Automatically open folder of attached files' (#10748)
  Implement test cases for search (#10193)
  Add cites field to bib entries for citation relation (#10752)
  Ignore submodule changes (for normal devs) (#10754)
  Fix line endings
  Bump lycheeverse/lychee-action from 1.8.0 to 1.9.0 (#10760)
  Bump appleboy/ssh-action from 1.0.2 to 1.0.3 (#10761)
  Bump org.apache.lucene:lucene-core from 9.9.0 to 9.9.1 (#10759)
  Bump org.openrewrite.rewrite from 6.5.4 to 6.6.3 (#10758)
  Bump org.glassfish.jersey.containers:jersey-container-grizzly2-http (#10755)
  Bump org.jsoup:jsoup from 1.16.2 to 1.17.2 (#10756)
  Bump org.mockito:mockito-core from 5.7.0 to 5.8.0 (#10757)
  Remove abstract
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions left review comments

@Siedlerchr Siedlerchr Siedlerchr approved these changes

@koppor koppor koppor approved these changes

Assignees
No one assigned
Labels
status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers type: code-quality Issues related to code or architecture decisions
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@calixtus @Siedlerchr @koppor