This activeItemIsNotFirstItem() check will only trigger once in the toggle group's lifecycle, as it will have a non-null value in subsequent renders.

I'm not sure I understand the logic here — if the current value is null, why would we call setValue if the active item is not the first item?

The logic in trunk was that, whenever an option is focused (i.e. becomes the active item), set that value in the Ariakit store explicitly (because we're managing this component in controlled mode). We want to prevent this behavior when there is no value selected yet and the active item is the first item. Once the active item is the second item, that means the user not only "focused on the ToggleGroupControl" but also actively selected an option, so we are safe to start setting values. This is how native radios behave.

Hope that made better sense, but also I'd like your input on how to improve the code comment so it is easier to understand.

Is the toggleGroupControlContext.activeItemIsNotFirstItem check necessary, though? Does it matter which item is the active item?

Yes. See for example the default example for Ariakit Radio. When the focus first enters the radio group, the active item is the first item ("apple"). It's only when the active item becomes something else ("orange") that we want to start setting values.

Once we start setting values (i.e. the value is non-null), then it no longer matters which is the active item.

Got it. I wish we could rely on the built-in behavior of the ariakit Radio component, without the need for us to re-implement the logic (I guess we can't since we're not using native input type="radio" elements?)

Also, if we even want to tweak the initial position of the active element before a selection is made, this solution won't cut it - but it's unlikely, so that doesn't seem like a big issue

Right, I experimented a bit, and we can't rely on the built-in behavior if we use button. I think it is ultimately possible to rewrite it so we use input type="radio", but that would be a larger refactor with possibly breaking changes (not sure).

Sounds good — we may want to consider a v2 with such changes when we have the new design spec

We used to check if the event was defaultPrevented or not - is this something we want to keep, and if not, why?

I can't see why we would need it, especially now that we're using onFocusVisible instead of onFocus. @ciampo Do you remember why you initially prevented the default (#50278)?

I can't find a specific reason, I believe it was just to respect a consumer wanting to preventDefault. I don't think we need it, as you say, since we're not using onFocus anymore

Thanks for confirming, folks 👍

Right, I experimented a bit, and we can't rely on the built-in behavior if we use button. I think it is ultimately possible to rewrite it so we use input type="radio", but that would be a larger refactor with possibly breaking changes (not sure).

I can't see why we would need it, especially now that we're using onFocusVisible instead of onFocus. @ciampo Do you remember why you initially prevented the default (#50278)?