Fix notifications that overlap task bar

[LIM0000]

Previous discussion for notification system that overlaps task bar: #8769

The new notification system works fine, but I am having the issue that the notifications overlap the task bar on Windows.

Proposal solution: Set owner of notification to Jabref window instead of whole screen

Fixes: #8769

.owner(JabRefGUI.getMainFrame().getScene().getWindow())

PR Checklist:

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

[claell]

@LIM0000 Isn't the task bar (of Windows, not of JabRef) in the screenshot still overlapped?

Maybe there was a misunderstanding, what #8769 is about (not the many notifications that fill the entire window, that is a coincidence, but the notification that does not stay inside the JabRef window, but is overlapping the Windows task bar).

The many notifications filling the entire window might also be of concern, though.

[LIM0000]

Thank you @claell, I definitely misread the issue.
I will look into look a fix for the overlap.

[Siedlerchr]

the NotificationPane comes from ControlsFX, maybe you find the issue somewhere in the code there https://github.com/controlsfx/controlsfx/

[LIM0000]

I have pushed a solution for this issue by setting the owner of notification to Jabref window instead of whole screen.
Please let me know if I should include the notification threshold for this PR as well.
Thanks.

[claell]

What does the notification threshold do exactly?

I assume this one: https://controlsfx.github.io/javadoc/11.1.1/org.controlsfx.controls/org/controlsfx/control/Notifications.html#threshold(int,org.controlsfx.control.Notifications)?

Would be interesting to see the behavior of it in action. Might feel a bit weird if there are first four notifications and if a fifth one comes up, they are all merged into one (if I understand the behavior correctly).

[LIM0000]

@claell
It collapses all the notifications into one if it exceeds threshold limit.
For example, if threshold limit is set to 5, the sixth notification closes the previous 5 notifications immediately and turn into one which is the sixth notification.

[claell]

That is what I thought. Does this offer a good experience? This probably doesn't happen often, so maybe not that important. I am only concerned that collapsing will lose information (in case different types of notifications are collapsed into one where you maybe cannot see the content of the individual ones anymore) or look weird.

[LIM0000]

Thanks for your review @Siedlerchr, they are really valuable to me!
I have applied those changes in the latest commit.
Cheers.

[yogeshvar]

@claell
It collapses all the notifications into one if it exceeds threshold limit.
For example, if threshold limit is set to 5, the sixth notification closes the previous 5 notifications immediately and turn into one which is the sixth notification.

The reason for not using the threshold is that users might tend to miss out on the notification. Unless we have a tab/option to give all the notifications. It would be better not to collapse the notification. @Siedlerchr thoughts on this?

[ThiloteE]

We have the event log. Can be found in the UI via Help > view event log

Maybe collaps, when there are too many popups, notify user somehow that it was collapsed and that the rest of the notifications can be found in the event log?

[claell]

Might be a good way. There also seems to be a method for what to do when the user clicks the notification (which could open the event log in case of a collapsed notification): https://controlsfx.github.io/javadoc/11.1.1/org.controlsfx.controls/org/controlsfx/control/Notifications.html#onAction(javafx.event.EventHandler).

[Siedlerchr]

@LIM0000 I would appreciate it if you could finish this soon with the suggested changes

[LIM0000]

Thank you @Siedlerchr , I have added all required changes in the latest commit.

[Siedlerchr]

LGTM! Thanks for the quick follow up!

[koppor]

I would like to see the last notification. Thus, I modified the text and code to appear as follows:

[ThiloteE]

Another alternative wording: "Check the event log to see all notifications"

I would suggest to keep calling "notifications" and "messages" coherent. Either call them messages or call them notifications. Not both.

On a side note: would it be possible to make this a two liner (or maybe even a one liner)?

e.g.:

Last notification (Check the event log to see all notifications):

Nothing to undo.

[Siedlerchr]

Adjusted based on @ThiloteE suggestiosn

[ThiloteE]

Fair enough.