Feature/add javafx gui #121#286
Conversation
|
|
LimesKey
left a comment
There was a problem hiding this comment.
I'm trying to run it from CMD for Windows, ./gradlew run --args="--gui" does not seem to work, '.' is not recognized as an internal or external command, operable program or batch file.. Only .\gradlew run --args="--gui" works with the backslash.
I see here it says something about Java 19, do you need Java 19 for it to run or does it work with Java 17. Currently Gradle only supports Java 17 so everyone only downloads Java 17, I don't want to have to download Java 19 too. javafx { version = "19"
I am unable to run this and .\gradlew build fails. I'll try to do it again.
Aw sorry, stupid me - ./gradlew is UNIX / Linux syntax, whereas for Windows, it should indeed by .\gradlew or simply gradlew. I'll update the README accordingly.
That's the JavaFX version - running it with Java 17 works perfectly fine (in fact, I'm using OpenJDK 17.0.4 myself)
|
The gradle build command failed with "Entry ... is a duplicate". To circumvent this, we use the exclude strategy for handling duplicates; see https://dzone.com/articles/gradle-goodness-handle-copying .
…3/frankschmitt/DNAnalyzer into feature/add-javafx-gui-#121
Should be fixed now - I've included a strategy for duplicate handling (I'm still not sure where the duplicate comes from, though - the file name mentioned in the error message (reflectionconfig.json) is totally unfamiliar to me. Might be a grade internal thingy) |
|
Seems to get stuck at 80%? The GUI opens and the GUI looks really good so far though. |
|
@frankschmitt could you please clear the merge conflicts? I'm really looking forward to testing the GUI later today! Thank you so much for working on this, I'll give you some feedback on it after some testing. |
# Conflicts: # README.md # build.gradle # src/main/java/DNAnalyzer/CmdArgs.java # src/main/java/DNAnalyzer/DNAAnalysis.java # src/main/java/DNAnalyzer/Properties.java
I've synced the branch with main and resolved the merge conflicts - should be ok now. |
Signed-off-by: Frank Schmitt <frank@qwhon.de>
LimesKey
left a comment
There was a problem hiding this comment.
It looks really good so far, although it is still stuck at 80% in the CLI, the GUI still works and is responsive.
I also get this message in the CLI everytime I click "Analyze"
Exception in thread "JavaFX Application Thread" java.lang.RuntimeException: java.lang.reflect.InvocationTargetException
Is it possible for you to make this the GUI launch from an executable file instead of the CLI? Most users are not comfortable with even touching the CLI for anything, so having this in a exe will make it a lot easier for beginners.
Low priority, but are you also able to incorporate drag and drop functionality where it says "put filename in DNA file, and click analyze"? I'm very impressed though!
|
|
@frankschmitt could you please add the features currently present in the GUI to the README: https://github.com/Verisimilitude11/DNAnalyzer/blob/main/README.md#:~:text=CLI%20GitHub%20repository.-,GUI,-A%20cross%2Dplatform |
|
@frankschmitt, please clear conflicts as well. Thanks for implementing this btw! |
I guess that's how "gradle run" works - it considers the task completed only when the program exits (i.e. when you close the GUI).
That happens if you didn't set the DNA file - you have to enter the file name first.
AFAIK, the usual approach would be to provide a .cmd file for Windows and a .sh file for Linux / UNIX that uses the java executable to launch the application. Do you think that would suffice?
I'll create another issue for this. Implementing Drag & Drop is usually a little bit involved. |
I've updated the README and re-synced with the main branch - all conflicts should be resolved now. |
|
@Verisimilitude11 One more question regarding: Verisimilitude11 removed the[ hacktoberfest-accepted ] Is there a specific reason why you removed the hacktoberfest-accepted label? |
|
Have you seen this? |
Yeah, that's fine - it doesn't really matter.
Sure, could you please add that to your commit? Thanks!
Regarding the hacktoberfest-accepted label, that was only for a PR challenge during the month of October. As October is now over, I removed the label. It has nothing to do with your PR - this is great! |
@LimesKey, I will add separate issues for this - @frankschmitt doesn't need to fix them. |
|
@frankschmitt one thing, did you mark this as //TODO? |
|
I guess it would be helpful if @frankschmitt could take care of adding Javadocs is he gets time; no urgency. In the meantime, I'm going to merge this PR if it looks fine to you @LimesKey. |
|
LGTM @Verisimilitude11 |
Can you please approve the PR? |
Sorry to be a nuisance, but could you please re-add it? Because currently, the PR isn't counted for the Hacktoberfest challenge:
|
I've added a new issues for this - #295. Can you please assign it to me? |
Sure, I added it back, no worries. |
Done. Thanks for working on those, we really appreciate your help with the GUI! |
Implements a very basic GUI for DNAnalyzer (using JavaFX).
Closes #121.
So far, you can
Results are written to a text area in the GUI.