`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.

KonnorRogers · 2024-10-01T14:25:13Z

I think I finally have a solution for this.

The video is from Web Awesome, but I believe the code should be fairly straight forward to backport and I have like 10+ tests for the new behavior.

I would love to hear if this video of lazy loaded options matches expectations:

https://www.youtube.com/watch?v=PZycQ8Zh7F4

davidcornu · 2024-10-01T15:12:25Z

@KonnorRogers that's exactly the behaviour I would expect 🙏

AlexandreBonaventure · 2024-10-01T15:17:45Z

very nice! I've been tracking this issue as well for some time now because we developed our own combobox solution using sl-select element. This will simplify a bit the logic

KonnorRogers · 2024-10-02T22:04:29Z

#2204

^ PR here. It does have a backwards incompatible change I still need to sort through, but I figured I'd show something so we can double check it meets expectations.

Codepen example:

https://codepen.io/paramagicdev/pen/LYwNPrK?editors=1000

Documentation section:

https://shoelace-git-konnorrogers-add-more-resilien-f32b0c-font-awesome.vercel.app/components/select/#lazy-loading-options

davidcornu · 2024-10-03T13:34:40Z

@KonnorRogers 🙏. I'll find some time to wire up your branch to the sample repo I linked in the issue description and do some testing.

davidcornu · 2024-10-04T15:38:59Z

@KonnorRogers If I'm wiring things up correctly (a bit tricky without a published build), your branch solves the issue as originally reported 🙌 (when loaded with a non-deferred <script> tag, the <sl-option>s for a <sl-select> don't reflect the initial value)

Steps taken

Cloned the shoelace repo alongside https://github.com/davidcornu/shoelace-bug and checked out the konnorrogers/add-more-resilient-lazy-loading-to-select
Ran npm install and npm run build

Applied the following diff to my repo:

diff --git a/app.js b/app.js
index 2d53a22..19ef545 100644
--- a/app.js
+++ b/app.js
@@ -1,4 +1,4 @@
-import "@shoelace-style/shoelace/dist/themes/light.css";
+import "../shoelace/dist/themes/light.css";
 
-import "@shoelace-style/shoelace/dist/components/option/option.js";
-import "@shoelace-style/shoelace/dist/components/select/select.js";
+import "../shoelace/dist/components/option/option.js";
+import "../shoelace/dist/components/select/select.js";
diff --git a/package.json b/package.json
index 97364ef..b1061d9 100644
--- a/package.json
+++ b/package.json
@@ -2,7 +2,6 @@
   "name": "shoelace-bug",
   "version": "1.0.0",
   "dependencies": {
-    "@shoelace-style/shoelace": "^2.8.0",
     "esbuild": "^0.19.3"
   },
   "scripts": {

Ran yarn run dev

KonnorRogers · 2024-10-04T15:53:25Z

when loaded with a non-deferred <script> tag, the s for a don't reflect the initial value

@davidcornu is this a new bug?? im not sure I follow

davidcornu · 2024-10-04T16:18:54Z

is this a new bug?? im not sure I follow

Not a new bug. It's the bug that started this thread 😬 #1570 (comment).

We were bundling Shoelace into a old school <script src="..."> in the <head> of our app and were noticing that <sl-elect> would not reflect their initial value, likely because the inner <sl-option> elements were not yet initialized when the <sl-select> was.

davidcornu · 2024-10-25T15:33:30Z

@KonnorRogers I'm gonna close this one as I consider it fixed as of #2204. Thanks!

brettwillis · 2024-10-30T21:00:55Z

Ok I've getting something similar for the first time now, since v2.18.0.

I have nothing special, no lazy loading just something like this, where initialValue matches the value of one of the options:

        <sl-select .value=${initialValue} >
          <sl-option value="a">Label a</sl-option>
          ...
        </sl-select>

On v2.17.1 it works fine.

On v2.18.0, the sl-select renders empty. If I change .value=${initialValue} to value=${initialValue} i.e. attribute syntax instead of property syntax then it works.

KonnorRogers · 2024-10-30T23:12:10Z

🤔 this is probably because it never trips the valueHasChanged private property. Something about the lifecycle if I had to guess.

Reproduction:

https://codepen.io/paramagicdev/pen/LYwmLYO?editors=1000

Thanks for the report.

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

KonnorRogers mentioned this issue Oct 2, 2024

add more resilient lazy loading to sl-select #2204

Merged

KonnorRogers mentioned this issue Oct 11, 2024

Select initial value shoelace-style/webawesome-alpha#82

Closed

davidcornu closed this as completed Oct 25, 2024

KonnorRogers mentioned this issue Nov 4, 2024

2.18 regression: sl-select does not show selected values on first render #2254

Open

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

Loading

sangeetha-armtek commented Nov 28, 2023 •

edited

Loading

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

Loading

KonnorRogers commented Dec 12, 2023

claviska commented Mar 13, 2024

KonnorRogers commented Mar 13, 2024

KonnorRogers commented Oct 1, 2024

davidcornu commented Oct 1, 2024

AlexandreBonaventure commented Oct 1, 2024

KonnorRogers commented Oct 2, 2024

davidcornu commented Oct 3, 2024

davidcornu commented Oct 4, 2024

KonnorRogers commented Oct 4, 2024

davidcornu commented Oct 4, 2024

davidcornu commented Oct 25, 2024

brettwillis commented Oct 30, 2024

KonnorRogers commented Oct 30, 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 Loading

sangeetha-armtek commented Nov 28, 2023 • edited Loading

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 Loading

KonnorRogers commented Dec 12, 2023

claviska commented Mar 13, 2024

KonnorRogers commented Mar 13, 2024

KonnorRogers commented Oct 1, 2024

davidcornu commented Oct 1, 2024

AlexandreBonaventure commented Oct 1, 2024

KonnorRogers commented Oct 2, 2024

davidcornu commented Oct 3, 2024

davidcornu commented Oct 4, 2024

KonnorRogers commented Oct 4, 2024

davidcornu commented Oct 4, 2024

davidcornu commented Oct 25, 2024

brettwillis commented Oct 30, 2024

KonnorRogers commented Oct 30, 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

Loading

sangeetha-armtek commented Nov 28, 2023 •

edited

Loading

YassSSH commented Dec 12, 2023 •

edited

Loading