-
Notifications
You must be signed in to change notification settings - Fork 220
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
feat: Enabled loading of FXML files via drag&drop to WelcomeDialog #708
Conversation
3a27bd7
to
ff1e253
Compare
@@ -511,6 +511,9 @@ welcome.recent.items.loading = Loading recent projects... | |||
welcome.recent.items.no.recent.items = no recent projects | |||
welcome.open.project.label = Open Project | |||
welcome.loading.label = Loading Components... | |||
welcome.loading.when.dropped.error.title=Unsupported file format or empty directory | |||
welcome.loading.when.dropped.error.message=The dropped object is either not a JavaFX FXML file or does not contain any FXML files to be loaded. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would keep it simple, similar to that of alert.open.failureMofN.details
:
Unable to load dropped file(s). Make sure file(s) are valid FXML documents.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds better to me. I'll rework this accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've reworked the window. The MofN
approach does not seem suitable to me due to the usage pattern as described below. It looks now as follows:
The text stops after 5 items. If one wants to see all details, one must click on Show details
. The message itself is now somewhat simpler.
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I would not show the directory path here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its now reworked accordingly.
if (inDir.isEmpty()) { | ||
unsupportedItems.add(file.getAbsolutePath()); | ||
} else { | ||
toOpen.addAll(inDir); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In case we add multiple items to SB and one of them is valid, it won't show the dialog box for others. Is this the desired behaviour?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. I'll look again into this and will come up with an alternative proposal. As of now it seems not to be the desired behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Loading FXML files is expected behavior. If Scene Builder fails to open an FXML, there will be a message. When trying to open anything else I would not expect any reaction. I would actually continue silently. May be log the attempt to load anything else than just a FXML file or a collection of FXML files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer the previous implementation. If we want to be silent let's be silent when there is a valid file :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. so I'll revert and rethink. I was playing with different (Windows based) apps yesterday and the Drag&Drop implementation was not consistent. Some complained invalid inputs, some not. Some others pretended that basically everything was valid (e.g. Ms Paint).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its now reverted and I added the conceptual explanation as Javadoc. There is more detailed logging for the case.
SceneBuilder will silently ignore unsupported files and empty directories as long as there is at least one FXML file to be opened. For individual FXML files, the actual error handling happens later. Here the files to be opened are not tested or validated.
The idea is, that if a user picks one file intentionally, a concise feedback in case of the wrong file format or an empty directory can be helpful and is desired.
But when selecting a whole directory or multiple files, it might be helpful to ignore unsupported files as long actual candidates to be opened exist. The unsupported ones can result from an inprecise selection. One example could be that the user knowingly picked a resources folder with FXMLs, which also includes other files such as properties or icons. In that case the user would get annoyed about a message.
Another case is a quick selection e. g. in Gnome Nautilus or Windows Explorer where some other files are unintentionally selected. As before, an error message could be annoying as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. Can this PR be moved to "Ready to review" now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its in review now.
…eneBuilder. SceneBuilder will only notify when it cannot open a given FXML file.
… into SceneBuilder. SceneBuilder will only notify when it cannot open a given FXML file." This reverts commit 8706347.
53f5975
to
d1320b0
Compare
…supported) contents at all as been dropped to welcome page.
…ome page shows the details now only in the actual details view. The dialog itself only contains the hint what went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This feature PR introduces drag&drop to the welcome dialog or welcome page. One now can select a group of files and directories and drag them over the welcome page. On drop, the files are filtered to be FXML files and dropped subdirectories are searched for FXML files in first level.
All FXML files found will be passed to the file open process of Scene Builder.
For unsupported files or empty directories no message will be displayed as long as at least one file can be loaded.
If no file can be loaded at all, a message is displayed to the user that there were either unsupported files and/or the selected directories did not contain any supported file. The welcome page remains open.
Issue
Fixes #584
Progress