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

Groups: Union/Intersection: needs re-selection of groups to take effect #6998

Closed
1 of 2 tasks
mlep opened this issue Oct 9, 2020 · 11 comments · Fixed by #8993
Closed
1 of 2 tasks

Groups: Union/Intersection: needs re-selection of groups to take effect #6998

mlep opened this issue Oct 9, 2020 · 11 comments · Fixed by #8993
Labels
bug Confirmed bugs or reports that are very likely to be bugs groups

Comments

@mlep
Copy link
Contributor

mlep commented Oct 9, 2020

JabRef 5.2--2020-10-07--4b1b7b4
Linux 4.9.0-13-amd64 amd64
Java 15

The group pane allows to choose between an intersection and a union when selecting several groups.

  • Groups: Union/Intersection: need restart of groups to take effect

    Issue: When clicking on the Intersection/Union button, an information message is displayed, but the list of entries in the table of entries is not changed. A restart of JabRef is needed for the selected mode to become active. A restart should not be needed.

    Steps to reproduce the behavior:

    1. Open the file jabref-authors.bib (https://github.com/JabRef/jabref/blob/master/src/test/resources/testbib/jabref-authors.bib)
    2. Select the groups "By status" and "By rating"
      Here you should have 1 entry displayed (if you are in intersection mode) or 48 entries displayed (if you are in union mode)
    3. Click once on the Union/Intersection button
      An information message at the bottom of the window confirms the change,
      but the number of displayed entries is still the same.
    4. Close and restart JabRef
    5. Reselect the 2 groups
      If you had one entry displayed, you now get 48, and vice-versa.

    Note 1: when clicking on the union/intersection button, the aspect of the button is not always changed
    Note 2: the hovering message ("Toggle union" or "Toggle intersection") can contradict the information message. E.g. hovering message is "Toggle union", and after the click, the information message is "Group view mode set to intersection".
    Note 3: resetting the preferences did not change the behavior.
    Note 4: In the preferences, the "Groups" tab has two radio buttons: "Display only entries belonging to all selected groups" and "Display all entries belonging to one or more of the selected entries". This seems equivalent to Intersection and Union, respectively. What is their use?

Edit: Meanwhile some progress has been made. What is left to do:

  • Groups: Union/Intersection: needs re-selection of groups to take effect
@Siedlerchr
Copy link
Member

Good observation, guess it's related to the fact
that the selected mode is stored in the preferences and not immediately saved/persisted and reloaded

The information message shows the current mode. The toggle button hover should always show the next mode.
Might be a timing thing related to the preference

@Siedlerchr Siedlerchr added groups bug Confirmed bugs or reports that are very likely to be bugs labels Oct 11, 2020
@mlep
Copy link
Contributor Author

mlep commented Oct 12, 2020

@Siedlerchr In the preferences, do the two radio buttons "Display only entries belonging to all selected groups" and "Display all entries belonging to one or more of the selected entries" could be removed? (what is their use?)

@Siedlerchr
Copy link
Member

I looked ab bit more into this.
The problem is that the groupViewMode is only used in the MainTableDataModel read from the preferences and passed to the matcher in groups.

entriesFiltered = new FilteredList<>(entriesViewModel);
entriesFiltered.predicateProperty().bind(
EasyBind.combine(stateManager.activeGroupProperty(), stateManager.activeSearchQueryProperty(), (groups, query) -> entry -> isMatched(groups, query, entry))
);
IntegerProperty resultSize = new SimpleIntegerProperty();
resultSize.bind(Bindings.size(entriesFiltered));
stateManager.setActiveSearchResultSize(context, resultSize);
// We need to wrap the list since otherwise sorting in the table does not work
entriesSorted = new SortedList<>(entriesFiltered);
groupViewMode = preferencesService.getGroupViewMode();

Here the value is used.

private Optional<MatcherSet> createGroupMatcher(List<GroupTreeNode> selectedGroups) {
if ((selectedGroups == null) || selectedGroups.isEmpty()) {
// No selected group, show all entries
return Optional.empty();
}
final MatcherSet searchRules = MatcherSets.build(groupViewMode == GroupViewMode.INTERSECTION ? MatcherSets.MatcherType.AND : MatcherSets.MatcherType.OR);

A solution would be to add another property to the stateManager, e.g. activeGroupViewModeProperty and bind it to the toggle switch or just set the value here.

private void toggleUnionIntersection() {
GroupViewMode mode = preferences.getGroupViewMode();

This property needs then to be passed to the above listed group matcher

@AEgit
Copy link

AEgit commented Dec 18, 2020

Note, that this issue appears to (indirectly) be related to the problem that there is still not floating mode available for the main table in JabRef 5 (#4237; see also comment here: #4237 (comment)).

Implementing the floating mode might (?) solve some of the groups issues mentioned here by design. Sorry for keeping pointing to that issue, but as long as it is not implemented I cannot switch from JabRef 3.8.2 to the new version 5.

@bemilton
Copy link

bemilton commented Jan 14, 2021

Issue: When clicking on the Intersection/Union button, an information message is displayed, but the list of entries in the table of entries is not changed. A restart of JabRef is needed for the selected mode to become active. A restart should not be needed.

I can replicate this. Looking forward to this bugfix.

JabRef 5.3--2021-01-10--0a971fd
Linux 5.9.13
Java 15.0.1 
JavaFX 15.0.1+1

$ java -version
openjdk version "14.0.2" 2020-07-14
OpenJDK Runtime Environment (build 14.0.2+12)
OpenJDK 64-bit Server VM (build 14.0.2+12, mixed mode)

@calixtus
Copy link
Member

Can maybe solved with the new preferences model we started to implement. See #8047

@Siedlerchr
Copy link
Member

For the thing noted by mlep:
grafik

@HoussemNasri
Copy link
Member

I can't reproduce this in the latest development version. Now, Union/Intersection will take effect after reselecting the groups.

@ThiloteE
Copy link
Member

True.

Closing this :)

@ThiloteE
Copy link
Member

One could make the argument though, that no re-selection of groups should be necessary for the change to take effect. Maybe worthy of being a Koppor issue.

@AEgit
Copy link

AEgit commented Jul 23, 2022

Agree - having to reselect the groups is not intuitive.

@ThiloteE ThiloteE changed the title Groups: Union/Intersection: need restart to take effect Groups: Union/Intersection: needs re-selection of groups to take effect Jul 23, 2022
@HoussemNasri HoussemNasri reopened this Jul 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed bugs or reports that are very likely to be bugs groups
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

7 participants