IgniteUI / igniteui-angular

Ignite UI for Angular is a complete library of Angular-native, Material-based Angular UI components with the fastest grids and charts, Pivot Grid, Dock Manager, Hierarchical Grid, and more.
https://www.infragistics.com/products/ignite-ui-angular
Other
571 stars 161 forks source link

[IgxButtonGroup] Deselection can't be disabled or prevented #13372

Closed gmartinezig closed 1 year ago

gmartinezig commented 1 year ago

Description

It seems like disabling the deselection of items is not possible. One of the usual scenarios for buttongroups is using them as a substitution of a radio button list (but with a different visual pattern). In that case we'd need to have always one item selected. If you click a selected item, the selected item should stay selected, instead of switching back to its normal (not selected) state.

Steps to reproduce

  1. Navigate to https://www.infragistics.com/products/ignite-ui-angular/angular/components/button-group
  2. Click the selected item of the first example (text alignment items)

Result

After step 2, there's no selected item, so how should be the text aligned in that state?

Expected result

We could have a way to prevent the deselection of a selected item by clicking it, probably by setting a property (for example, [allowDeselection]="false", or something like that, I'm just thinking out loud).

I logged it as a bug and not as a feature request, because I consider this a basic feature of a buttongroup component. Please, feel free to move it as a feature request if you this that's better.

Workarounds

I tried some ways to prevent the deselection, but none of them was succeeded.

  1. Deselected event can't be prevented or stopped. I tried to prevent the deselected event (It's not a good approach, because we should prevent it again on every buttongroup instance), besides that, the event prevention is not working due to the way the component is being subscribed (on its internal code) to buttons click events.

  2. Avoiding the use of selected and deselected events doesn't help. When I don't use the selected and deselected events to avoid the deselection and I use the inner buttons click event instead, I found a different issue, the unselected state is visually set by the internal code, by ignoring the current state of the bound model.

ChronosSF commented 1 year ago

To be synced with @damyanpetev ,

Preliminary discussion is leading to the implementation of these two things:

ChronosSF commented 1 year ago

@georgianastasov , as an update, after some additional back and forth we decided it's best to have just the event cancellable and no additional options added. This means we should also fix this for all supported versions.

pmoleri commented 1 year ago

@ChronosSF @damyanpetev

[..] as an update, after some additional back and forth we decided it's best to have just the event cancellable and no additional options added. This means we should also fix this for all supported versions.

Is this the best control ergonomics?

As a user I don't want to cancel something that shouldn't happen in the first place. I can understand this if I'm manually coding a "group of toggle buttons", in that case it's my responsibility to make them act like a group. But here I'm using a Button Group component, at this point them being individual toggles is just an implementation detail.

Same exact issue solved by Material Android 3 years ago using a selectionRequired flag. https://stackoverflow.com/a/59594634/488012

not the prettiest name, not terrible either. Apparently Flutter version doesn't have this issue because it's dumber and the state is set from outside as in React.

pmoleri commented 1 year ago

Also, even if I add additional code to cancel the unwanted event, the UX is not the one expected: image

kdinev commented 1 year ago

Here's my 2-cents. The change should be:

  1. Introduce selectionMode for consistency with the grid and other components
  2. Modes should be single, singleRequired, multi. Default to whatever the default is currently (single, I believe)
  3. Deprecate multiSelection and migrate all references to it to selectionMode="multi".
  4. Last but not necessarily in this order - update the specification
ChronosSF commented 1 year ago

@radomirchev , @kdinev , @teodosiah , @sdimchevski we think this should be included in 16.1 as it comes with new API. We need to prioritize it for this sprint on the Astrea team.

damyanpetev commented 1 year ago

Reopening to keep the old prop as deprecated, see https://github.com/IgniteUI/igniteui-angular/pull/13474#pullrequestreview-1654606547