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.

Sign up for GitHub

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

Jump to bottom

Avoid hiding the background tasks status indicator on smaller window sizes #8781

Closed
claell opened this issue May 10, 2022 · 15 comments · Fixed by Beingmani/jabref#1
Closed

Avoid hiding the background tasks status indicator on smaller window sizes #8781

claell opened this issue May 10, 2022 · 15 comments · Fixed by Beingmani/jabref#1
Labels
type: enhancement ui

Comments

@claell
Copy link
Contributor

claell commented May 10, 2022

Is your suggestion for improvement related to a problem? Please describe.
On smaller window sizes, the status indicator (and button) for background tasks is hidden. I think such an indicator should never be hidden.

Describe the solution you'd like
Possibly change its location or never hide it at its current place.

Additional context
grafik
@ThiloteE
Copy link
Member

Related: #8295

@ThiloteE
Copy link
Member

ThiloteE commented May 10, 2022 •
edited
Loading

Could potentially be solved by implementing #8774 and moving the other buttons slightly to the left. I think, there is some space between the search bar and the other buttons.

@claell
Copy link
Contributor Author

claell commented May 10, 2022

Agreed. Probably the responsive design behavior can be improved.

Just wanted to keep the issue description neutral to the solution (moving the button might help to improve discoverability and highlighting the special nature of it, for example).

@Beingmani Beingmani mentioned this issue May 11, 2022
Closed
@0CoolMichael
Copy link

This question is quite interesting and I will try to do it in this moment. After modifying the position of the button, I will submit a pull request.

@ThiloteE
Copy link
Member

Of course you can. You might want to coordinate and communicate with @Beingmani so as to not create any merge conflicts. He wants to work on #8774, which is very closely related.

@0CoolMichael 0CoolMichael mentioned this issue May 15, 2022
Closed
6 tasks
@Beingmani
Copy link

Yes, i do. I already worked on it and it should fix the issue. Fixing #8774 will fix this.

Will raise a PR by tomorrow :)

@Beingmani Beingmani mentioned this issue May 16, 2022
Merged
6 tasks
@Beingmani
Copy link

Hey Guys,

Solution :

Moving the Background task button to the front will resolve the issues on smaller screens and since it's responsive, it will be the first one that is visible even on smaller screens.

Here is a picture of the screen where the solution is implemented. I'm trying to get rid of the space but this solution works for now I guess. let me know if I can draft a PR for this.

image

@claell
Copy link
Contributor Author

claell commented May 17, 2022

@Beingmani I read that you wanted to add a PR by yesterday. Are there current problems occurring that you need help with? Also, is that screenshot a mockup or an already implemented solution?

Additionally, please have a look at #8795, which is related. Maybe, you can work on that basis.

@claell
Copy link
Contributor Author

claell commented May 17, 2022

Just saw that you are working on https://github.com/Beingmani/jabref/commits/main, so adding for reference.

@Beingmani
Copy link

Hi @claell , The screenshot is from my local jabref where I made code changes.

#8795 - Regarding this, I can see that there is space that is left intentionally for text (i.e n entries found) .

Moving the Background task button to front should actually fix these issues.

@Beingmani Beingmani mentioned this issue May 17, 2022
Closed
6 tasks
@calixtus
Copy link
Member

As I wrote in the other PR, I don't think the the backgroundtask-indicator is important enough to be displayed right next to the search bar.
The space is there because there is a hidden label that (as i remember) displays the number of hits of a search query. The space can probably removed if no search query is entered (managedProperty).

@claell
Copy link
Contributor Author

claell commented May 20, 2022

Removing the space (and adding it again after a query) will likely lead to "jumping" icons, though. I think that is why it has not been done before.

@ThiloteE
Copy link
Member

ThiloteE commented May 20, 2022 •
edited
Loading

on 1366x768 dpi

grafik

  • removing a tiny bit of the white space would allow to at least show one more button. Instead of the arrow at the right, maybe the github button would then fit there too.

  • I also can trigger an Error pressing the button background task status indicator. It completely crashes JabRef and I will have to do a force close. Unfortunately this is not just a problem concerning usability and beauty of the UI.

    • Here the error message:

      JabRef 5.7--2022-05-19--4baf2a1
      Linux 5.4.0-110-generic amd64
      Java 18.0.1
      JavaFX unknown

            2022-05-20 12:59:19 [JavaFX Application Thread] org.jabref.gui.FallbackExceptionHandler.uncaughtException()
ERROR: Uncaught exception occurred in Thread[JavaFX Application Thread,5,main]: java.lang.NullPointerException: Cannot invoke "com.sun.glass.ui.Window.isTransparentWindow()" because the return value of "com.sun.javafx.tk.quantum.WindowStage.getPlatformWindow()" is null
    at [email protected]/com.sun.javafx.tk.quantum.GlassScene.getClearColor(Unknown Source)
    at [email protected]/com.sun.javafx.tk.quantum.SceneState.update(Unknown Source)
    at [email protected]/com.sun.javafx.tk.quantum.GlassScene.updateSceneState(Unknown Source)
    at [email protected]/com.sun.javafx.tk.quantum.GlassScene.releaseSynchronization(Unknown Source)
    at [email protected]/javafx.scene.Scene$ScenePulseListener.pulse(Unknown Source)
    at [email protected]/com.sun.javafx.tk.Toolkit.lambda$runPulse$2(Unknown Source)
    at java.base/java.security.AccessController.doPrivileged(Unknown Source)
    at [email protected]/com.sun.javafx.tk.Toolkit.runPulse(Unknown Source)
    at [email protected]/com.sun.javafx.tk.Toolkit.firePulse(Unknown Source)
    at [email protected]/com.sun.javafx.tk.quantum.QuantumToolkit.pulse(Unknown Source)
    at [email protected]/com.sun.javafx.tk.quantum.QuantumToolkit.pulse(Unknown Source)
    at [email protected]/com.sun.javafx.tk.quantum.QuantumToolkit.pulseFromQueue(Unknown Source)
    at [email protected]/com.sun.javafx.tk.quantum.QuantumToolkit.lambda$runToolkit$11(Unknown Source)
    at [email protected]/com.sun.glass.ui.InvokeLaterDispatcher$Future.run(Unknown Source)
    at [email protected]/com.sun.glass.ui.gtk.GtkApplication._runLoop(Native Method)
    at [email protected]/com.sun.glass.ui.gtk.GtkApplication.lambda$runLoop$11(Unknown Source)
    at java.base/java.lang.Thread.run(Unknown Source)
2022-05-20 12:59:19 [JavaFX Application Thread] org.jabref.gui.FallbackExceptionHandler.uncaughtException()
ERROR: Uncaught exception occurred in Thread[JavaFX Application Thread,5,main]: java.lang.IllegalStateException: The window has already been closed
    at [email protected]/com.sun.glass.ui.Window.checkNotClosed(Unknown Source)
    at [email protected]/com.sun.glass.ui.Window.getNativeWindow(Unknown Source)
    at [email protected]/com.sun.glass.ui.gtk.GtkWindow.getNativeWindow(Unknown Source)
    at [email protected]/com.sun.prism.PresentableState.update(Unknown Source)
    at [email protected]/com.sun.javafx.tk.quantum.SceneState.update(Unknown Source)
    at [email protected]/com.sun.javafx.tk.quantum.GlassScene.updateSceneState(Unknown Source)
    at [email protected]/com.sun.javafx.tk.quantum.WindowStage.lambda$setScene$1(Unknown Source)
    at [email protected]/com.sun.javafx.tk.quantum.QuantumToolkit.runWithRenderLock(Unknown Source)
    at [email protected]/com.sun.javafx.tk.quantum.WindowStage.setScene(Unknown Source)
    at [email protected]/javafx.stage.Window$12.invalidated(Unknown Source)
    at [email protected]/javafx.beans.property.BooleanPropertyBase.markInvalid(Unknown Source)
    at [email protected]/javafx.beans.property.BooleanPropertyBase.set(Unknown Source)
    at [email protected]/javafx.stage.Window.setShowing(Unknown Source)
    at [email protected]/javafx.stage.Window.hide(Unknown Source)
    at [email protected]/javafx.stage.PopupWindow.hide(Unknown Source)
    at [email protected]/org.controlsfx.control.PopOver.lambda$hide$5(Unknown Source)
    at [email protected]/javafx.animation.Animation.runHandler(Unknown Source)
    at [email protected]/javafx.animation.Animation.finished(Unknown Source)
    at [email protected]/javafx.animation.AnimationAccessorImpl.finished(Unknown Source)
    at [email protected]/com.sun.scenario.animation.shared.SingleLoopClipEnvelope.timePulse(Unknown Source)
    at [email protected]/javafx.animation.Animation.doTimePulse(Unknown Source)
    at [email protected]/javafx.animation.Animation$1.lambda$timePulse$0(Unknown Source)
    at java.base/java.security.AccessController.doPrivileged(Unknown Source)
    at [email protected]/javafx.animation.Animation$1.timePulse(Unknown Source)
    at [email protected]/com.sun.scenario.animation.AbstractPrimaryTimer.timePulseImpl(Unknown Source)
    at [email protected]/com.sun.scenario.animation.AbstractPrimaryTimer$MainLoop.run(Unknown Source)
    at [email protected]/com.sun.javafx.tk.quantum.QuantumToolkit.pulse(Unknown Source)
    at [email protected]/com.sun.javafx.tk.quantum.QuantumToolkit.pulse(Unknown Source)
    at [email protected]/com.sun.javafx.tk.quantum.QuantumToolkit.pulseFromQueue(Unknown Source)
    at [email protected]/com.sun.javafx.tk.quantum.QuantumToolkit.lambda$runToolkit$11(Unknown Source)
    at [email protected]/com.sun.glass.ui.InvokeLaterDispatcher$Future.run(Unknown Source)
    at [email protected]/com.sun.glass.ui.gtk.GtkApplication.enterNestedEventLoopImpl(Native Method)
    at [email protected]/com.sun.glass.ui.gtk.GtkApplication._enterNestedEventLoop(Unknown Source)
    at [email protected]/com.sun.glass.ui.Application.enterNestedEventLoop(Unknown Source)
    at [email protected]/com.sun.glass.ui.EventLoop.enter(Unknown Source)
    at [email protected]/com.sun.javafx.tk.quantum.QuantumToolkit.enterNestedEventLoop(Unknown Source)
    at [email protected]/javafx.stage.Stage.showAndWait(Unknown Source)
    at [email protected]/javafx.scene.control.HeavyweightDialog.showAndWait(Unknown Source)
    at [email protected]/javafx.scene.control.Dialog.showAndWait(Unknown Source)
    at [email protected]/org.jabref.gui.JabRefDialogService.showErrorDialogAndWait(Unknown Source)
    at [email protected]/org.jabref.gui.FallbackExceptionHandler.lambda$uncaughtException$0(Unknown Source)
    at [email protected]/com.sun.javafx.application.PlatformImpl.lambda$runLater$10(Unknown Source)
    at java.base/java.security.AccessController.doPrivileged(Unknown Source)
    at [email protected]/com.sun.javafx.application.PlatformImpl.lambda$runLater$11(Unknown Source)
    at [email protected]/com.sun.glass.ui.InvokeLaterDispatcher$Future.run(Unknown Source)
    at [email protected]/com.sun.glass.ui.gtk.GtkApplication._runLoop(Native Method)
    at [email protected]/com.sun.glass.ui.gtk.GtkApplication.lambda$runLoop$11(Unknown Source)
    at java.base/java.lang.Thread.run(Unknown Source)
2022-05-20 12:59:19 [JavaFX Application Thread] org.jabref.gui.FallbackExceptionHandler.uncaughtException()
ERROR: Uncaught exception occurred in Thread[JavaFX Application Thread,5,main]: java.lang.IllegalStateException: The window has already been closed
    at [email protected]/com.sun.glass.ui.Window.checkNotClosed(Unknown Source)
    at [email protected]/com.sun.glass.ui.Window.getNativeWindow(Unknown Source)
    at [email protected]/com.sun.glass.ui.gtk.GtkWindow.getNativeWindow(Unknown Source)
    at [email protected]/com.sun.prism.PresentableState.update(Unknown Source)
    at [email protected]/com.sun.javafx.tk.quantum.SceneState.update(Unknown Source)
    at [email protected]/com.sun.javafx.tk.quantum.GlassScene.updateSceneState(Unknown Source)
    at [email protected]/com.sun.javafx.tk.quantum.GlassScene.releaseSynchronization(Unknown Source)
    at [email protected]/javafx.scene.Scene$ScenePulseListener.pulse(Unknown Source)
    at [email protected]/com.sun.javafx.tk.Toolkit.lambda$runPulse$2(Unknown Source)
    at java.base/java.security.AccessController.doPrivileged(Unknown Source)
    at [email protected]/com.sun.javafx.tk.Toolkit.runPulse(Unknown Source)
    at [email protected]/com.sun.javafx.tk.Toolkit.firePulse(Unknown Source)
    at [email protected]/com.sun.javafx.tk.quantum.QuantumToolkit.pulse(Unknown Source)
    at [email protected]/com.sun.javafx.tk.quantum.QuantumToolkit.pulse(Unknown Source)
    at [email protected]/com.sun.javafx.tk.quantum.QuantumToolkit.pulseFromQueue(Unknown Source)
    at [email protected]/com.sun.javafx.tk.quantum.QuantumToolkit.lambda$runToolkit$11(Unknown Source)
    at [email protected]/com.sun.glass.ui.InvokeLaterDispatcher$Future.run(Unknown Source)
    at [email protected]/com.sun.glass.ui.gtk.GtkApplication.enterNestedEventLoopImpl(Native Method)
    at [email protected]/com.sun.glass.ui.gtk.GtkApplication._enterNestedEventLoop(Unknown Source)
    at [email protected]/com.sun.glass.ui.Application.enterNestedEventLoop(Unknown Source)
    at [email protected]/com.sun.glass.ui.EventLoop.enter(Unknown Source)
    at [email protected]/com.sun.javafx.tk.quantum.QuantumToolkit.enterNestedEventLoop(Unknown Source)
    at [email protected]/javafx.stage.Stage.showAndWait(Unknown Source)
    at [email protected]/javafx.scene.control.HeavyweightDialog.showAndWait(Unknown Source)
    at [email protected]/javafx.scene.control.Dialog.showAndWait(Unknown Source)
    at [email protected]/org.jabref.gui.JabRefDialogService.showErrorDialogAndWait(Unknown Source)
    at [email protected]/org.jabref.gui.FallbackExceptionHandler.lambda$uncaughtException$0(Unknown Source)
    at [email protected]/com.sun.javafx.application.PlatformImpl.lambda$runLater$10(Unknown Source)
    at java.base/java.security.AccessController.doPrivileged(Unknown Source)
    at [email protected]/com.sun.javafx.application.PlatformImpl.lambda$runLater$11(Unknown Source)
    at [email protected]/com.sun.glass.ui.InvokeLaterDispatcher$Future.run(Unknown Source)
    at [email protected]/com.sun.glass.ui.gtk.GtkApplication._runLoop(Native Method)
    at [email protected]/com.sun.glass.ui.gtk.GtkApplication.lambda$runLoop$11(Unknown Source)
    at java.base/java.lang.Thread.run(Unknown Source)

    • I think this is a javafx bug though that has to do with the popup. I think I have seen that issue before, but would need to search first.

To summarize: Would really be good to always display "background task status indicator" even on small screens, because that would likely be a workaround to triggering the error. Edit: Yes I also agree It should not be displayed prominently on the left. In my opinion, the following buttons are very important too:

grafik
grafik

Edit2: Maybe one could enable disable groups of buttons via view? Just like one can hide the groups panel or websearch. But maybe this is overkill.

@claell
Copy link
Contributor Author

claell commented May 20, 2022

That spaces are currently set on purpose:

Here you can see where the width is set for the left spacer.

jabref/src/main/java/org/jabref/gui/JabRefFrame.java

Line 541 in 6a2332f

leftSpacer.setPrefWidth(50);

I think, that this can be done more responsive (like on Firefox for example). The space around the search and address bar dynamically changes depending on the window width.

@koppor
Copy link
Member

koppor commented May 23, 2022

In the devcall, we decided to move forward with the less intrusive solution of #8798. That solution works in smaller window sizes equal to 1280x960 or bigger than that.

@koppor koppor closed this as completed May 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
type: enhancement ui
Projects
Features & Enhancements
Archived in project
JabRef UI Improvements
Archived in project
Milestone
No milestone
6 participants
@koppor @claell @Beingmani @calixtus @0CoolMichael @ThiloteE