shoelace-style / shoelace

A collection of professionally designed, every day UI components built on Web standards. SHOELACE IS BECOMING WEB AWESOME 👇👇👇
https://webawesome.com
MIT License
12.97k stars 835 forks source link

Accessibility issue: sl-radio-group #2191

Open akuu2510 opened 2 months ago

akuu2510 commented 2 months ago

Describe the bug

This bug is in the radio group component where all the sl-radios are disabled and then when I am using tab key from keyboard, disabled radio with value = true gets focussed and after I press up and down arrow keys the radio with value = true is getting un selected.

Expected behaviour

Component should not get focussed when disabled

To Reproduce

Steps to reproduce the behavior:

  1. Go to radio group using sl-radios and then select one of them
<script type="module" src="https://cdn.jsdelivr.net/npm/@shoelace-style/shoelace@2.17.1/cdn/shoelace.js"></script>
<sl-radio-group label="Select an option" name="a" value="1">
  <sl-radio value="1">Option 1</sl-radio>
  <sl-radio value="2">Option 2</sl-radio>
  <sl-radio value="3">Option 3</sl-radio>
</sl-radio-group>
  1. disable all the radio buttons that is all the sl-radios
<script type="module" src="https://cdn.jsdelivr.net/npm/@shoelace-style/shoelace@2.17.1/cdn/shoelace.js"></script>
<sl-radio-group label="Select an option" name="a" value="1">
  <sl-radio value="1" disabled>Option 1</sl-radio>
  <sl-radio value="2" disabled>Option 2</sl-radio>
  <sl-radio value="3" disabled>Option 3</sl-radio>
</sl-radio-group>
  1. Then by using tab try to focus the component you will see it gets focused
  2. Then use up and down arrow key to move you will see that it will throw error and the selected button get disappeared

Demo

https://github.com/user-attachments/assets/03738d1d-0d09-499d-9d12-85d8f94b0844

Screenshots

image

Browser / OS

akuu2510 commented 2 months ago

Probably this can be the fix ADD THE BELOW LINE if (this.getAllRadios().every(radio => radio.disabled)) return;

CODE

private handleKeyDown(event: KeyboardEvent) {
    if (this.getAllRadios().every(radio => radio.disabled)) return;

    if (!['ArrowUp', 'ArrowDown', 'ArrowLeft', 'ArrowRight', ' '].includes(event.key)) {
      return;
    }

    const radios = this.getAllRadios().filter(radio => !radio.disabled);
    const checkedRadio = radios.find(radio => radio.checked) ?? radios[0];
    const incr = event.key === ' ' ? 0 : ['ArrowUp', 'ArrowLeft'].includes(event.key) ? -1 : 1;
    const oldValue = this.value;
    let index = radios.indexOf(checkedRadio) + incr;

    if (index < 0) {
      index = radios.length - 1;
    }

    if (index > radios.length - 1) {
      index = 0;
    }

    this.getAllRadios().forEach(radio => {
      radio.checked = false;

      if (!this.hasButtonGroup) {
        radio.tabIndex = -1;
      }
    });

    this.value = radios[index].value;
    radios[index].checked = true;

    if (!this.hasButtonGroup) {
      radios[index].tabIndex = 0;
      radios[index].focus();
    } else {
      radios[index].shadowRoot!.querySelector('button')!.focus();
    }

    if (this.value !== oldValue) {
      this.emit('sl-change');
      this.emit('sl-input');
    }

    event.preventDefault();
  }

That is when all the sl-radios are disabled then the up and down key board navigation should not work so in code I am returning it if all the radios are disabled @claviska can you please help me in this fix