On Thu, 31 Aug 2023 15:09:05 GMT, Tejesh R <[email protected]> wrote:

>> On `NewFolderAction`, plain String is added `Action.ACTION_COMMAND_KEY`. 
>> Converting the `String `to `locale` before adding as command key fix the 
>> issue. 
>> I have verified the test in all other platforms and Look and Feel which has 
>> option to create New Folder, results were fine. No regressions found on CI 
>> system with the fix. Added manual test to verify the fix.
>
> Tejesh R has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Review fix

Changes requested by aivanov (Reviewer).

test/jdk/javax/swing/JFileChooser/FileChooserNewFolderLocaleTest.java line 36:

> 34:  */
> 35: public class FileChooserNewFolderLocaleTest {
> 36:     static String FRENCH_NEW_FOLDER = "Nouveau dossier";

Suggestion:

    static final String FRENCH_NEW_FOLDER = "Nouveau dossier";

test/jdk/javax/swing/JFileChooser/FileChooserNewFolderLocaleTest.java line 51:

> 49:             newFolderKey = "FileChooser.other.newFolder";
> 50:             ENGLISH_NEW_FOLDER = "NewFolder";
> 51:         }

Suggestion:

        if (IS_WINDOWS) {
            newFolderKey = "FileChooser.win32.newFolder";
        } else {
            newFolderKey = "FileChooser.other.newFolder";
        }
        String englishNewFolder = UIManager.getString(newFolderKey);

`ENGLISH_NEW_FOLDER` is not a constant, it should be declared here. You should 
also use the value from `UIManager`, otherwise the test may fail.

Neither is `IS_WINDOWS` a constant, it should be in camel case: `isWindows`.

-------------

PR Review: https://git.openjdk.org/jdk/pull/15069#pullrequestreview-1606811056
PR Review Comment: https://git.openjdk.org/jdk/pull/15069#discussion_r1312942427
PR Review Comment: https://git.openjdk.org/jdk/pull/15069#discussion_r1312938113

Reply via email to