fix: ensure aria-selected attribute is removed from the item clone

[web-padawan]

Description

• Updated selected property in ItemMixin to use sync: true so it triggers observer synchronously
• Added call to performUpdate() on the item clone in vaadin-select to remove aria-selected
• Added missing unit test to ensure aria-selected attribute is removed (in Lit version it was not)

Note: there was an error in the "keyboard selection" test apparently caused by the fact that "append value item" was called too late. Added await nextUpdate() to this test helped to remove the error so I think it was a test issue.

        at SelectItem.update (node_modules/lit-element/src/lit-element.ts:163:23)
        at SelectItem.performUpdate (node_modules/@lit/reactive-element/src/reactive-element.ts:1440:13)
        at Select.__appendValueItemElement (packages/select/src/vaadin-select-base-mixin.js:467:21)
        at Select.__attachSelectedItem (packages/select/src/vaadin-select-base-mixin.js:430:12)

Type of change

• Bugfix

[vursen]

Issue: The aria-selected attribute is still not removed when the select is populated with items that have a label attribute.

<vaadin-select></vaadin-select>

<script type="module">
  import '@vaadin/select';
  import '@vaadin/list-box';
  import '@vaadin/item';
  import { render, html } from 'lit';

  const select = document.querySelector('vaadin-select');
  select.renderer = (root) => {
    render(
      html`
        <vaadin-list-box>
          <vaadin-item label="Label 0" value="value-0">Option 0</vaadin-item>
          <vaadin-item label="Label 1" value="value-1">Option 1</vaadin-item>
        </vaadin-list-box>
      `,
      root,
    );
  };
</script>

UPD: This issue is actually reproducible with both Lit and Polymer.

[web-padawan]

This issue is actually reproducible with both Lit and Polymer.

Good catch, this seems to be caused by setting selected to true after attaching the item which is done to prevent resetting selected to false when setting select value to an item that is also disabled: vaadin/vaadin-select#106

Note, we most probably don't need selected attribute on the clone since the original issue mentions style adjustments related to padding vaadin/vaadin-select#103 - in V14 they are defined here, but in V22 we use different selector:

web-components/packages/select/src/vaadin-select-value-button-styles.js

Lines 21 to 25 in 6999a73

	::slotted(*) {
	padding-left: 0;
	padding-right: 0;
	flex: auto;
	}

So IMO can remove selected from the item clone. I'd prefer to change that in a separate PR though.

[vursen]

So IMO can remove selected from the item clone. I'd prefer to change that in a separate PR though.

I'm just wondering if both issues could be resolved by ensuring the selected property is false before attaching the element. This should make it unnecessary to manually remove the aria-selected attribute and consequently enforce a sync update.

[web-padawan]

Could both issues be resolved by ensuring the selected property is false before attaching the element?

Makes sense, thanks. Updated and renamed the PR accordingly.

[web-padawan]

UPD: unfortunately we still would need to remove aria-selected attribute with selected set to false.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[vaadin-bot]

This ticket/PR has been released with Vaadin 24.6.0.