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

Quirks with groups panel scrolling when adding groups as subgroups or adding items to groups #9754

Closed
2 tasks done
AEgit opened this issue Apr 10, 2023 · 17 comments · Fixed by #10488 or #10523
Closed
2 tasks done
Labels
bug Confirmed bugs or reports that are very likely to be bugs groups ui
Milestone

Comments

@AEgit
Copy link

AEgit commented Apr 10, 2023

JabRef version

JabRef 5.10--2023-04-05--643319b
Windows 10 10.0 amd64
Java 19.0.2
JavaFX 20+19

Operating system

Windows

Details on version and operating system

Windows 10

Checked with the latest development build

  • I made a backup of my libraries before testing the latest development version.
  • I have tested the latest development version and the problem persists

Steps to reproduce the behaviour

Initial comment here: #2869 (comment)

There are a few quirks, which may or may not be related to the fix for scrolling in the groups panel when adding groups as subgroups (#9728):

  1. Open a JabRef database with a large number of static groups and items (thousands of groups and items)
  2. Add a group to another group as subgroup using drag'n'drop with the newly implemented scrolling feature. To see the scrolling feature in action, you need to add the group to another group that is not already visible in the groups panel (i.e. the group needs to be further distant from the other group in the groups panel).
  3. Note, that the scrolling speed is really high.
    On one hand this is a nice feature for large databases with thousands of groups when you want the child group to be added to a parent group that is very distant in the group table view. On the other hand, it makes it nearly impossible to precisely add a child group to a parent group, since it is very likely that you will whoosh past the parent group due to the scrolling speed. It would be nice if the user could control the scrolling speed by moving the mouse more or less into the direction he wants to put the child group.

Another issue that may or may not be related can be triggered as follows:

  1. Open a JabRef database with a large number of static groups and items (thousands of groups and items)
  2. Add an item to a group using drag'n'drop with the newly implemented scrolling feature. To see the scrolling feature in action, you need to add the item to a group that is not already visible in the groups panel (i.e. the items current position in the groups panel needs to be further distant from the other group in the groups panel).
  3. If you add an article to a group that is far distant (using the new scrolling groups feature) the group table view gets `confused' sometimes and either scrolls to the bottom of the groups panel (after the article has been added) or starts scrolling from top to bottom non-stop. Sometimes it appears to be necessary to have a search term (e.g. a bibtex key) in the search form to trigger this behaviour (at least, that is what it appears to me).
@felipecrp
Copy link

It is also happening when you drag a paper to a group.

@felipecrp
Copy link

A workaround is to rollback and use version 5.9.

@felipecrp
Copy link

I attached a screenshot of the problem.

jabref scroll bug

@Siedlerchr Siedlerchr added the bug Confirmed bugs or reports that are very likely to be bugs label Sep 3, 2023
@tobiasdiez
Copy link
Member

Increasing the priority of this bug, since it renders the group tree almost unusable. I think the problem is the following event handler:

groupTree.setOnDragExited(event -> {
if (event.getY() > 0) {
scrollVelocity = 1.0 / SCROLL_SPEED;
} else {
scrollVelocity = -1.0 / SCROLL_SPEED;
}
scrollTimer.restart();
});

For example, if you leave the treeview sidewards, then it still scrolls down.

Instead of listening to the "onDragExcited" event, I might be better to add two boundary nodes on the top and bottom of the treeview. If the user drags something over them, then it starts scrolling up or down. Can also be used to show a small indicator (if there is a drag & drop in progress).

@tobiasdiez tobiasdiez added this to the 5.11 milestone Oct 12, 2023
@calixtus calixtus mentioned this issue Oct 12, 2023
6 tasks
@calixtus
Copy link
Member

Fixed version should be available here: https://builds.jabref.org/pull/10488/merge
Please try and report if this fixes the issue.

@Siedlerchr
Copy link
Member

Siedlerchr commented Oct 16, 2023

Lefover:

  • Refine scroll speed

@tobiasdiez
Copy link
Member

It's now too slow instead of too fast (at least for smaller group trees). Probably the scroll speed needs to be made proportional to the total number of (expanded) groups.

@AEgit
Copy link
Author

AEgit commented Oct 17, 2023

JabRef 5.11--2023-10-17--8ede31a
Windows 10 10.0 amd64
Java 21-internal
JavaFX 20.0.2+3

As far as I can tell, the issues reported in the original post, have, indeed, been fixed. Well done! There would be an argument, to potentially make the group assignment slightly faster for large group trees, but I think this already works really well and I guess there would be better alternative options to achieve an even higher speed while keeping high precision (e.g., you select the new subgroup you want to move and then scroll to the (parent) group you want to move it to - then after a rightclick on the new parent group a drop-down menu opens where you can assign your currently selected group to the respective parent group).

As for @tobiasdiez observations - if the scroll speed is too low for databases with smaller group trees I would suggest to have a minimum scrolling speed. Basically you keep the current implementation (which, as far as I understand provides a relative scrolling speed, that increases with the size of the group tree), but you set a hard limit on how low the scrolling speed can be. This way users of large group trees would still enjoy the benefits of having a scrolling speed that is high (but not absurdly high, like it was before), and users with a small group tree can be sure, that the scrolling speed will always be fast enough for them, no matter how small their group tree is.

@tobiasdiez
Copy link
Member

minimal absolute speed should work as well, but the implementation overhead is the same (since you need to calculate the number of groups to determine if your speed will be too slow). In principle, you also still have the problem that the scroll speed for really large trees is too high, so you probably need to cap it at some maximum as well.

Just for fun, I read the article The design, use, and performance of edge-scrolling techniques. According to this study, the best results are:

  • Use rate control, in px / s
  • Increase rate linearly with the distance x from the edge (so users can increase the scolling speed by pointing further down)

A good equation would be: speed = 20px/s (1+x).

@AEgit
Copy link
Author

AEgit commented Oct 17, 2023

Cheers, well if there is a peer-reviewed article that found the best solution for scroll speed, then maybe the suggestions of the authors should be followed ^^

Increase rate linearly with the distance x from the edge (so users can increase the scolling speed by pointing further down)

I think it was implemented in a similar way in older JabRef versions.

@koppor koppor mentioned this issue Oct 18, 2023
6 tasks
@koppor
Copy link
Member

koppor commented Oct 21, 2023

Increase rate linearly with the distance x from the edge (so users can increase the scolling speed by pointing further down)
I think it was implemented in a similar way in older JabRef versions.

I could be that the Java technique "Swing" had that build in - but "newer" "JavaFX" technology doesn't 🙈.

@koppor
Copy link
Member

koppor commented Oct 21, 2023

We now have an area of three groups at the top and at the bottom where scrolling starts. The nearer the border the faster it gets. It took some time for me to use that feature. Not sure if this is good.

I am unsure if the authors meant by (1+x) x outside of the area or inside. I remember a tool, where the speedup was when being outside of the window, but I couldn't find it easily.

We implemented it with being inside the Window. Similar to Firefox's functionality.

A build to test should be available in 30 min - linked from #10523 (as last comment - the toolchain moves the comment as soon as build is available. This way, one can (somehow) track on which source code change the build happened)

@AEgit
Copy link
Author

AEgit commented Oct 21, 2023

Cheers, once it is ready I will try to give it a test!

@AEgit
Copy link
Author

AEgit commented Oct 22, 2023

JabRef 5.12--2023-10-22--b1abe02
Windows 10 10.0 amd64
Java 21.0.1
JavaFX 20.0.2+3

I can confirm that the new dev version offers different speeds depending on the mouse position relative to the top 3 and bottom 3 groups in the groups panel. Well done!

Something could be said to add one additional speed level for really large group trees (even with the current highest scrolling speed it still takes a long time to get from one end of the groups tree to the other, if you have thousands of groups). But at the moment at least we have again the functionality that was also present in JabRef 3.8.2 so that is definitely a good sign. And, instead of the additional speed level it might be better to have additional control on groups when using the group filtering: e.g. when the groups filter is applied, groups that have already been selected prior to filtering are not removed but kept in the filtered groups panel view. This way you could quickly add groups from one part of the tree to the other by selecting the relevant group and then filtering for the new parent group.

@koppor
Copy link
Member

koppor commented Oct 22, 2023

@AEgit Thank you for the feedback. Could you create a new issue for "And, instead of the additional speed level it might be better to have additional ..."? This is different from this one. We want to have small, self-contained issues to support students to work on the issue (and thus have "quick wins".

@AEgit
Copy link
Author

AEgit commented Oct 22, 2023

@koppor Done: #10545

Cheers!

@felipecrp
Copy link

I just updated to version 5.11 and I was able to use the sidebar without the bug. Thank you for the fix.

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 ui
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

6 participants