Re: RFR: 8299522: JFilechooser open button size is incorrectly shown [v2]

[Tejesh R]

On Tue, 10 Jan 2023 05:18:52 GMT, Tejesh R <t...@openjdk.org> wrote:

>> FileChooser Open/Approve button size is shown incorrectly when no Approve 
>> button text is set in `CUSTOM_DIALOG` type. Reason being that no default 
>> Approve Button text is returned during Dialog Type Property change. Since 
>> `null` is returned as Button string the Button size is incorrectly shown. 
>> The fix here addresses the issue by adding a default Approve Button Text 
>> when no text is set explicitly in case of `CUSTOM_DIALOG` type.
>> Automated test is attached which has been tested with multiple test runs.
>
> Tejesh R has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Updated based on review comments

> > > I suggest handling the missing text for the Approve button if a custom 
> > > dialog is used with adding a `JFileChooser` instance as component by 
> > > falling back to the `Open` type.
> > 
> > 
> > I didn't get this point ? Can you please tell me what does "falling back to 
> > the 'Open' type means?
> 
> This code
> 
> ```java
> new JFileChooser().showDialog(null, null);
> ```
> 
> produces the dialog that looks like: <img alt="JFileChooser custom dialog 
> falls back to Open dialog" width="662" 
> src="https://user-images.githubusercontent.com/70774172/211583355-6fed7d24-fff2-4c4b-9887-0bdd75f98930.png";>
> 
> It looks exactly the same as the one created by
> 
> ```java
> new JFileChooser().showOpenDialog(null);
> ```
> 
> The above code works well in the three L&F that I tested, the custom dialog 
> behaves as if it's an open dialog and the Approve button has the text "Open".
> 
> Thus, the issue exists only if you use 
> `setDialogType(JFileChooser.CUSTOM_DIALOG);` then add the instance of 
> `JFileChooser` to a container such as `JFrame`.
> 
> What I meant is that you can use `openButtonText` for the Approve button if 
> it wasn't set explicitly without creating additional properties and members. 
> This can be achieved as simple as
> 
> ```diff
> diff --git 
> a/src/java.desktop/share/classes/javax/swing/plaf/basic/BasicFileChooserUI.java
>  
> b/src/java.desktop/share/classes/javax/swing/plaf/basic/BasicFileChooserUI.java
> index c307a138114..bf214389bda 100644
> --- 
> a/src/java.desktop/share/classes/javax/swing/plaf/basic/BasicFileChooserUI.java
> +++ 
> b/src/java.desktop/share/classes/javax/swing/plaf/basic/BasicFileChooserUI.java
> @@ -922,7 +922,8 @@ public class BasicFileChooserUI extends FileChooserUI {
>          String buttonText = fc.getApproveButtonText();
>          if (buttonText != null) {
>              return buttonText;
> -        } else if (fc.getDialogType() == JFileChooser.OPEN_DIALOG) {
> +        } else if (fc.getDialogType() == JFileChooser.OPEN_DIALOG
> +                   || fc.getDialogType() == JFileChooser.CUSTOM_DIALOG) {
>              return openButtonText;
>          } else if (fc.getDialogType() == JFileChooser.SAVE_DIALOG) {
>              return saveButtonText;
> ```

Yeah, initially had thought of doing this, but to keep the default text open 
for future changes and to maintain the default for CUSTOM Type separately I 
added additional properties and members. If using the OPEN type properties is 
fine for CUSTOM also then can use the same I guess.

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

PR: https://git.openjdk.org/jdk/pull/11901