Skip to content

Commit

Permalink
[GSOC22] - A - Implement a fully functional three way merge UI (#8945)
Browse files Browse the repository at this point in the history
* Update README.md

* Fix Readme

* Create grid cells for rendering field name and field value

* Implement FieldNameCell and define style class

* Make FieldValueCell selectable

- Defined style class
- Defined pseudo class for the selected state
- Created the selection box UI

* Implement MergedFieldCell for rendering editable merged field value

* Change MergedFieldCell text based on the selected FieldValueCell

* Style MergedFieldCell

* Rename ThreeWayMerge css stylesheet

* Create HeadView for rendering left, right and merged entry headers

* Fix typo

* Fix a bug that causes the selection of both left and right field cells simultaneously.

- Added a style class for field value cell

* Prevent the selection of FieldValueCell when it's disabled

- A cell is disabled when it's either empty or not visible e.g. when the left cell spans 2 columns, right cell is disabled because it's not visible
- Added comments

* Display a copy icon inside FieldValueCell

* Disable left or right FieldValueCell when it's empty

- Added getters for row cells

* Update Base CSS style

* Design and implement the merge toolbox

- It will be added to the top of the merge dialog

* Implement the initial prototype of the new merge UI

- Renamed ThreeWayMerge.java to ThreeWayMergeView

* Use ThreeWayMergeView in MergeEntriesDialog

- At this stage headers aren't updated and merged bib entry is not created

* Fix ThreeWayMergeView.css stylesheet not being imported

* Span left field value to 2 columns when left and right field values are the same

* Wrap the copy icon inside a button

- Updated the selection box UI

* Use HAND cursor when hover over FieldValueCell

* Move field cells styles to ThreeWayMergeView.css

* Update merge toolbox FXML UI

* Implement and style HeaderCell

* Take row index as an argument in AbstractCell

- Created odd and even pseudo classes that will update the background color of the cell when state changes.
- Created NO_ROW_INDEX to be used by HeaderCell, because HeaderCell doesn't base its background color on its row position which is 0.

* Color cell background based on its row position (odd or even)

* Bind textArea.textProperty() and textProperty() using BindingsHelper

* Copy field value to Clipboard user clicks on the copy button

- Renamed copy-icon style class
- Used ActionFactory for creating the copy icon button

* Implement a more complete HeaderView

- Created ColumnConstraints that matches the field grid constraints, so when the window is resized grid and header resize equally

* Pass left and right headers to HeaderView

* Create merged entry

* Add the entry type field at the top of the fields list

- Sorted fields by name
- Removed internal fields

* Display entry type and bind merged entry

* Minor changes

* Style CSS

* Update ThreeWayMergeToolbox.fxml

* Remove MergeEntries from MergeEntriesDialog

* Create EmptyCell for optimization

- When cell is empty there is no point of creating a label object

* Update HeaderCell

* Rename HeaderView to ThreeWayMergeHeaderView

* Add buttons to accept all left or right entry changes to ThreeWayMergeToolbar

* Disable right field cell when it's not visible

* Listen for select all left/right toolbar buttons

* Style stuff

* Add DiffHighlighter

- Implemented UnifiedDiffHighlighter

* Create DiffHighlighter.css

* Add left padding to HeaderCell

* Move diff highlighting styles to Dark.css

- Just for testing

* Show differences between left and right values using a unified diff view

- Just for testing

* Change MergeEntriesDialog stage style

* Remove BackgroundTone from AbstractCell

* Move CopyFieldValueCommand

* Make DiffMethod public

* Move CopyFieldValueCommand

* Implement SplitDiffHighlighter

* Show diffs when user selects "Show Diff" in the toolbar UI

- Only unified diff view will be shown for now

* Fix bugs in UnifiedDiffHighlighter

* Style css stuff

* Fix typos

* Refactor SplitDiffHighlighter and fix bugs

- Added "updated" style class for styling CHANGE diffs

* Remove redundant constructor from UnifiedDiffHighlighter

* Allow users to switch between split and unified diff view

- Fixed some bugs

* Set left and right headers in MergeEntriesDialog

* Refactor ThreeWayMergeHeaderView

* Remove wellbehavedfx module dependency

* Delete EmptyCell.java

- I might recreate it later, but for now, I'm choosing simplicity over performance

* Refactor FieldValueCell

* Use the new merge UI in DuplicateResolverDialog

* Expose an API to change diff view and highlight method from outside the merge toolbar

* Use the new merge UI when an entry is changed from external

* Minor styling

* Remove DiffHighlighter.css stylesheet

- You can find diff styles in Base.css and Dark.css

* Remove unused dependencies

* Allow selecting empty field values

* Fix readme :(

* Use a theme color for selection box background

* Refactor FieldValueCell

* Add an action to FieldValueCell for opening Urls

* Add the ability to open DOI identifier

* Cleanup

* Checkstyle

* Improve URL validation logic

* Use DiffMethod in merge toolbar for consistency

* i18n

* Cleanup css

* Remove redundant icon style

* Rename style class field-value to merge-field-value

- Since it is used in Base.css and Dark.css, it is critical to distinguish the field cell used is related to the merge UI.

* Remove old Merge entries UI

* Fix missing header styles

* i18n

* Garbage collect right cell when it's invisible

- When both the left and right cells have the same value, only the left value is displayed, making it unnecessary to keep allocating memory for the right cell.

* Fix typo

Co-authored-by: Siedlerchr <siedlerkiller@gmail.com>
  • Loading branch information
HoussemNasri and Siedlerchr authored Aug 10, 2022
1 parent 1c84a5c commit a90b868
Show file tree
Hide file tree
Showing 29 changed files with 1,644 additions and 466 deletions.
48 changes: 48 additions & 0 deletions src/main/java/org/jabref/gui/Base.css
Original file line number Diff line number Diff line change
@@ -1,4 +1,13 @@


.root {
-jr-row-odd-background: -fx-control-inner-background-alt;
-jr-row-even-background: -fx-control-inner-background;
/*
On light theme, the text is hard to see when it's on top of the accent color. This is an alternative lighter accent color
for better text visibility.
*/
-jr-accent-alt: derive(-jr-accent, 15%);

/*
The theme color and some derived colors from it are used for icons, tab-headers, marking of selected inputs and
Expand All @@ -9,6 +18,7 @@
/* This theme is the original JabRef dark blue color */
-jr-theme: #50618F;
-jr-accent: #a3b7e6;
-jr-transparent-accent: rgba(163, 183, 230, 0.16);
-jr-selected: -jr-accent;
-jr-checked: -jr-theme;
-jr-hover: #0002;
Expand Down Expand Up @@ -245,6 +255,23 @@
-jr-header-height: 3em;
}

.unchanged {
-rtfx-background-color:#0000;
}

.updated {
-rtfx-background-color: rgba(41, 166, 236, 0.66);
}

.addition {
-rtfx-background-color: rgba(29, 209, 161, 0.5);
}

.deletion {
-rtfx-background-color: rgba(255, 107, 107, 0.55);
}


#frame {
-fx-background-color: -jr-background-alt;
}
Expand Down Expand Up @@ -607,6 +634,27 @@ TextFlow > .tooltip-text-monospaced {
-fx-background-insets: 0;
}

.merge-field-value .action-icon {
-fx-blend-mode: multiply;
-fx-opacity: 69%;
-fx-icon-size: 14;
-fx-icon-color: -fx-text-background-color;
}

.merge-field-value:disabled .action-icon {
-fx-opacity: 0%;
}

.merge-header-cell {
-fx-border-width: 0 0 1 0;
-fx-border-color: -jr-gray-1;
-fx-background-color: -jr-row-even-background;
}

.merge-header {
-fx-background-color: -jr-row-even-background;
}

.table-view .groupColumnBackground {
-fx-stroke: -jr-gray-2;
}
Expand Down
30 changes: 30 additions & 0 deletions src/main/java/org/jabref/gui/Dark.css
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
-jr-selected: -jr-accent;
-jr-hover: #fff1;

-jr-row-odd-background: #272b38;
-jr-row-even-background: #212330;
-jr-accent-alt: -jr-accent;

-jr-red: #b71c1f;
-jr-light-red: #db1d2b;
Expand Down Expand Up @@ -60,6 +63,18 @@
-js-summary-text-color-selected: derive( -fx-dark-text-color, 70%);
}

.unchanged {

}

.addition {
-rtfx-background-color: -jr-green;
}

.deletion {
-rtfx-background-color: -jr-red;
}

#previewBody {
background-color: #272b38; /* -fx-control-inner-background*/
color: #7d8591; /* -fx-mid-text-color*/
Expand All @@ -82,6 +97,21 @@
-fx-background-color: -jr-hover;
}

.merge-field-value .action-icon {
-fx-blend-mode: none;
-fx-opacity: 90%;
}

.merge-header-cell {
-fx-border-width: 0 0 1 0;
-fx-border-color: -fx-outer-border;
-fx-background-color: -jr-row-odd-background;
}

.merge-header {
-fx-background-color: -jr-row-odd-background;
}

.table-view .groupColumnBackground {
-fx-stroke: -jr-gray-3;
}
Expand Down
22 changes: 13 additions & 9 deletions src/main/java/org/jabref/gui/collab/EntryChangeViewModel.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
import javafx.scene.control.Label;
import javafx.scene.layout.VBox;

import org.jabref.gui.mergeentries.MergeEntries;
import org.jabref.gui.mergeentries.MergeEntries.DefaultRadioButtonSelectionMode;
import org.jabref.gui.mergeentries.newmergedialog.ShowDiffConfig;
import org.jabref.gui.mergeentries.newmergedialog.ThreeWayMergeView;
import org.jabref.gui.mergeentries.newmergedialog.diffhighlighter.DiffHighlighter;
import org.jabref.gui.mergeentries.newmergedialog.toolbar.ThreeWayMergeToolbar;
import org.jabref.gui.undo.NamedCompound;
import org.jabref.gui.undo.UndoableInsertEntries;
import org.jabref.logic.l10n.Localization;
Expand All @@ -17,7 +19,7 @@ class EntryChangeViewModel extends DatabaseChangeViewModel {

private final BibEntry oldEntry;
private final BibEntry newEntry;
private MergeEntries mergePanel;
private ThreeWayMergeView threeWayMergeView;

public EntryChangeViewModel(BibEntry entry, BibEntry newEntry) {
super();
Expand All @@ -37,17 +39,17 @@ public EntryChangeViewModel(BibEntry entry, BibEntry newEntry) {
public void setAccepted(boolean accepted) {
super.setAccepted(accepted);
if (accepted) {
mergePanel.selectAllRightRadioButtons();
threeWayMergeView.selectRightEntryValues();
} else {
mergePanel.selectAllLeftRadioButtons();
threeWayMergeView.selectLeftEntryValues();
}
}

@Override
public void makeChange(BibDatabaseContext database, NamedCompound undoEdit) {
this.description(); // Init dialog to prevent NPE
database.getDatabase().removeEntry(oldEntry);
BibEntry mergedEntry = mergePanel.getMergedEntry();
BibEntry mergedEntry = threeWayMergeView.getMergedEntry();
mergedEntry.setId(oldEntry.getId()); // Keep ID
database.getDatabase().insertEntry(mergedEntry);
undoEdit.addEdit(new UndoableInsertEntries(database.getDatabase(), oldEntry));
Expand All @@ -56,13 +58,15 @@ public void makeChange(BibDatabaseContext database, NamedCompound undoEdit) {

@Override
public Node description() {
mergePanel = new MergeEntries(oldEntry, newEntry, Localization.lang("In JabRef"), Localization.lang("On disk"), DefaultRadioButtonSelectionMode.LEFT);
threeWayMergeView = new ThreeWayMergeView(oldEntry, newEntry, Localization.lang("In JabRef"), Localization.lang("On disk"));
threeWayMergeView.selectLeftEntryValues();
threeWayMergeView.showDiff(new ShowDiffConfig(ThreeWayMergeToolbar.DiffView.SPLIT, DiffHighlighter.DiffMethod.WORDS));
VBox container = new VBox(10);
Label header = new Label(name);
header.getStyleClass().add("sectionHeader");
container.getChildren().add(header);
container.getChildren().add(mergePanel);
VBox.setMargin(mergePanel, new Insets(5, 5, 5, 5));
container.getChildren().add(threeWayMergeView);
VBox.setMargin(threeWayMergeView, new Insets(5, 5, 5, 5));
return container;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import org.jabref.gui.StateManager;
import org.jabref.gui.duplicationFinder.DuplicateResolverDialog.DuplicateResolverResult;
import org.jabref.gui.help.HelpAction;
import org.jabref.gui.mergeentries.MergeEntries;
import org.jabref.gui.mergeentries.newmergedialog.ThreeWayMergeView;
import org.jabref.gui.util.BaseDialog;
import org.jabref.gui.util.DialogWindowState;
import org.jabref.logic.help.HelpFile;
Expand Down Expand Up @@ -39,7 +39,7 @@ public enum DuplicateResolverResult {
BREAK
}

private MergeEntries mergeEntries;
private ThreeWayMergeView threeWayMerge;
private final DialogService dialogService;

public DuplicateResolverDialog(BibEntry one, BibEntry two, DuplicateResolverType type, BibDatabaseContext database, StateManager stateManager, DialogService dialogService) {
Expand Down Expand Up @@ -69,14 +69,14 @@ private void init(BibEntry one, BibEntry two, DuplicateResolverType type) {
first = new ButtonType(Localization.lang("Keep left"), ButtonData.APPLY);
second = new ButtonType(Localization.lang("Keep right"), ButtonData.APPLY);
both = new ButtonType(Localization.lang("Keep both"), ButtonData.APPLY);
mergeEntries = new MergeEntries(one, two);
threeWayMerge = new ThreeWayMergeView(one, two);
break;
case INSPECTION:
first = new ButtonType(Localization.lang("Remove old entry"), ButtonData.APPLY);
second = new ButtonType(Localization.lang("Remove entry from import"), ButtonData.APPLY);
both = new ButtonType(Localization.lang("Keep both"), ButtonData.APPLY);
mergeEntries = new MergeEntries(one, two, Localization.lang("Old entry"),
Localization.lang("From import"));
threeWayMerge = new ThreeWayMergeView(one, two, Localization.lang("Old entry"),
Localization.lang("From import"));
break;
case DUPLICATE_SEARCH_WITH_EXACT:
first = new ButtonType(Localization.lang("Keep left"), ButtonData.APPLY);
Expand All @@ -85,14 +85,14 @@ private void init(BibEntry one, BibEntry two, DuplicateResolverType type) {

removeExactVisible = true;

mergeEntries = new MergeEntries(one, two);
threeWayMerge = new ThreeWayMergeView(one, two);
break;
default:
first = new ButtonType(Localization.lang("Import and remove old entry"), ButtonData.APPLY);
second = new ButtonType(Localization.lang("Do not import entry"), ButtonData.APPLY);
both = new ButtonType(Localization.lang("Import and keep old entry"), ButtonData.APPLY);
mergeEntries = new MergeEntries(one, two, Localization.lang("Old entry"),
Localization.lang("From import"));
threeWayMerge = new ThreeWayMergeView(one, two, Localization.lang("Old entry"),
Localization.lang("From import"));
break;
}
if (removeExactVisible) {
Expand All @@ -109,7 +109,7 @@ private void init(BibEntry one, BibEntry two, DuplicateResolverType type) {
this.setY(state.getY());
}

BorderPane borderPane = new BorderPane(mergeEntries);
BorderPane borderPane = new BorderPane(threeWayMerge);
borderPane.setBottom(options);

this.setResultConverter(button -> {
Expand All @@ -136,6 +136,6 @@ private void init(BibEntry one, BibEntry two, DuplicateResolverType type) {
}

public BibEntry getMergedEntry() {
return mergeEntries.getMergedEntry();
return threeWayMerge.getMergedEntry();
}
}
7 changes: 6 additions & 1 deletion src/main/java/org/jabref/gui/fieldeditors/URLUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,12 @@ public static String cleanGoogleSearchURL(String url) {
* @return true if <c>url</c> contains a valid URL
*/
public static boolean isURL(String url) {
return url.contains("://");
try {
new URL(url);
return true;
} catch (MalformedURLException e) {
return false;
}
}

/**
Expand Down
6 changes: 5 additions & 1 deletion src/main/java/org/jabref/gui/icon/IconTheme.java
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,11 @@ public enum JabRefIcons implements JabRefIcon {
KEEP_SEARCH_STRING(MaterialDesignE.EARTH),
KEEP_ON_TOP(MaterialDesignP.PIN),
KEEP_ON_TOP_OFF(MaterialDesignP.PIN_OFF_OUTLINE),
OPEN_GLOBAL_SEARCH(MaterialDesignO.OPEN_IN_NEW);
OPEN_GLOBAL_SEARCH(MaterialDesignO.OPEN_IN_NEW),

ACCEPT_LEFT(MaterialDesignS.SUBDIRECTORY_ARROW_LEFT),

ACCEPT_RIGHT(MaterialDesignS.SUBDIRECTORY_ARROW_RIGHT);

private final JabRefIcon icon;

Expand Down
12 changes: 0 additions & 12 deletions src/main/java/org/jabref/gui/mergeentries/MergeEntries.css

This file was deleted.

Loading

0 comments on commit a90b868

Please sign in to comment.