sghpjuikit / player

Audio player and management application.
22 stars 2 forks source link

Guide not closeable when Settings are opened before it #58

Closed xeruf closed 6 years ago

xeruf commented 6 years ago

Whenever I open the Settings window and then the Guide, be it at application start or while running the application, the Guide cannot be closed unless I close the Settings. Focusing it, pressing ESC doesn't work.

Even if we fix this I propose that every window should have a close button.

sghpjuikit commented 6 years ago

All windows have close buttons, you are referring to popups. I must admit JavaFX has multiple issues related to popup and focus, which may be whats going on as well. Anyway, the only popups that do not have close button are those that close themselves automatically when user clicks elsewhere. The lack of a close button is actually a feature intending to give away this behavior.

I think this was also reproduced on Mac OS.

sghpjuikit commented 6 years ago

There was a bug causing close button to not appear sometimes (will be fixed in next commit), This should make it possible to close the guide, but I can still reproduce the issue of ESCAPE not having an effect when multiple popups are open.

sghpjuikit commented 6 years ago

The ESCAPE issue is caused by TreeView control consuming ESCAPE event. I could not find a way to solve this so far - this affects all TreeView's in entire application and first it must be determined why this is happening (rather than try to bypass the mechanism).

There is yet another issue. JavaFX popup windows are unfocusable and somehow the events they receive are also passed into their parent window. This has been causing problems for me for a long time - simply put, user has no way of knowing which event will be received by the popup and which by the owning window - this should be determined by whether the popup is focused, but there is no such mechanism in place. I managed to make it so the popup always receives the events and never its parent, but this effectively turns modality on for every popup, idea which I'm not fond of. So I'm trying to improve on this, so far with difficulty.

sghpjuikit commented 6 years ago

I implemented workaround for ESCAPE consumption in TreeView. After making a small demo application and investigating internal JavaFX code, it is clear that TreeView consumes ESCAPE by default. This is supposed to cancel tree item editing, however the developers overlooked the fact that consuming event only makes sense when the action is actually invoked. When the TreeView is not editable or none of its items is being edited, no such consuming of ESCAPE should take place. This can not be sufficiently fixed, as the code in question is located in internal com.sun packages.

For now, I created an extension function that bypasses this behavior when no editing is active.

The last issue I mentioned previously is unrelated, thus I'm closing this issue as fixed.