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

fix: Prevents CTRL+P from being masked by a given accelerator defined in FXML (#553) #593

Merged
merged 15 commits into from
Nov 10, 2024

Conversation

Oliver-Loeffler
Copy link
Collaborator

@Oliver-Loeffler Oliver-Loeffler commented Oct 20, 2022

This quick fix will prevent CTRL+P from being blocked by a accelerator defined in an FXML. CTRL+P is used to display the preview window and it works with some FXMLs, not with others.

A clean solution to this requires lot more work. With the current way the Scene Builder works, the problem can occurr with any other accelerator defined in Scene Builder.

Issue

Fixes #553

Progress

… in FXML. Hence allowing to show preview with CTRL+P.
@abhinayagarwal abhinayagarwal added this to the 22 milestone Sep 25, 2023
@Oliver-Loeffler Oliver-Loeffler modified the milestones: 22, 23 Aug 26, 2024
@Oliver-Loeffler Oliver-Loeffler modified the milestones: 23, 24 Sep 27, 2024
@Oliver-Loeffler Oliver-Loeffler added the bug Something isn't working label Oct 1, 2024
@Oliver-Loeffler Oliver-Loeffler self-assigned this Oct 1, 2024
@Oliver-Loeffler Oliver-Loeffler changed the title fix: Prevents CTRL+P from being masked by a given accelerator defined in FXML fix: Prevents CTRL+P from being masked by a given accelerator defined in FXML (#553) Oct 1, 2024
Copy link
Collaborator

@jperedadnr jperedadnr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good. I've tested with an FXML file that has the accelerator on Mac (but it didn't happen) and Windows (failed before this PR, passes after it).

I wonder why the accelerator from the FXML gets installed and somehow takes precedence, but your fix won't hurt in any case.

@Oliver-Loeffler
Copy link
Collaborator Author

I actually have to admit, that this behavior also irritated me. However, it seems to be confusing. I'm updating this branch to see if it still works after all those recent changes.

@Oliver-Loeffler Oliver-Loeffler merged commit ceece31 into gluonhq:master Nov 10, 2024
3 checks passed
@Oliver-Loeffler Oliver-Loeffler deleted the issue-553 branch November 10, 2024 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Version 18.0.0 bug.
3 participants