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

Refactor unlinked files #7209

Merged
merged 91 commits into from
Jan 24, 2021
Merged

Refactor unlinked files #7209

merged 91 commits into from
Jan 24, 2021

Conversation

Siedlerchr
Copy link
Member

@Siedlerchr Siedlerchr commented Dec 20, 2020

create dialog in fxml and use a background task for import

Will fix #7205 and fix #7206

Scanning:
Bildschirmfoto 2021-01-04 um 11 36 25

Importing:
Bildschirmfoto 2021-01-04 um 11 38 50

Expanded results:
Bildschirmfoto 2021-01-04 um 11 37 59

  • Finish GUI Dialog
  • Show results after Import in own dialog (see copy files)
  • Background Task for Import
  • Basic dialog structure in MVVM
  • Change in CHANGELOG.md described (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 documentation: Is the information available and up to date? If not created an issue at https://github.com/JabRef/user-documentation/issues or, even better, submitted a pull request to the documentation repository.

create dialog in fxml and use a background task for import
…dtask

* upstream/master:
  Bump pdfbox from 2.0.21 to 2.0.22 (#7213)
  Bump fontbox from 2.0.21 to 2.0.22 (#7219)
  Bump archunit-junit5-api from 0.14.1 to 0.15.0 (#7220)
  Bump unoloader from 7.0.3 to 7.0.4 (#7214)
  Bump guava from 30.0-jre to 30.1-jre (#7218)
  Bump xmpbox from 2.0.21 to 2.0.22 (#7217)
  Bump classgraph from 4.8.94 to 4.8.97 (#7211)
  Bump byte-buddy-parent from 1.10.18 to 1.10.19 (#7216)
  Bump archunit-junit5-engine from 0.14.1 to 0.15.0 (#7215)
  Bump org.beryx.jlink from 2.22.3 to 2.23.0 (#7212)
  Add missing author
  Remove field check for journal abbrev in entry editor (#7208)
copy over some methods
TODO: Import Button does not yet work, always null
…dtask

* upstream/master:
  Output java error on console, too (#7222)
  Speedup processResources (#7221)
  Fix ClipboardManager <-> Prefs ordering (#7224)
TODO: Progress not yet shown
…dtask

* upstream/master:
  fxdocs changed their URL (#7230)
  New Crowdin updates (#7227)
  Fix some fetcher test (#7225)
…dtask

* upstream/master:
  add language mapping for chinese
  remove chinese content
  fix hamcrest link
  Add Traditional Chinese (#7240)
  Show development information
  Allow manual trigger of the deployment workflow
  Release v5.2
  Adapt changelog for 5.2 release
  Update external-libraries.md
  checkstyle
  L10n master (#7235)
  fix fetcher architecture test
  Add missing author
  Add error dialog "Problem finding files" (#6842)
  Disable ACM, Google Scholar, JSTOR (#7229)
checkstyle
disable import button
@Siedlerchr Siedlerchr changed the title [WIP] Refactor unlinked files Refactor unlinked files Dec 26, 2020
@Siedlerchr
Copy link
Member Author

Siedlerchr commented Jan 15, 2021

Arrows are now changed to up and down

grafik

grafik

Siedlerchr and others added 3 commits January 17, 2021 13:58
…dtask

* upstream/master:
  Export urldate to MSOffice (#7357)
  Fix DOI fetcher and add documentation on fetcher trust levels (#6990)
@calixtus
Copy link
Member

grafik

Worked a bit on the visuals and on mvvm-pattern.

@calixtus
Copy link
Member

I took a more radical approach with the "Import" button. Since it does not close the dialog anymore, but opens imports and opens the import results pane, (AND was driving me nuts, since it was not found outside the constructor) I moved it up to the buttonbar.

grafik

@Siedlerchr
Copy link
Member Author

Any more things left or can we merge? Think it looks good now

@calixtus
Copy link
Member

I am biased, so i can't judge... 😉

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.

I have some nitpicks 😇

src/main/resources/l10n/JabRef_en.properties Outdated Show resolved Hide resolved
config/checkstyle/suppressions.xml Show resolved Hide resolved
Copy link
Member

@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

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

A few remarks!

private final FileUpdateMonitor fileUpdateMonitor;
private final ExternalFilesEntryLinker linker;
private final ExternalFilesContentImporter contentImporter;
private final UndoManager undoManager;
private final StateManager stateManager;
private List<ImportFilesResultItemViewModel> results;
Copy link
Member

Choose a reason for hiding this comment

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

Still think this, why did you marked it as "resolved"?

scanDirectoryValidator = new FunctionBasedValidator<>(directoryPath, isDirectory,
ValidationMessage.error(Localization.lang("Please enter a valid file path.")));

treeRootProperty.setValue(null);
Copy link
Member

Choose a reason for hiding this comment

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

You can use the EasyBind.OptionalProperty instead of handling with null values.

Copy link
Member Author

Choose a reason for hiding this comment

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

No, I think it's necessary to prevent showing any empty things

Copy link
Member

Choose a reason for hiding this comment

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

The optional property is similar to Optional. Just a nice wrapper so that you don't need null but have an isEmpty etc.

Copy link
Member

Choose a reason for hiding this comment

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

I took a closer look into the EasyBind package, although there is an OptionalBinding, there seems to be no OptionalProperty. I took the liberty to make this an ObjectProperty<Optional<FileNodeViewModel>>.

src/main/java/org/jabref/gui/util/FileFilterConverter.java Outdated Show resolved Hide resolved
src/main/java/org/jabref/gui/util/FileFilterConverter.java Outdated Show resolved Hide resolved
src/main/java/org/jabref/logic/l10n/Localization.java Outdated Show resolved Hide resolved
…dtask

* upstream/master:
  Upgrade citeproc to 3.x snapshot without graal (#7370)
  Fix Exception if no AzureInstrumentationKey is available (#7373)
  Update snapcraft source url (#7372)
  Fix checkstyle and adjust language
  GitBook: [master] 3 pages and 32 assets modified
  Add migration to special field (#7368)
  GitBook: [master] 5 pages and 29 assets modified
  Modify message at the duplicates found dialog (#7231)
  Fixes miss-parsed names in `AutomaticPersonsGroup` (#7228)
  Fix an issue where the password for a shared SQL database was only remembered if it was the same as the username (#7364)
  Fix harvard exporter by changing AuthorsFormatter (#7355)
  Bump styfle/cancel-workflow-action from 0.6.0 to 0.7.0 (#7363)
  Bump mockito-core from 3.7.0 to 3.7.7 (#7360)
  Bump org.beryx.jlink from 2.23.1 to 2.23.2 (#7361)
  Bump libreoffice from 7.0.3 to 7.0.4 (#7362)
  Export full month name instead of number in ms office (#7358)
…mporterbackgroundtask

* upstream/importerbackgroundtask:
  Remove obsolete language key
  Refactored for mvvm pattern and optics

# Conflicts:
#	src/main/java/org/jabref/gui/externalfiles/UnlinkedFilesDialogView.java
#	src/main/java/org/jabref/gui/externalfiles/UnlinkedFilesDialogViewModel.java
and l10n
add explaination to checkstyle
@Siedlerchr
Copy link
Member Author

Addressed all comments except for those Binding or Optional stuff, cause I can't get it to work...

tobiasdiez
tobiasdiez previously approved these changes Jan 23, 2021
Copy link
Member

@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

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

OK thanks!

@Siedlerchr Siedlerchr merged commit 86d52cd into master Jan 24, 2021
@Siedlerchr Siedlerchr deleted the importerbackgroundtask branch January 24, 2021 17:56
Siedlerchr added a commit to koppor/jabref that referenced this pull request Jan 26, 2021
* upstream/master: (217 commits)
  Fix handling of URL in file field (JabRef#7347)
  Fix expansion of bracketed expressions in RegExpBasedFileFinder (JabRef#7338)
  Refactor unlinked files (JabRef#7209)
  Add pressing enter when the search field is focused as a way to trigger search (JabRef#7377)
  Upgrade citeproc to 3.x snapshot without graal (JabRef#7370)
  Fix Exception if no AzureInstrumentationKey is available (JabRef#7373)
  Update snapcraft source url (JabRef#7372)
  Fix checkstyle and adjust language
  GitBook: [master] 3 pages and 32 assets modified
  Add migration to special field (JabRef#7368)
  GitBook: [master] 5 pages and 29 assets modified
  Modify message at the duplicates found dialog (JabRef#7231)
  Fixes miss-parsed names in `AutomaticPersonsGroup` (JabRef#7228)
  Fix an issue where the password for a shared SQL database was only remembered if it was the same as the username (JabRef#7364)
  Fix harvard exporter by changing AuthorsFormatter (JabRef#7355)
  Bump styfle/cancel-workflow-action from 0.6.0 to 0.7.0 (JabRef#7363)
  Bump mockito-core from 3.7.0 to 3.7.7 (JabRef#7360)
  Bump org.beryx.jlink from 2.23.1 to 2.23.2 (JabRef#7361)
  Bump libreoffice from 7.0.3 to 7.0.4 (JabRef#7362)
  Export full month name instead of number in ms office (JabRef#7358)
  ...

# Conflicts:
#	external-libraries.md
#	src/main/java/module-info.java
#	src/main/java/org/jabref/gui/shared/SharedDatabaseLoginDialogViewModel.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers
Projects
None yet
5 participants