[GSOC22] - A - Implement a fully functional three way merge UI

[HoussemNasri]

Contributes to #6190

Summary

The goal of this pr is to complete the first and second milestones listed in my proposal, which include designing and implementing a 3-way merge UI and replacing the existing merge UI with the new one in components that depend on it, which are:

• Duplicate resolver dialog
• External changes resolver dialog
• Merge entries dialog

Notable Changes

An improved diff highlighting logic and visuals

• Differences are now highlighted by coloring background rather than text, thanks to RichTextFX.
• Split view and unified view were introduced as more common terminology for viewing differences.
• Several bugs and pitfalls in the previous implementation were fixed.

Editable Merged Entry

• Now you can see and edit the merged entry from the dialog
• The merged entry, as well as the left and right entries, are completely bound, thus selecting a new entry will update the merged entry text, and modifying the merged entry text will update the selection.

Miscellaneous

• Extracted the merge toolbar into its own class
• Added a sticky header that doesn't get hidden when you scroll
• Support light and dark theme

Notes

RichTextFX is heavy + many are used in the same screen

Although RichTextFX's StyleClassedTextArea makes it very easy to change text's background color, it's heavy and can cause performance issues, so I may need to find an alternative in the future. I don't have any data yet, and I'm not sure whether RichTextFX's performance issues are critical, so I need to do some experiments before deciding whether to keep it or look for an alternative.

PDF Metadata merge dialog #7929

For keeping JabRef consistent, I agreed with my mentors to find a good strategy for converting the PDF metadata merge dialog to the new design while reusing as much of the existing code as possible. But that's for another pr.

Screenshots

Light Theme

Plain Text

Unified Diff View - Highlighting Words

Split Diff View - Highlighting Words

Dark Theme

• 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]

A controller? Doesn't this conflict with the mvvm pattern we are using?

[HoussemNasri]

The idea was that a field row would take model data (left and right field values), create the views and bind the model to it. A controller would do exactly that. It cannot be a view-model because it contains a reference to the view, and it cannot be a view because it doesn't inherit the Node class.

Also, JabRef architecture docs state that because FXML is not powerful enough and it can't do all the binding, we need a controller. So a controller does have a place in our architecture. The only new thing that I'm introducing here is the controller keyword, which I plan to remove and I removed while implementing the merging groups feature.

Other than that, I'll appreciate any ideas or hints you have about making the current design better.

[calixtus]

I was just wondering about the word.
Yes sometimes we do more in the java view class beyond binding nodes from the parsed fxml to the viewmodel (see https://www.ctan.org/tex-archive/biblio/bibtex/contrib/doc/ ), because fxml is not powerful enough and there are architectural limitations in java/javafx. However, I still see some business logic in the controller, that can be pushed into a viewmodel to be able to test it, like keeping the values of the cells and hasEqualLeftAndRightValues. It would just be great to be able to test the logic of your custom row.
Btw, not using FXML is totally ok with me, because parsing FXML takes time - a lot of time. So I think you already did it right not to introduce an FXML for the row, since parsing 10+ fxml files is wasting too much cpu time.

[Siedlerchr]

I agree with Carl, rename this, just call it FieldRow
and create a new class FieldRowViewModel where you put the logic.
You would bind it then e.g fieldNameCell.text().bindBidirectional(viewModel.fieldNameText)

[calixtus]

MergableCell? AbstractCell sounds somewhat ... abstract ... 😅

[calixtus]

Or maybe as ms excl calls it: BandedRowCell? (https://support.microsoft.com/en-us/office/apply-color-to-alternate-rows-or-columns-30002ce0-7a1c-4d70-a70c-4b6232f09f5e)

[HoussemNasri]

I kind of want to avoid using the merge keyword because there is already a class called MergedFieldCell. However, BandedRowCell sounds good; although I had to put Banded in google translate to understand it :^)

Another option could be ThreeWayMergeCell? Since the ThreeWayMerge prefix is used in other places too.

I could also use JavaFX Cell which already provide odd and even states. In addition, it has selectedProperty() and textProperty(). It is however a control so I need to implement a custom layout.

[Siedlerchr]

This should be moved to a separate viewModel

[HoussemNasri]

It is moved in pr B

[wujastyk]

This is a wonderful feature. I use it often, now. Thank you!