flow-components icon indicating copy to clipboard operation
flow-components copied to clipboard

`setValue` method in `RadioButtonGroup` incorrectly deselects all options when null is passed

Open Bashooor opened this issue 1 year ago • 9 comments

Description

In the RadioButtonGroup component, the setValue method deselects all options when null is passed as the value. This behavior is not always desirable, as null might represent a valid state that should be selectable rather than deselecting all options.

Expected outcome

When null is passed to the setValue method, the RadioButtonGroup should allow for custom handling of this value, rather than automatically deselecting all options. This would enable null to be represented meaningfully within the selection.

OR:

Make the RadioButton class public so that the method can be overridden with some workaround using inheritance. https://github.com/vaadin/flow-components/issues/1952

Minimal reproducible example

RadioButtonGroup<Boolean> radioButtonGroup = new RadioButtonGroup<>(); radioButtonGroup.setItems(true, false, null); radioButtonGroup.setValue(null);

Steps to reproduce

  1. Create a RadioButtonGroup<Boolean> and add the values true, false, and null (e.g. null for "not specified").
  2. Set the initial value of the RadioButtonGroup to null.
  3. Observe how the selection in the RadioButtonGroup behaves, particularly when null is set as the value.

Environment

Vaadin version(s): 24.4.4 OS: Windows

Browsers

Firefox, Edge

Bashooor avatar Jul 10 '24 01:07 Bashooor

@knoobie May I ask why you gave it a thumbs down? Do you think that it should not be possible to select null in a radio button group?

The only other way I can think of to select no option is to click on the selected option again. However, this is not intuitive to all users, and it is also not visible whether it is possible to select null or not.

leluna avatar Jul 10 '24 07:07 leluna

null / emptyValue has a special meaning for all fields - changing it just for radio buttons is not good practice overall. Addittionally the native type=radio does not support it either, once a value is selected it stays selected - that's common UX for over two decades and native behavior. If you need the possibility to reset the value or frankly saying: you need a default. This is normally done by adding a "Default" option to your RadioButtonGroup so that the user can select it and no, null is not an option.

knoobie avatar Jul 10 '24 08:07 knoobie

Thanks @knoobie for explaining this. I don't have much to add, and IMO we should consider this issue a won't fix. Also, please note that the vaadin-radio-group web component generally expects a string value, not boolean.

web-padawan avatar Jul 10 '24 08:07 web-padawan

I totally agree that unselecting a single radio button is a unintuitive behavior. What is the alternative for Booleans though? Is it to

  1. either not use radio button group at all for Booleans (Whats the alternative?)
  2. or to always convert Booleans zu String (just to convert it with TextRenderer again for internationalisation)

Especially for questionairs, it is very common to use a radio button group to choose between "yes", "no", and "no answer". Always converting to String feels like a dirty hack for me.

leluna avatar Jul 10 '24 09:07 leluna


@Getter
@AllArgsConstructor
enum YesNoNope {
  YES(true), 
  NO(false), 
  NOPE(null);

  private final Boolean value;

}

knoobie avatar Jul 10 '24 10:07 knoobie

Ideally you'll want a properly typed data structure that expresses intent, rather than having a null value that, as seen in this issue, could mean many things. For example an enum where one of the values is "no answer". If your data is dynamic it's a bit more effort, but still doable by having a wrapper class where an instance knows whether it is the "no answer" or not.

sissbruecker avatar Jul 10 '24 10:07 sissbruecker

The use case with "no selection" applies to all kind of data types, not only to Booleans or enums. What you suggest is basically that radio button goup can generally only be used with data types that employ the null object pattern. At least according to my experience, it is often simply not practical to use the pattern. Additionally, the data type is often determined by the domain model. Creating a wrapper type for all types that are used by domain model would be not only tedious, but also difficult to maintain. "A bit more effort" is such an understatement.

leluna avatar Jul 10 '24 12:07 leluna

What I did now as a workaround is to wrap everything in a Optional in a converter for radio button groups, and change all text renders accordingly.... This does not feel like "the intended good way" at all... (Thanks for the idea for that still, having a workaround is better than not having one)

leluna avatar Jul 10 '24 12:07 leluna

For now this is the intended behavior, however it should be documented in the JavaDoc of RadioButtonGroup.setValue.

sissbruecker avatar Jul 11 '24 13:07 sissbruecker