Refine "File exists" warning#13491
Conversation
koppor
left a comment
There was a problem hiding this comment.
Code looks OK - someone needs to try out.
|
|
||
| Do\ not\ wrap\ when\ saving=Do not wrap when saving | ||
|
|
||
| File\ already\ exists=File already exists |
There was a problem hiding this comment.
Pleae group to line 2999
(there is File '%0' already exists. Overwriting.)
| // Build the dialog window | ||
| Dialog<ButtonType> dialog = new Dialog<>(); | ||
| dialog.setTitle(Localization.lang("File already exists")); | ||
| dialog.setContentText(Localization.lang("File name: \n'%0'", targetFileName)); | ||
| dialog.getDialogPane().getButtonTypes().addAll(Replace, keepBoth, provideAlternative, cancel); |
There was a problem hiding this comment.
I don't think this should be in viewmodel
| ButtonType cancel = new ButtonType(Localization.lang("Cancel"), ButtonBar.ButtonData.OTHER); | ||
|
|
||
| // Build the dialog window | ||
| Dialog<ButtonType> dialog = new Dialog<>(); |
There was a problem hiding this comment.
Please rework. Use org.jabref.gui.JabRefDialogService. Please do not create dialogs on your own. - For example, look at the removed code 😅
MVVM is about testability.
This will also help to test this --> try to craft a test case after you use the JabRefDialogService :)
There was a problem hiding this comment.
I created a custom modal because the current JabRefDialogService methods don’t support tooltips on buttons.
Should I add a new function to JabRefDialogService for this, or is there a better approach you’d recommend?
There was a problem hiding this comment.
Yes please add a new function, maybe with a supplier as parameter for the tool tips
| ButtonType replace = new ButtonType(Localization.lang("Replace"), ButtonBar.ButtonData.OTHER); | ||
| ButtonType keepBoth = new ButtonType(Localization.lang("Keep both"), ButtonBar.ButtonData.OTHER); | ||
| ButtonType provideAlternative = new ButtonType(Localization.lang("Provide alternative file name"), ButtonBar.ButtonData.OTHER); | ||
| ButtonType cancel = new ButtonType(Localization.lang("Cancel"), ButtonBar.ButtonData.OTHER); |
There was a problem hiding this comment.
should be cancel close for button data
|
@trag-bot didn't find any issues in the code! ✅✨ |
Pull Request is not mergeable
| Do\ not\ wrap\ when\ saving=Do not wrap when saving | ||
|
|
||
| File\ already\ exists=File already exists | ||
| File\ name\:\ \n'%0'=File name: \n'%0' |
There was a problem hiding this comment.
There should be no space before newline
Maybe you can submit a follow-up PR?
* upstream/main: Bump com.squareup.okhttp3:okhttp from 5.0.0 to 5.1.0 in /versions (#13541) Fixed Issue 13418: "Search ShortSience" should do a latex-to-unicode conversion (#13532) New Crowdin updates (#13548) Add focus to field Link in the "Add file link" dialog. (#13520) Bump org.ow2.asm:asm from 9.6 to 9.8 in /versions (#13547) Make validation optional for saving library properties preferences (#13488) Bump org.apache.commons:commons-lang3 from 3.17.0 to 3.18.0 in /versions (#13546) --porcelain does not output any info or warn errors on the console (#13469) Bump jablib/src/main/resources/csl-locales from `7e137db` to `3bad433` (#13545) Refine "File exists" warning (#13491) Bump org.openrewrite.recipe:rewrite-recipe-bom from 3.8.0 to 3.11.1 (#13544) Bump org.openrewrite.rewrite from 7.8.0 to 7.11.0 (#13542) Bump jablib/src/main/resources/csl-styles from `704ff9f` to `59f124d` (#13540) Add architecture test for logging infrastructure (#13534) More harsh test on title of PR New Crowdin updates (#13533) Update extra-java-module-info plugin (#13527) [Junie]: fix: display help output for --help argument (#13446) Refactor OpenExternalFileAction (#13508)
fixes #12565 refine file exists warning
Mandatory checks
CHANGELOG.mddescribed in a way that is understandable for the average user (if change is visible to the user)