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

Refactored quitting process #10783

Merged
merged 5 commits into from
Jan 15, 2024
Merged

Refactored quitting process #10783

merged 5 commits into from
Jan 15, 2024

Conversation

calixtus
Copy link
Member

Mandatory checks

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

@calixtus calixtus added status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers type: code-quality Issues related to code or architecture decisions labels Jan 14, 2024
@calixtus calixtus requested a review from Siedlerchr January 14, 2024 19:53
@calixtus
Copy link
Member Author

@Siedlerchr can you test this with mac please?

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 14, 2024 •
edited
Loading

The build for this PR is no longer available. Please visit https://builds.jabref.org/main/ for the latest build.

koppor
koppor reviewed Jan 15, 2024
View reviewed changes
src/main/java/org/jabref/gui/JabRefFrame.java
@@ -337,21 +349,6 @@ private void showTrackingNotification() {
telemetryPreferences.setAskToCollectTelemetry(false);
}

/**
* The MacAdapter calls this method when a "BIB" file has been double-clicked from the Finder.
Copy link
Member

Choose a reason for hiding this comment

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

Don't we have the MacAdapter any more? Oh, I did not find it in the current source. Maybe gone? I only found a recent comment at #10143 (comment).

Copy link
Member

Choose a reason for hiding this comment

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

There's no mac adapter any more it was an old swing thing I think

Copy link
Member Author

Choose a reason for hiding this comment

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

A Solution for this would probably be https://github.com/0x4a616e/NSMenuFX .
I think that the methods removed are of no use for the new approach to fix this with modern mac and javafx, so I removed them. As I have no mac to test this on, I would leave this for someone else with a mac to do.

@Siedlerchr
Copy link
Member

Siedlerchr commented Jan 15, 2024 •
edited
Loading

Cmd + Q works on my mac with closing, position is restored

@koppor koppor enabled auto-merge January 15, 2024 20:55
@koppor koppor added this pull request to the merge queue Jan 15, 2024
Merged via the queue into main with commit 3e7e58a Jan 15, 2024
20 checks passed
@koppor koppor deleted the refactor-frame branch January 15, 2024 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@koppor koppor koppor left review comments

@Siedlerchr Siedlerchr Siedlerchr approved these changes

Assignees
No one assigned
Labels
status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers type: code-quality Issues related to code or architecture decisions
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@calixtus @Siedlerchr @koppor