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

fix: SceneBuilder will not load missing files from recent projects. #585

Merged
merged 44 commits into from
Mar 29, 2024
Merged

fix: SceneBuilder will not load missing files from recent projects. #585

merged 44 commits into from
Mar 29, 2024

Conversation

Oliver-Loeffler
Copy link
Collaborator

@Oliver-Loeffler Oliver-Loeffler commented Oct 10, 2022 •
edited
Loading

Scene Builder will ask to remove missing files from recent files list.
Welcome Page will not close, there will not be an error message.

AskUserToRemoveMissingFile

Issue

Fixes #582

Progress

@Oliver-Loeffler
Copy link
Collaborator Author

Bildschirmaufnahme.2022-10-10.um.23.58.04-1.mov

Window positioning is not yet okay. I am hesitating to touch the AlertDialog for this. Something like positionCentralOver or so could be helpful.

@Oliver-Loeffler Oliver-Loeffler mentioned this pull request Nov 7, 2022
Closed
4 tasks
@abhinayagarwal abhinayagarwal self-requested a review November 8, 2022 03:49
@abhinayagarwal
Copy link
Collaborator

The new tests added to the app project doesn't seem to run.

@Oliver-Loeffler
Copy link
Collaborator Author

I've moved the surefire-plugin from kit/POM.xml to POM.xml so that now tests of app and kit are executed during Maven build.

Oliver-Loeffler and others added 7 commits November 8, 2022 17:19
@Oliver-Loeffler
fix: Scene Builder will ask to remove missing files from list of rece…
1641ee4 
…nt projects. gluonhq#582
@Oliver-Loeffler
fix: incremented version to 20.0.0-SNAPSHOT. (gluonhq#598)
b303f9d 
Co-authored-by: Oliver-Loeffler <[email protected]>
@Oliver-Loeffler
build: Moved maven surefire plugin from kit-pom to parent-pom so that…
484c4dd 
… eventually tests are also picked up for app module.
@Oliver-Loeffler
build: Add Maven Wrapper to the project (gluonhq#588)
4cd7775 
Co-authored-by: Oliver-Loeffler <[email protected]>
@Oliver-Loeffler
fix: Removed FX8 qualifier in BuiltInLibrary (gluonhq#592)
484a1fe 
Co-authored-by: Oliver-Loeffler <[email protected]>
@Oliver-Loeffler
fix: Disabled mnemonic parsing on RecentDocumentsButton instances on …
437dadc 
…welcome page in oder to have correct file name rendering for filenames with underscores. (gluonhq#578)

Co-authored-by: Oliver-Loeffler <[email protected]>
@Oliver-Loeffler
Modified error handling so that WelcomePage is no longer closed after…
e676e1a 
… confirming the error dialog.
@Oliver-Loeffler
Copy link
Collaborator Author

Oliver-Loeffler commented Nov 8, 2022 •
edited
Loading

This PR now additionally addresses issue #600 which describes the behavior of unintended closing of SceneBuilder.

@Oliver-Loeffler
Copy link
Collaborator Author

Hi @abhinayagarwal, I have now corrected the POM for app. Tests are running now. Could you please review this one again?

@Oliver-Loeffler
Copy link
Collaborator Author

@AlmasB Hi Almas, may be you also could have a look on this one!

@Oliver-Loeffler
Added warning and informative logging for cases of removing files fro…
f11a8a1 
…m recent items and for cases where errors occured during FXML loading.
@Oliver-Loeffler
Copy link
Collaborator Author

Oliver-Loeffler commented Jan 3, 2023 •
edited
Loading

Happy new year SceneBuilder!
Hope you'll had a good start into 2023.

@abhinayagarwal, could you please review this one again? The tests are now triggered properly.

@abhinayagarwal
Copy link
Collaborator

A very happy new year to you too!

Hopefully, I can get back to this PR some time this week.

@abhinayagarwal
Copy link
Collaborator

Can you please update the branch with latest changes?

@Oliver-Loeffler
Copy link
Collaborator Author

I've now merged the latest changes into this one. Also applied reformatting according what checkstyle finds for the change files in main.

@abhinayagarwal
Copy link
Collaborator

Testing the changes, it looks good on Mac (including the positioning). However, I find the text on the buttons to be too descriptive for a dialog.

Here is a screenshot from IntelliJ IDEA of a dialog when project is not found:

Screenshot 2023-03-30 at 8 52 34 AM

@Oliver-Loeffler
Copy link
Collaborator Author

Oliver-Loeffler commented Mar 30, 2023 via email

@Oliver-Loeffler
Copy link
Collaborator Author

Oliver-Loeffler commented Mar 19, 2024 •
edited
Loading

With the reworked ErrorDialog it is also now possible to have a useful title for the error detail. Also in SceneBuilderApp the dialog is now called in such way, so we always have a correct modality and one cannot loose a window.

As mentioned before, the error detail is a detail, the dialog requires user action to open.

Actually, it would be quite useful to add some proper log messages here.

ModalityEditor

ModalityWelcome

@Oliver-Loeffler
Copy link
Collaborator Author

If this PR gets approved and merged, I'd like to continue with #576 which then would fix #120.

abhinayagarwal
abhinayagarwal approved these changes Mar 28, 2024
View reviewed changes
Copy link
Collaborator

@abhinayagarwal abhinayagarwal left a comment

Choose a reason for hiding this comment

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

LGTM!

@Oliver-Loeffler
Copy link
Collaborator Author

It took a while but actually I am also now happy with the outcome. Thanks for your feedback.

Oliver-Loeffler
Oliver-Loeffler commented Mar 28, 2024
View reviewed changes
Copy link
Collaborator Author

@Oliver-Loeffler Oliver-Loeffler left a comment

Choose a reason for hiding this comment

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

Thanks for the feedback @abhinayagarwal and @AlmasB. That seems to be okay now. The issues with the error dialog are valid but should be resolved in a separate PR.

app/src/main/resources/com/oracle/javafx/scenebuilder/app/i18n/SceneBuilderApp_ja.properties Show resolved Hide resolved
app/src/main/java/com/oracle/javafx/scenebuilder/app/SceneBuilderApp.java Show resolved Hide resolved
kit/src/main/java/com/oracle/javafx/scenebuilder/kit/editor/panel/util/dialog/ErrorDialog.java Show resolved Hide resolved
@abhinayagarwal abhinayagarwal merged commit 976c754 into gluonhq:master Mar 29, 2024
3 checks passed
@Oliver-Loeffler Oliver-Loeffler deleted the issue-582 branch April 2, 2024 07:42
@Oliver-Loeffler
Copy link
Collaborator Author

This one fixed also #600.

@Oliver-Loeffler Oliver-Loeffler mentioned this pull request Apr 5, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlmasB AlmasB AlmasB approved these changes

@abhinayagarwal abhinayagarwal abhinayagarwal approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
22
Development

Successfully merging this pull request may close these issues.

Scene Builder closes after attempt to open a no longer existing file from welcome page recent projects
3 participants
@Oliver-Loeffler @abhinayagarwal @AlmasB