Missing focus states/events for radio buttons in VPN-support dialog

[stephendonner]

Description

Missing focus states/events for radio buttons in VPN-support dialog

Steps to Reproduce

1. install 1.39.86
2. launch Brave
3. set up VPN on development env for the SKUs SDK (lots of separate steps)
4. click on the VPN icon
5. click on the location
6. press tab / shift+tab (reverse) to try to cycle through the radio buttons of the elements

Actual result:

Radio button controls are not in the tabindex and are skipped

Expected result:

Each element and their radio-button control should be tab-focusable/have a focus ring

Reproduces how often:

100%

Brave version (brave://version info)

Brave	1.39.86 Chromium: 101.0.4951.41 (Official Build) beta (x86_64)
Revision	93c720db8323b3ec10d056025ab95c23a31997c9-refs/branch-heads/4951@{#904}
OS	macOS Version 11.6.5 (Build 20G527)

@bsclifton @aguscruiz @nullhook

[GeetaSarvadnya]

++reproduced on window 10 x64

[spylogsster]

works for me, @GeetaSarvadnya @stephendonner could you recheck pls?

[GeetaSarvadnya]

Looks like the issue is fixed and working fine on Windows 10 x64 - Brave version 1.41.74. I have tried both the Tab and shift+tab to cycle through the radio buttons, and I could see the focus ring around the selected radio button.

[stephendonner]

works for me, @GeetaSarvadnya @stephendonner could you recheck pls?

Not working for me, see below.

[stephendonner]

Looks like the issue is fixed and working fine on Windows 10 x64 - Brave version 1.41.74. I have tried both the Tab and shift+tab to cycle through the radio buttons, and I could see the focus ring around the selected radio button.

Thanks for taking a look @GeetaSarvadnya. Even in this video, we can see that the radio buttons themselves are only selectable/tab-able via the mouse. Keyboard operations work but only the country labels get a focus ring.

IMHO (back me up @aguscruiz!) there should be a focus ring on the radio buttons, same as there currently is for the country labels' controls.

[stephendonner]

(The radio buttons clearly have a selected state, but not the focused one I'd expect from tabing.)

[bsclifton]

cc: @fallaciousreasoning in case this was an interesting one for possible Nala cleanup 🙂 Problem is with the keyboard shortcut usage - it's not treating the radio button as linked to the label (like it was one cohesive control).

[fallaciousreasoning]

How are these buttons laid out? Because keyboard controls work nicely in our test page:

Screencast.from.2024-02-26.09-23-01.webm

Is there a chance you're using them inside another label?

<label>
  <LeoRadiobutton/>
  Foo
</label>

The radiobutton already has a label inside it, so correct usage would be:

<LeoRadiobutton>Foo</LeoRadiobutton>

[fallaciousreasoning]

Oh wait, this isn't actually using the Nala radio buttons at all. I could migrate it over, but I'm on Linux so I don't have the VPN. Should be something like this though:

import RadioButton from '@brave/leo/react/radiobutton'

// ...


{regions.map(r => <RadioButton key={r.countryIsoCode} name="region" value={r.countryIsoCode} selectedValue={currentRegion?.countryIsoCode} onChange={e => setSelectedRegion(r)} />

[bsclifton]

That is correct (this is not using Nala... yet). It would be great to add - but that's right, you're on Linux 😂