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

Changes to FXMLView logging and autoloading of javafx binary css files #4

Closed
wants to merge 2 commits into from

Conversation

koppor
Copy link

@koppor koppor commented Mar 27, 2020

This PR missors AdamBien#68.

I've really loved using afterburner.fx and I use it for several work projects. There were a few small things that were issues for me which I addressed in my own branch, but I wanted to offer them back into the main repository. The first thing was to change the logging messages in afterburner.fx so that they don't log to System.err. We have had bug reports from customers that think something is wrong because it can't find the css files for all views. The other change I made was to allow the loading of binary css files that javafx produces, we have a few large css files and the ability to use the binary format is important for us.
I implemented a set of tests to check the functionality as well and discovered a small bug in the css loading functionality. When getView() is called multiple times, the css sheet is added multiple times as a reference. I added a check to only add the css/bss files when they haven't already been added.

This is my first proper pull request, so I'm aware I might not be following etiquette, please provide feedback on the pull request if appropriate.

In @JabRef, we need a maintained version of afterburner.fx. We did it at https://github.com/JabRef/afterburner.fx. Now we saw activity here and would be happy to contribute our improvements here (and then delete our own fork).

@koppor
Copy link
Author

koppor commented Aug 18, 2020

Much more done at https://github.com/JabRef/afterburner.fx, thus closing this out.

@koppor koppor closed this Aug 18, 2020
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.

2 participants