`sl-select` does not honor its `value` when included in the response HTML #1570

davidcornu · 2023-09-19T18:17:55Z

Describe the bug

When a sl-select element is included in a page's HTML (as opposed to added afterwards via JS) with a value attribute and several nested sl-options, it does not render the initial value.

To Reproduce

I've created a minimal reproduction over at https://github.com/davidcornu/shoelace-bug, which renders the exact same sl-select twice: once in the page's HTML and once by appending it to the DOM once the sl-option is defined.

You'll notice the first sl-select does not show One as the initial value, whereas the second one does.

Browser / OS

OS: macOS 13.5.2 (22G91)
Browsers:
- Firefox 117.0.1
- Safari 16.6 (18615.3.12.11.2)

Additional information

After some time digging around in the debugger, I believe the source of the issue is that when selectionChanged first runs, this.getAllOptions returns [] as the various <sl-option> elements have not been initialized yet:

shoelace/src/components/select/select.component.ts

Line 554 in 2ed5a4f

this.selectedOptions = this.getAllOptions().filter(el => el.selected);

This leads it to clear value

shoelace/src/components/select/select.component.ts

Line 567 in 2ed5a4f

this.value = this.selectedOptions[0]?.value ?? '';

which means that as its <sl-option>s are connected it can't establish which option is selected.

The text was updated successfully, but these errors were encountered:

claviska · 2023-09-19T20:35:48Z

Thank for the report. I can confirm that the value is not set when Shoelace is bundled and loaded as a script in the <head> of the document. Adding type="module" defers loading and works around the bug.

@KonnorRogers Seems like a lifecycle issue. Since we almost always load Shoelace as a module, we've never run into it. It would be worth checking other components for similar bugs. I wonder if we can create a test for this somehow. 🤔

davidcornu · 2023-09-20T00:54:35Z

Thank for the report. I can confirm that the value is not set when Shoelace is bundled and loaded as a script in the <head> of the document. Adding type="module" defers loading and works around the bug.

Oh that's good to know. I should be able to split the Shoelace bits into a separate deferred bundle.

Seems like a lifecycle issue. Since we almost always load Shoelace as a module, we've never run into it. It would be worth checking other components for similar bugs. I wonder if we can create a test for this somehow. 🤔

While looking into this I also tested <sl-radio-group>/<sl-radio> as that's another case of a parent element depending on a child that's also a custom element. It does render the initial value as expected but also throws an error due to an unchecked radios[0] access (this is from memory – I don't recall the exact line unfortunately).

KonnorRogers · 2023-09-21T14:10:41Z

@claviska definitely seems to be a life cycle issue. We could either emit a sl-select event when the option is set to selected and have it bubble up to the <sl-select>, the other option is perhaps to have a mutation observer attached to the items slotted into <sl-select> and listen for when the selected attribute changes.

EDIT: As for tests, this is a little more difficult because it requires a separate file due to how WTR loads every test file in the same window. But the test itself would be relatively straightforward.

EDIT 2: This can actually be reproduced in a regular file if you append an <sl-option> to an <sl-select> after it finishes rendering.

    // Passes
    it('Should set option to selected when options are lazily loaded', async () => {
      const select = await fixture<SlSelect>(html`
        <sl-select value="option-1">
          <sl-option value="option-1">Option 1</sl-option>
          <sl-option value="option-2">Option 2</sl-option>
          <sl-option value="option-3">Option 3</sl-option>
        </sl-select>
      `)

      await select.updateComplete

      expect((select.querySelector("[value='option-1']") as SlOption).selected).to.be.true
    })

   // Fails
   it('Should set option to selected when options are lazily loaded', async () => {
      const select = await fixture<SlSelect>(html`
        <sl-select value="option-1">
        </sl-select>
      `)

      select.innerHTML = `
        <sl-option value="option-1">Option 1</sl-option>
        <sl-option value="option-2">Option 2</sl-option>
        <sl-option value="option-3">Option 3</sl-option>
      `
      await select.updateComplete

      expect((select.querySelector("[value='option-1']") as SlOption).selected).to.be.true
    })

## Description Finally we're here: you can just set `language="es"` on the calculator element and it'll be in Spanish. Getting the strings out of the XLIFF file and into code requires running `lit-localize build`. Rather than making people do that manually, I wrote a Parcel resolver that runs the command behind the scenes. Overengineered? Maybe! But I learned a bunch about Parcel! Note about switching `language` dynamically. Apart from the problem I noted in the comment on the `language` attribute (text that came back from the API will not change language until the next API fetch), there's another problem, with the Shoelace select elements: the text shown in them won't change until you make a new selection in the element. I _think_ it may be related to [this](shoelace-style/shoelace#1570); in any case, once all the immediate i18n work is wrapped up I may try to isolate the issue and file an issue with them if it's not the same bug. With this bug, dynamically setting the attribute _on page load_ will leave a couple of untranslated strings, and I think that's a use case we do want to support. ## Test Plan Add a `language="es"` attribute to the main element in `rhode-island.html`, and make sure the UI shows up in Spanish. Query for incentives; make sure the program names of federal incentives show up in Spanish. (Those are the only thing localized on the backend right now.)

## Description Finally we're here: you can just set `lang="es"` on the calculator element (or any ancestor element!) and it'll be in Spanish. Getting the strings out of the XLIFF file and into code requires running `lit-localize build`. Rather than making people do that manually, I wrote a Parcel resolver that runs the command behind the scenes. Overengineered? Maybe! But I learned a bunch about Parcel! Note about switching `lang` dynamically. Apart from the problem I noted in the comment on the `lang` attribute (text that came back from the API will not change language until the next API fetch), there's another problem, with the Shoelace select elements: the text shown in them won't change until you make a new selection in the element. I _think_ it may be related to [this](shoelace-style/shoelace#1570); in any case, once all the immediate i18n work is wrapped up I may try to isolate the issue and file an issue with them if it's not the same bug. With this bug, dynamically setting the attribute _on page load_ will leave a couple of untranslated strings, and I think that's a use case we do want to support. ## Test Plan Add a `lang="es"` attribute to the main element in `rhode-island.html`, and make sure the UI shows up in Spanish. Query for incentives; make sure the program names of federal incentives show up in Spanish. (Those are the only thing localized on the backend right now.)

sangeetha-armtek · 2023-11-28T16:51:27Z

Any update on this? Stuck with the same issue.
Or is there a work around for the time being?

claviska · 2023-11-28T16:55:21Z

No update yet. Contributions are welcome, otherwise we'll try to get to it as soon as we can!

KonnorRogers · 2023-12-01T19:30:22Z

What's interesting is that the native <select> seems to have the same problem with lazy loaded options.

https://codepen.io/paramagicdev/pen/JjxmzxW

if when it initially connects, there are no <option>, when the <option>elements load, it selects the first one

KonnorRogers · 2023-12-08T17:29:16Z

So, as I've had time to sit with this, I think the best course of action would be to not change the value of the select until the user has interacted with the select. Basically, wait until a user has manually changed the value of the <sl-select> until we start updating value, to allow for lazy loaded options, but also not change the value on a user while they're using the control.

@claviska to me this feels like it has the least amount of "surprise" to it

davidcornu · 2023-12-08T18:42:29Z

I think the best course of action would be to not change the value of the select until the user has interacted with the select.

Would that impact form submissions (i.e. I submit the <form> without clicking on the <sl-select>)?

KonnorRogers · 2023-12-08T19:45:19Z

@davidcornu in what way?

So today if you do something like this:

<sl-select value="doesnt-exist"></sl-select>

its "value" will become "" if that's what youre asking.

I think if we track the "initial value attribute", and only update the value when the lazy loaded option comes in, then it may be able to work. There's definitely some kinks to work out.

YassSSH · 2023-12-12T15:06:23Z

Hi there! 👋

I've updated the documentation for multiple selection in Shoelace. Specifically, I've replaced the string-based 'value' prop with an array in the example code to accurately reflect the recommended usage for multiple selection.

import SlOption from '@shoelace-style/shoelace/dist/react/option';
import SlSelect from '@shoelace-style/shoelace/dist/react/select';

const App = () => (
  <SlSelect label="Select a Few" value={["option-1", "option-2", "option-3"]} multiple clearable>
    <SlOption value="option-1">Option 1</SlOption>
    <SlOption value="option-2">Option 2</SlOption>
    <SlOption value="option-3">Option 3</SlOption>
    {/* Additional options here */}
  </SlSelect>
);

I've also created a pull request with these changes. You can find it here

KonnorRogers · 2023-12-12T16:08:09Z

Any update on this? Stuck with the same issue. Or is there a work around for the time being?

Nothing great I can think of beyond manually updating value after the <sl-option> connects to the DOM

claviska · 2024-03-13T17:02:35Z

Fixed in #1785

KonnorRogers · 2024-03-13T17:05:49Z

Reopening, as the docs update doesn't really address lazy loaded options.

davidcornu added the bug Things that aren't working right in the library. label Sep 19, 2023

claviska assigned KonnorRogers Oct 26, 2023

oyamauchi mentioned this issue Nov 14, 2023

Use Spanish translations in the UI rewiringamerica/embed.rewiringamerica.org#66

Merged

claviska closed this as completed Mar 13, 2024

KonnorRogers reopened this Mar 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`sl-select` does not honor its `value` when included in the response HTML #1570

`sl-select` does not honor its `value` when included in the response HTML #1570

davidcornu commented Sep 19, 2023

claviska commented Sep 19, 2023

davidcornu commented Sep 20, 2023

KonnorRogers commented Sep 21, 2023 •

edited

sangeetha-armtek commented Nov 28, 2023 •

edited

claviska commented Nov 28, 2023

KonnorRogers commented Dec 1, 2023

KonnorRogers commented Dec 8, 2023

davidcornu commented Dec 8, 2023

KonnorRogers commented Dec 8, 2023

YassSSH commented Dec 12, 2023 •

edited

KonnorRogers commented Dec 12, 2023

claviska commented Mar 13, 2024

KonnorRogers commented Mar 13, 2024

sl-select does not honor its value when included in the response HTML #1570

sl-select does not honor its value when included in the response HTML #1570

Comments

davidcornu commented Sep 19, 2023

Describe the bug

To Reproduce

Browser / OS

Additional information

claviska commented Sep 19, 2023

davidcornu commented Sep 20, 2023

KonnorRogers commented Sep 21, 2023 • edited

sangeetha-armtek commented Nov 28, 2023 • edited

claviska commented Nov 28, 2023

KonnorRogers commented Dec 1, 2023

KonnorRogers commented Dec 8, 2023

davidcornu commented Dec 8, 2023

KonnorRogers commented Dec 8, 2023

YassSSH commented Dec 12, 2023 • edited

KonnorRogers commented Dec 12, 2023

claviska commented Mar 13, 2024

KonnorRogers commented Mar 13, 2024

`sl-select` does not honor its `value` when included in the response HTML #1570

`sl-select` does not honor its `value` when included in the response HTML #1570

KonnorRogers commented Sep 21, 2023 •

edited

sangeetha-armtek commented Nov 28, 2023 •

edited

YassSSH commented Dec 12, 2023 •

edited