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

Making importing a single file easier (Issue #5508) #5513

Merged
merged 4 commits into from
Nov 3, 2019
Merged

Making importing a single file easier (Issue #5508) #5513

merged 4 commits into from
Nov 3, 2019

Conversation

P4trice
Copy link
Contributor

@P4trice P4trice commented Oct 25, 2019 •
edited by Siedlerchr
Loading

Fixes #5508
I added a condition to see if only one entry is being imported.
If the condition adheres -> mark the entry as selected.

  • Change in CHANGELOG.md described
  • Tests created for changes
  • Manually tested changed features in running JabRef
  • Screenshots added in PR description (for bigger UI changes)
  • Ensured that the git commit message is a good one
  • Check documentation status (Issue created for outdated help page at help.jabref.org?)

@P4trice
Added condition: if only one entry is imported, it automatically sele…
9dae8e5 
…cts said entry, making importing single files easier, referring to Isse JabRef#5508
@P4trice P4trice mentioned this pull request Oct 25, 2019
Closed
tobiasdiez
tobiasdiez requested changes Oct 25, 2019
View reviewed changes
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.

Welcome to JabRef and thanks a lot for your contribution!

The code added looks good. However, it's placed at a slightly convenient place as the withGraphic handler is invoked for every item. Since the action needs to be done only once after all entries are added to the list, I propose to move the added code after the entriesListView is initialized, i.e. after https://github.com/JabRef/jabref/pull/5513/files#diff-2a18170a87fa584872d07d90dbb8e9feR131

@P4trice
Copy link
Contributor Author

P4trice commented Oct 26, 2019

I would totally agree with you. But when I try that, it doesn't seem to select the entry.
I had though to put it there too at first. Any other suggestion as to how to make it work?

@tobiasdiez
Copy link
Member

Usually, the problem is that the code is run before the control is initialized completely. To fix this it is usually sufficient to wrap everything in a Platform.runLater(...) call. Can you please try this out here as well? Thanks

@P4trice
trying Platform.runLater around the if-statement, also tried around e…
50270d6 
…verything in 'initialize', including and excluding the if-statement
@P4trice
Copy link
Contributor Author

P4trice commented Oct 29, 2019

I tried to wrap the if-statement in a Platform.runLater(..) and it did not work.
Also tried to wrap all of 'initialze' in a Platform.runLater with and without the if-statement in it.
It did not select the single file being imported.
Do you have any suggestions, as to what I could do differently?
Sorry for the inconvenience of this PR.

tobiasdiez
tobiasdiez reviewed Oct 29, 2019
View reviewed changes
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.

That's really strange, but ok... Then please move the code again to where you had it in the first place (and add a small comment why it's placed there).

tobiasdiez
tobiasdiez reviewed Nov 1, 2019
View reviewed changes
src/main/java/org/jabref/gui/importer/ImportEntriesDialog.java Outdated
selectAllNewEntries();
}

return container;
Copy link
Member

Choose a reason for hiding this comment

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

Now you have two return statements 🦊

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am so sorry for all the inconveniences. I'll correct it immediately.

@tobiasdiez tobiasdiez merged commit 7e98453 into JabRef:master Nov 3, 2019
Siedlerchr added a commit that referenced this pull request Nov 5, 2019
@Siedlerchr
Merge remote-tracking branch 'upstream/master' into convertRemainingTest
3778214 
* upstream/master: (116 commits)
  New translations JabRef_en.properties (French) (#5564)
  Select newly added jstyle in table to prevent exception (#5556)
  Make entry editor DND behave as specified in settings (#5554)
  Disabled Windows directory picker temporarily
  Disabled Windows directory picker temporarily
  Adding wix script to support jpackage update
  Making importing a single file easier (Issue #5508) (#5513)
  Fix #5551 - Don't remove unwanted characters before first author is selected (#5558)
  Update JabRef_it.properties
  New translations JabRef_en.properties (Vietnamese)
  New translations JabRef_en.properties (Dutch)
  New translations JabRef_en.properties (French)
  New translations JabRef_en.properties (German)
  New translations JabRef_en.properties (Greek)
  New translations JabRef_en.properties (Indonesian)
  New translations JabRef_en.properties (Italian)
  New translations JabRef_en.properties (Japanese)
  New translations JabRef_en.properties (Danish)
  New translations JabRef_en.properties (Norwegian)
  New translations JabRef_en.properties (Polish)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tobiasdiez tobiasdiez tobiasdiez requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Make it easier to import single entries
2 participants
@P4trice @tobiasdiez