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.

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

Fix for issue 4682 : Restructuring the side pane structure having additional functionality and merging the remove group menus #7714

Merged
merged 24 commits into from
Jun 8, 2021
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
4ffcffb
removed three menu titles: Remove subgroups, Remove group and subgrou…
josipovicjohn Apr 15, 2021
8bd0748
Move the add group button to the bottom of the group view pane
damassh Apr 30, 2021
53c75c7
created remove group submenu function in group tree view
josipovicjohn Apr 30, 2021
22549b3
Fix the header tile in the add group/subgroup dialog box
Apr 30, 2021
f851464
implemented remove group submenu in view only
josipovicjohn Apr 30, 2021
955523f
fixed menu titles
josipovicjohn Apr 30, 2021
96f69d4
implemented subgroup behaviours in menu
josipovicjohn May 3, 2021
e196041
about to merge upstream with local
josipovicjohn May 3, 2021
783198a
Ready to test implementation of submenu
josipovicjohn May 3, 2021
35b9a06
fixed the header title with the latest code
May 4, 2021
705f29f
Update CHANGELOG.md with fixes for #4682
apurvachuri May 8, 2021
f97c656
Merge branch 'main' into fix-for-issue-4682
Siedlerchr May 8, 2021
d8ab8bb
Change the maxWidth of the `add group` button to infinity
damassh May 11, 2021
1cc0946
Add the maxwidth of `add group` button in fxml and remove it form css
damassh May 12, 2021
c9cdff3
Changed isGroup boolean to enum declaration, added GroupDialogHeader.…
May 12, 2021
c69b9cb
Revert "Changed isGroup boolean to enum declaration, added GroupDialo…
May 12, 2021
29f51b3
Revert "Revert "Changed isGroup boolean to enum declaration, added Gr…
May 12, 2021
9ff3879
Revert "Changed isGroup boolean to enum declaration, added GroupDialo…
May 13, 2021
fb2ffc8
Changed isGroup boolean to enum declaration, added GroupDialogHeader.…
May 13, 2021
051c5d6
passed LocalizationConsistencyTest
josipovicjohn May 19, 2021
28b1761
Delete dupllicate `add group`
damassh May 20, 2021
b9f7012
renamed langauge localization keys in UndoableAddOrRemoveGroup.java a…
josipovicjohn May 20, 2021
6d9a554
Update CHANGELOG.md
apurvachuri May 20, 2021
edd440c
Use PseudoClass to implement the changes of `NewGroup` button
damassh May 26, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
- We added two new fields to track the creation and most recent modification date and time for each entry. [koppor#130](https://github.com/koppor/jabref/issues/130)
- We added a feature that allows the user to copy highlighted text in the preview window. [#6962](https://github.com/JabRef/jabref/issues/6962)
- We added a feature that allows you to create new BibEntry via paste arxivId [#2292](https://github.com/JabRef/jabref/issues/2292)
- We added a add group functionality at the bottom of the side pane. [#4682](https://github.com/JabRef/jabref/issues/4682)
- We added a feature that allows the user to choose whether to trust the target site when unable to find a valid certification path from the file download site. [#7616](https://github.com/JabRef/jabref/issues/7616)

### Changed
Expand All @@ -35,6 +36,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
- We improved the linking of the `python3` interpreter via the shebang to dynamically use the systems default Python. Related to [JabRef-Browser-Extension #177](https://github.com/JabRef/JabRef-Browser-Extension/issues/177)
- Automatically found pdf files now have the linking button to the far left and uses a link icon with a plus instead of a briefcase. The file name also has lowered opacity(70%) until added. [#3607](https://github.com/JabRef/jabref/issues/3607)
- We simplified the select entry type form by splitting it into two parts ("Recommended" and "Others") based on internal usage data. [#6730](https://github.com/JabRef/jabref/issues/6730)
- We improved the submenu list by merging the'Remove group' having two options, with or without subgroups. [#4682](https://github.com/JabRef/jabref/issues/4682)
- The export to MS Office XML now uses the month name for the field `Month` instead of the two digit number [forum#2685](https://discourse.jabref.org/t/export-month-as-text-not-number/2685)

### Fixed
Expand Down Expand Up @@ -79,6 +81,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
- We fixed an issue where font size of the preferences dialog does not update with the rest of the GUI. [#7416](https://github.com/JabRef/jabref/issues/7416)
- We fixed an issue in which a linked online file consisting of a web page was saved as an invalid pdf file upon being downloaded. The user is now notified when downloading a linked file results in an HTML file. [#7452](https://github.com/JabRef/jabref/issues/7452)
- We fixed an issue where opening BibTex file (doubleclick) from Folder with spaces not working. [#6487](https://github.com/JabRef/jabref/issues/6487)
- We fixed the header title in the Add Group/Subgroup Dialog box. [#4682](https://github.com/JabRef/jabref/issues/4682)
- We fixed an issue with saving large `.bib` files [#7265](https://github.com/JabRef/jabref/issues/7265)
- We fixed an issue with very large page numbers [#7590](https://github.com/JabRef/jabref/issues/7590)
- We fixed an issue with opacity of disabled icon-buttons [#7195](https://github.com/JabRef/jabref/issues/7195)
Expand All @@ -92,6 +95,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
- We fixed an issue where the RFC fetcher is not compatible with the draft [7305](https://github.com/JabRef/jabref/issues/7305)

### Removed
- We removed add group button beside the filter group tab. [#4682](https://github.com/JabRef/jabref/issues/4682)
Copy link
Member

Choose a reason for hiding this comment

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

Keep a blank line before and after a heading.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a blank line after the heading "#Removed"

Copy link
Member

Choose a reason for hiding this comment

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

In this case, Visual Studio Code helps - with following plugin: https://marketplace.visualstudio.com/items?itemName=DavidAnson.vscode-markdownlint

I am not aware of any similar plugin for IntelliJ.


## [5.2] – 2020-12-24

Expand Down
10 changes: 7 additions & 3 deletions src/main/java/org/jabref/gui/groups/GroupDialogView.java
Original file line number Diff line number Diff line change
Expand Up @@ -67,15 +67,19 @@ public class GroupDialogView extends BaseDialog<AbstractGroup> {
private final ControlsFxVisualizer validationVisualizer = new ControlsFxVisualizer();
private final GroupDialogViewModel viewModel;

public GroupDialogView(DialogService dialogService, BibDatabaseContext currentDatabase, PreferencesService preferencesService, AbstractGroup editedGroup) {
viewModel = new GroupDialogViewModel(dialogService, currentDatabase, preferencesService, editedGroup);
public GroupDialogView(DialogService dialogService, BibDatabaseContext currentDatabase, PreferencesService preferencesService, AbstractGroup editedGroup, boolean isGroup) {
viewModel = new GroupDialogViewModel(dialogService, currentDatabase, preferencesService, editedGroup, isGroup);

ViewLoader.view(this)
.load()
.setAsDialogPane(this);

if (editedGroup == null) {
this.setTitle(Localization.lang("Add subgroup"));
if (isGroup == true) {
calixtus marked this conversation as resolved.
Show resolved Hide resolved
this.setTitle(Localization.lang("Add group")); // header title
calixtus marked this conversation as resolved.
Show resolved Hide resolved
} else {
this.setTitle(Localization.lang("Add subgroup")); // header title\
}
} else {
this.setTitle(Localization.lang("Edit group") + " " + editedGroup.getName());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,12 +107,14 @@ public class GroupDialogViewModel {
private final PreferencesService preferencesService;
private final BibDatabaseContext currentDatabase;
private final AbstractGroup editedGroup;
private final boolean isGroup;

public GroupDialogViewModel(DialogService dialogService, BibDatabaseContext currentDatabase, PreferencesService preferencesService, AbstractGroup editedGroup) {
public GroupDialogViewModel(DialogService dialogService, BibDatabaseContext currentDatabase, PreferencesService preferencesService, AbstractGroup editedGroup, boolean isGroup) {
this.dialogService = dialogService;
this.preferencesService = preferencesService;
this.currentDatabase = currentDatabase;
this.editedGroup = editedGroup;
this.isGroup = isGroup;
calixtus marked this conversation as resolved.
Show resolved Hide resolved

setupValidation();
setValues();
Expand Down
13 changes: 11 additions & 2 deletions src/main/java/org/jabref/gui/groups/GroupTree.css
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,8 @@
-fx-translate-x: -0.4em;
}

#newGroupButton {
-fx-padding: 0.1em 1.5em 0.1em 1.5em;
#addNewGroup {
/*-fx-padding: 0.1em 1.5em 0.1em 1.5em;*/
}
Copy link
Member

Choose a reason for hiding this comment

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

Why keeping it, when commented? For version history we got git.


#groupFilterBar {
Expand All @@ -116,3 +116,12 @@
#groupFilterBar .glyph-icon {
-fx-font-size: 2em;
}

#groupBar .glyph-icon {
-fx-font-size: 2em;
}

.add-group-button:hover {
-fx-background-color: -jr-icon-background-active;
-fx-text-fill: -jr-black;
}
21 changes: 11 additions & 10 deletions src/main/java/org/jabref/gui/groups/GroupTree.fxml
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,12 @@
<?import javafx.scene.control.TreeTableView?>
<?import javafx.scene.layout.BorderPane?>
<?import javafx.scene.layout.HBox?>
<?import org.jabref.gui.icon.JabRefIconView?>
<?import org.controlsfx.control.textfield.CustomTextField?>
<BorderPane xmlns:fx="http://javafx.com/fxml/1" prefHeight="600.0" prefWidth="150.0"
xmlns="http://javafx.com/javafx/8.0.112" fx:controller="org.jabref.gui.groups.GroupTreeView">
<top>
<HBox fx:id="groupFilterBar" alignment="CENTER" spacing="8">
<CustomTextField fx:id="searchField" promptText="%Filter groups" HBox.hgrow="ALWAYS"/>
<Button fx:id="newGroupButton" onAction="#addNewGroup" styleClass="icon-button"
ButtonBar.buttonData="RIGHT">
<graphic>
<JabRefIconView glyph="NEW_GROUP" glyphSize="18"/>
</graphic>
<tooltip>
<Tooltip text="%New group"/>
</tooltip>
</Button>
</HBox>
</top>
<center>
Expand All @@ -39,4 +29,15 @@
</columnResizePolicy>
</TreeTableView>
</center>
<bottom>
<HBox fx:id="groupBar" alignment="CENTER" spacing="8">
<Button fx:id="addNewGroup" onAction="#addNewGroup" styleClass="add-group-button"
calixtus marked this conversation as resolved.
Show resolved Hide resolved
text="%Add group" maxWidth="1.7976931348623157E308" HBox.hgrow="ALWAYS">
calixtus marked this conversation as resolved.
Show resolved Hide resolved
<tooltip>
<Tooltip text="%New group"/>
</tooltip>
</Button>
</HBox>

</bottom>
</BorderPane>
63 changes: 48 additions & 15 deletions src/main/java/org/jabref/gui/groups/GroupTreeView.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@
import javafx.beans.property.ObjectProperty;
import javafx.css.PseudoClass;
import javafx.fxml.FXML;
import javafx.scene.control.Button;
import javafx.scene.control.ContextMenu;
import javafx.scene.control.Control;
import javafx.scene.control.Menu;
import javafx.scene.control.MenuItem;
import javafx.scene.control.SelectionMode;
import javafx.scene.control.SeparatorMenuItem;
Expand Down Expand Up @@ -64,6 +66,7 @@ public class GroupTreeView {
@FXML private TreeTableColumn<GroupNodeViewModel, GroupNodeViewModel> numberColumn;
@FXML private TreeTableColumn<GroupNodeViewModel, GroupNodeViewModel> expansionNodeColumn;
@FXML private CustomTextField searchField;
@FXML private Button addNewGroup;

@Inject private StateManager stateManager;
@Inject private DialogService dialogService;
Expand Down Expand Up @@ -101,6 +104,8 @@ public void initialize() {
});
searchField.textProperty().addListener((observable, oldValue, newValue) -> searchTask.restart());

setNewGroupButtonStyle(groupTree);

groupTree.rootProperty().bind(
EasyBind.map(viewModel.rootGroupProperty(),
group -> {
Expand Down Expand Up @@ -171,6 +176,7 @@ public void initialize() {
groupTree.setRowFactory(treeTable -> {
TreeTableRow<GroupNodeViewModel> row = new TreeTableRow<>();
row.treeItemProperty().addListener((ov, oldTreeItem, newTreeItem) -> {
setNewGroupButtonStyle(treeTable);
boolean isRoot = newTreeItem == treeTable.getRoot();
row.pseudoClassStateChanged(rootPseudoClass, isRoot);

Expand Down Expand Up @@ -322,7 +328,9 @@ private Optional<TreeItem<GroupNodeViewModel>> getTreeItemByValue(TreeItem<Group
}

private ContextMenu createContextMenuForGroup(GroupNodeViewModel group) {

ContextMenu menu = new ContextMenu();
Menu removeGroup = new Menu(Localization.lang("Remove group"));

MenuItem editGroup = new MenuItem(Localization.lang("Edit group"));
editGroup.setOnAction(event -> {
Expand All @@ -331,33 +339,43 @@ private ContextMenu createContextMenuForGroup(GroupNodeViewModel group) {
groupTree.refresh();
});

MenuItem removeGroupKeepSubgroups = new MenuItem(Localization.lang("Keep subgroups"));
removeGroupKeepSubgroups.setOnAction(event -> viewModel.removeGroupKeepSubgroups(group));

MenuItem removeGroupAndSubgroups = new MenuItem(Localization.lang("Also remove subgroups"));
removeGroupAndSubgroups.setOnAction(event -> viewModel.removeGroupAndSubgroups(group));

MenuItem addSubgroup = new MenuItem(Localization.lang("Add subgroup"));
addSubgroup.setOnAction(event -> {
menu.hide();
viewModel.addNewSubgroup(group);
viewModel.addNewSubgroup(group, false);
});
MenuItem removeGroupAndSubgroups = new MenuItem(Localization.lang("Remove group and subgroups"));
removeGroupAndSubgroups.setOnAction(event -> viewModel.removeGroupAndSubgroups(group));
MenuItem removeGroupKeepSubgroups = new MenuItem(Localization.lang("Remove group, keep subgroups"));
removeGroupKeepSubgroups.setOnAction(event -> viewModel.removeGroupKeepSubgroups(group));

MenuItem removeSubgroups = new MenuItem(Localization.lang("Remove subgroups"));
removeSubgroups.setOnAction(event -> viewModel.removeSubgroups(group));
removeSubgroups.setOnAction(event -> {
viewModel.removeSubgroups(group);
});

MenuItem addEntries = new MenuItem(Localization.lang("Add selected entries to this group"));
MenuItem sortSubgroups = new MenuItem(Localization.lang("Sort subgroups"));
sortSubgroups.setOnAction(event -> viewModel.sortAlphabeticallyRecursive(group));

MenuItem addEntries = new MenuItem(Localization.lang("Add Selected Entries to this Group"));
addEntries.setOnAction(event -> viewModel.addSelectedEntries(group));
MenuItem removeEntries = new MenuItem(Localization.lang("Remove selected entries from this group"));
removeEntries.setOnAction(event -> viewModel.removeSelectedEntries(group));

MenuItem sortAlphabetically = new MenuItem(Localization.lang("Sort all subgroups (recursively)"));
sortAlphabetically.setOnAction(event -> viewModel.sortAlphabeticallyRecursive(group));
MenuItem removeEntries = new MenuItem(Localization.lang("Remove Selected Entries from this Group"));
removeEntries.setOnAction(event -> viewModel.removeSelectedEntries(group));

menu.getItems().add(editGroup);
removeGroup.getItems().add(removeGroupKeepSubgroups);
removeGroup.getItems().add(removeGroupAndSubgroups);
menu.getItems().add(removeGroup);
menu.getItems().add(new SeparatorMenuItem());
menu.getItems().addAll(addSubgroup, removeSubgroups, removeGroupAndSubgroups, removeGroupKeepSubgroups);
menu.getItems().add(addSubgroup);
menu.getItems().add(removeSubgroups);
menu.getItems().add(sortSubgroups);
menu.getItems().add(new SeparatorMenuItem());
menu.getItems().addAll(addEntries, removeEntries);
menu.getItems().add(new SeparatorMenuItem());
menu.getItems().add(sortAlphabetically);
menu.getItems().add(addEntries);
menu.getItems().add(removeEntries);

// TODO: Disable some actions under certain conditions
// if (group.canBeEdited()) {
Expand Down Expand Up @@ -397,6 +415,21 @@ private void setupClearButtonField(CustomTextField customTextField) {
}
}

private void setNewGroupButtonStyle(TreeTableView<GroupNodeViewModel> groupTree) {
if (groupTree.getRoot() != null) {
if (groupTree.getExpandedItemCount() <= 10) {
addNewGroup.setStyle("-fx-border-width: 0px; -fx-background-color: -jr-theme;" +
" -fx-padding: 0.5em; -fx-text-fill: -jr-white;");
} else {
addNewGroup.setStyle("-fx-border-width: 0px; -fx-background-color: -jr-icon-background-active;" +
" -fx-padding: 0.5em; -fx-text-fill: -jr-black;");
}
} else {
addNewGroup.setStyle("-fx-border-width: 0px; -fx-background-color: -jr-theme;" +
" -fx-padding: 0.5em; -fx-text-fill: -jr-white;");
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Instead of hardcoding the css in the java file, please make this a pseudoclass in css, which should be very easy to implement. See BindingsHelper::includePseudoClassWhen !

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @calixtus , i've checked this BindingsHelper::includePseudoClassWhen needs a parameter of node and the css changes should be applied to the nodes whenever a condition is applied. however, in my change, what I'm changing is the color of Add Group button whenever the user added 10 groups (doesn't include the count of subgroups) and not the nodes. Any advise is greatly appreciated.

Copy link
Member

Choose a reason for hiding this comment

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

The node should be the addNewGroup.
A new Binding to be checked for can be created in some initialize method or the constructor of the class. You only need one Observable which is triggered when change to the list happens.
Please take a look at other occurences of includePseudoClassWhen to get an impression, how to implement this. Again, in theory this should be very straightforward. If you really don't get this to work, I can help a little bit in the next days, im just a bit busy today.


private static class DragExpansionHandler {
private static final long DRAG_TIME_BEFORE_EXPANDING_MS = 1000;
private TreeItem<GroupNodeViewModel> draggedItem;
Expand Down
11 changes: 7 additions & 4 deletions src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ private void onSelectedGroupChanged(ObservableList<GroupNodeViewModel> newValue)
*/
public void addNewGroupToRoot() {
if (currentDatabase.isPresent()) {
addNewSubgroup(rootGroup.get());
addNewSubgroup(rootGroup.get(), true);
} else {
dialogService.showWarningDialogAndWait(Localization.lang("Cannot create group"), Localization.lang("Cannot create group. Please create a library first."));
}
Expand Down Expand Up @@ -147,13 +147,15 @@ private void onActiveDatabaseChanged(Optional<BibDatabaseContext> newDatabase) {
/**
* Opens "New Group Dialog" and add the resulting group to the specified group
*/
public void addNewSubgroup(GroupNodeViewModel parent) {

public void addNewSubgroup(GroupNodeViewModel parent, boolean isGroup) {
calixtus marked this conversation as resolved.
Show resolved Hide resolved
currentDatabase.ifPresent(database -> {
Optional<AbstractGroup> newGroup = dialogService.showCustomDialogAndWait(new GroupDialogView(
dialogService,
database,
preferences,
null));
null,
isGroup));

newGroup.ifPresent(group -> {
parent.addSubgroup(group);
Expand Down Expand Up @@ -184,7 +186,8 @@ public void editGroup(GroupNodeViewModel oldGroup) {
dialogService,
database,
preferences,
oldGroup.getGroupNode().getGroup()));
oldGroup.getGroupNode().getGroup(),
false));

newGroup.ifPresent(group -> {
// TODO: Keep assignments
Expand Down
2 changes: 1 addition & 1 deletion src/main/resources/l10n/JabRef_en.properties
Original file line number Diff line number Diff line change
Expand Up @@ -1782,7 +1782,7 @@ No\ full\ text\ document\ found\ for\ entry\ %0.=No full text document found for
Delete\ Entry=Delete Entry
Next\ library=Next library
Previous\ library=Previous library
add\ group=add group
add\ group=Add group
Copy link
Member

Choose a reason for hiding this comment

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

Translation string must be consistent with english translation.

Suggested change
add\ group=Add group
Add\ group=Add group

Copy link
Contributor

Choose a reason for hiding this comment

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

fixed

Entry\ is\ contained\ in\ the\ following\ groups\:=Entry is contained in the following groups\:
Delete\ entries=Delete entries
Keep\ entries=Keep entries
Expand Down