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

[WIP] Add External Application Selection to Preferences #3942

Closed
wants to merge 1 commit into from
Closed

[WIP] Add External Application Selection to Preferences #3942

wants to merge 1 commit into from

Conversation

slyr91
Copy link

@slyr91 slyr91 commented Apr 10, 2018

WIP - Add External Application Selection to Preferences

This pull request is in regards to #674.
This initial commit adds a button to the settings panel for each external application which allows the user to select an application to make default. There is currently a problem with this build where the application will not update the pushToApplicationButton. I believe the issue has something to do with the interaction between javafx and swing components. Also I plan on adding keybinding options as well in the future, it's just taking me a bit of time to familiarize myself with the program between life events. Let me know what you think so far and on the direction I am taking.

Addresses #674
This commit adds a button to the settings panel of each external application to set the currently selected application to be the default application. It is known that pressing the button does not properly update the applications pushToApplicationButton properly.
@slyr91 slyr91 changed the title Add option to set default external application WIP - Add External Application Selection to Preferences Apr 10, 2018
@tobiasdiez
Copy link
Member

This looks like a good start. Nice work!

The button is not updated because such an update mechanism is not yet implemented. The icon is generated from an action (which itself is created in PushToApplicationButton.getMenuAction). However, the Action class only has static return values. I would propose to follow the example of controlsfx Action and replace the getters in the action interface by javafx properties. Then you can use bindings in the adapter class JabRefAction and in this way update the UI if the underlying property is changed.

@slyr91 do you want to implement these changes or prefer if somebody from the team does it? (I would volunteer but it may take some time).

@slyr91
Copy link
Author

slyr91 commented Apr 11, 2018

I will definitely give this a solid try. @tobiasdiez I am still learning about the proper procedures on GitHub but if I can implement those changes should I add them to this pull request or to a separate one?

@tobiasdiez
Copy link
Member

Good question. In general, it is better to create a few smaller PRs because that makes reviewing and understanding the code changes easier. Having said this, in this particular case, I think however that the changes are small and localized enough so that you can simply push them to this PR.

@koppor koppor changed the title WIP - Add External Application Selection to Preferences [WIP] Add External Application Selection to Preferences May 11, 2018
@LinusDietz LinusDietz changed the base branch from maintable-beta to master June 29, 2018 14:01
@stefan-kolb
Copy link
Member

@slyr91 Will you work on this in the future?

@slyr91
Copy link
Author

slyr91 commented Jul 12, 2018

@stefan-kolb I definitely will give it a shot soon. I have just been too busy to dive into this. I had finals to deal with and now I am trying to study for a certification so all my free time goes to that. As soon as I am done with that I will be returning to this.

@slyr91
Copy link
Author

slyr91 commented Aug 30, 2018

I'm getting back into this now and I noticed that the branch this was originally built on, maintainable-beta, is closed. Which branch should I apply these changes and proposed improvements to?

@stefan-kolb
Copy link
Member

Hi, on the master branch please 😄

@Siedlerchr
Copy link
Member

You probably have to rework your code a bit as recently the preferences dialog was transformed to javaFX as well.

@tobiasdiez
Copy link
Member

Surpassed by #5024.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants