Convert crossref editor to tag-like interface using JavaFX#2840
Conversation
|
This PR is now ready-for-review. There are still some points that can be improved but I think for now this is ok:
|
lenhard
left a comment
There was a problem hiding this comment.
I looked at the code and there are no major obstacles. I haven't tested this in a running JabRef, though, since I do not really use the crossref feature.
Somebody else should still test this.
| linkedEntriesBar.setOnTagClicked((parsedEntryLink, mouseEvent) -> viewModel.jumpToEntry(parsedEntryLink)); | ||
| Bindings.bindContentBidirectional(linkedEntriesBar.tagsProperty(), viewModel.linkedEntriesProperty()); | ||
|
|
||
| //TODO: Add toolitp for tag: Localization.lang("Jump to entry") |
There was a problem hiding this comment.
I assume this TODO is legacy and not coming from the current PR, right?
| } | ||
|
|
||
| public void jumpToEntry(ParsedEntryLink parsedEntryLink) { | ||
| // TODO: Implement jump to entry |
There was a problem hiding this comment.
Related to the comment above: I would prefer it very much to remove the commented-out code here directly. Way back in time when JabRef was still full of such stuff, we decided to remove commented-out code.
There was a problem hiding this comment.
Ok that policy is very understandable. In the current case, I'm not sure if we really should remove it. The "jump to entry" feature was present in previous versions and this PR removes it (temporarily). I don't see a nice way to re-implement it right now as we have no good interface to control the focus of the main table (I really don't want to directly use the JabRefFrame class). But removing the comment here and above results also in a change of the language files and the "Jump to entry" string has to be readded by the translators at some point in the future.
Should I still remove it? I don't have a strong opinion about it.
There was a problem hiding this comment.
I see. It would be better to keep it in the language file. Thus, I would say it is ok to keep the code, but please extend the explanation, so that we know that this is not just the legacy stuff. Be verbose (no-problem with a multi-line comment) and explain the rationale and why it's problematic to re-add the code, like in you comment here.
The crossref editor is the last one still implemented in Swing. With this PR it is converted to JavaFX and looks like this:
gradle localizationUpdate?