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

Tejesh R tr at openjdk.org
Tue Jan 10 17:32:59 UTC 2023


On Tue, 10 Jan 2023 05:18:52 GMT, Tejesh R <tr at 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



More information about the client-libs-dev mailing list