Add switch to change from biblatex to bibtex and vice versa (fi…

[Ka0o0]

Hi,
this PR adds an additional menu item to switch from biblatex to bibtex and vice versa. As discussed in the issue, I first wanted to add another combobox to the library properties pane but I noticed that in the save actions was a button which depended on the selected.
To keep things simple for 5.0 I rather adopted the way things used to work in Jabref 4.x by adding another menu item.

Any suggestions on this solution?

All the best

---

• 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

Updated Sreen:

[tobiasdiez]

I would really prefer if you could try to add the option in the library properties. As @Siedlerchr said in the issue, you normally don't switch between modes so a menu item is not the "correct" way to add the feature.

If the label is really the problem, you can also rename the button to "Reset to recommended" (which actually is a better name anyway). Would be nice if you could try to implement it that way. Thanks!

[Ka0o0]

@Siedlerchr and @tobiasdiez I've now added another combo box to the library properties pane.
I've also changed the name of the button to "Reset to recommended".

However, now there is reset and Reset to recommended. Isn't it a bit confusing for the user?

[Siedlerchr]

I would suggest renaming "Reset" to "clear" to avoid confusion.

I personally would have expected a radio box for switching between biblatex and bibtex but a combox is fine, too.

[koppor]

I would have expected a toggle switch - or a slider with two settings only.

https://controlsfx.bitbucket.io/org/controlsfx/control/ToggleSwitch.html

Keeping as is (drop down) is very fine for me.

+1 for renaming "Reset" to "Clear"!

[tobiasdiez]

I'm also in favor of renaming Reset to Clear. But then also the behavior needs to be changed to really clear the list. (The current behavior is strange anyway...why do we have to different sets of recommended/default actions.)

@koppor Toggle switches are for boolean values and not to switch between two options. The radio button would put a bit more emphasize on this option (as it would take more space) and would allow the user to immediately see the options. But the dropdown should be also fine.

[Ka0o0]

I would then suggest to also change the order of the buttons to keep clearing actions (remove selected and remove all) together. How about having

Reset to recommended - Remove selected - Remove all

[tobiasdiez]

A bit of code nitpicking...apart from this it looks very good.

[tobiasdiez]

If you use ComboBox<BibDatabaseMode> here (and propagate the change), then you don't have to constantly convert between formatted values and parsed modes.

[Ka0o0]

If I do so, the names are not formatted anymore. Also, the view would have knowledge about the model.
I normally always try to avoid that by converting the model to non model datatypes. So I would suggest to leave it this way, what do you think?

[tobiasdiez]

You can get the proper display quite easily using similar code as the one below:

jabref/src/main/java/org/jabref/gui/cleanup/FieldFormatterCleanupsPanel.java

Lines 174 to 177 in a76b8f3

	new ViewModelListCellFactory<Formatter>()
	.withText(Formatter::getName)
	.withStringTooltip(Formatter::getDescription)
	.install(formattersCombobox);

You are right, one should usually not use model classes directly in the ui. So here the "correct" solution would be to wrap the DatabaseMode in a DatabaseModeViewModel which exposes the getFormattedName method. I feel like this is however overkill for this particular situation.

[Ka0o0]

Before this PR is getting merged, I want to discuss the following:
Should we consider updating the save actions if the user changes the database mode and he still has the default/recommended save actions set?

[calixtus]

Hi, Ka0o0.
About the buttons: since I saw that iconbuttons were used in some places, I implemented them also in the preferences dialog. Maybe this an idea here too? You could also implement the "remove selected" button in an action column on the right. Just an idea, so the ui would be a bit more consistent.

[Ka0o0]

Hi @calixtus,
yes but I think, we should try to keep this PR small and discuss this separately. Maybe you can create another suggestion issue then?

[tobiasdiez]

Should we consider updating the save actions if the user changes the database mode and he still has the default/recommended save actions set?

I think this would be a neat addition!

[koppor]

@Ka0o0 +1 for changing the default save actions. This would IMHO be very intuitive. - Only pro users change the save actions.

Refs #2051

Refs #2601 - follow-up-issue: #3644

[Ka0o0]

Okay, great. I will create an issue for this then.

[koppor]

@Ka0o0 I currently see following checklist at the PR description:

Could you update this regarding what is missing? - Background: We go through the PRs and check for the status. It would be very helpful if the checklist was modified w.r.t. the status. - No over-engineering here. If you just have to work again, you can prefix the PR title with [WIP] . So we know, the token is on your side - and not on ours to check review comments. I am aware that the PR title then changes often - however, it is more easy to manage (at least for me).

[koppor]

@Ka0o0 While resolving the conflicts, I saw that a new string "Database mode" was inserted to the language file. We decided to use "Library" instead of "Database". See #2095 (refs #2748).

We use %0 mode, New %0 library. We have Library properties, so it is IMHO OK to add Library mode. Could you please change that @Ka0o0? This is IMHO the last thing TODO here, right @tobiasdiez?

[Ka0o0]

@koppor @tobiasdiez sorry for my late reply. I was on holiday this we.
I have changed the strings now. Does this PR need anything else?

[koppor]

@Ka0o0 Thank you for following up 🎉

[tobiasdiez]

Thanks for your work on this issue, and I'm sorry that you came under friendly crossfire on how to best implement this feature which made the whole progress a bit longer than it needed be. I promise the next one will be more straightforward ;-)