Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

onChange in Select does not work with Laminar #36

Closed
pme123 opened this issue Dec 24, 2022 · 5 comments
Closed

onChange in Select does not work with Laminar #36

pme123 opened this issue Dec 24, 2022 · 5 comments

Comments

@pme123
Copy link

pme123 commented Dec 24, 2022

I try to use Laminar's Var with the Select component.

But it does not work. I tried different things (I found in similar components - there is none with Select)

As soon I add onChange it stops working - with the last two versions I can select the option as long as the option is after the selected option. If the last is selected - you can't even change it anymore.

val allowedSteps =
  List("my/path/to/dmnConfigs", "another/path", "yet/another/path")
    .map(p => Select.option(value := p, p))
val selectedPathVar: Var[String] = Var("another/path")

      Select(
        value <-- selectedPathVar.signal,          
       allowedSteps,
     //      _.events.onChange.mapToValue --> selectedPathVar,
    // _ => onChange.mapToValue --> (x =>  dom.window.alert(s"onChange_ ${x}")),
      //  onChange.mapToValue.map{x => println(x); x} --> selectedPathVar,
      inContext { thisNode =>
          onChange.map(x => {
            println(s"${thisNode.ref.selectedOption.textContent}")
            thisNode.ref.selectedOption.textContent
          }) --> selectedPathVar
        } , /*
        _.events.onChange.map { x =>
          println(x.detail.selectedOption.textContent);
          x.detail.selectedOption.textContent
        } --> selectedPathVar.writer*/
      )

Do I miss something?

@sherpal
Copy link
Owner

sherpal commented Dec 24, 2022

Yes, these Select are a bit counterintuitive regarding the native html <select>.
They don't work with a value on the Select itself. Rather, they work with each option having a selected attribute.

So, if you want to keep it in sync with some Observable, each option basically has to do

_.selected <-- myObservable.map(v => amISelected(v))

The amISelected depends on how you identify your options (for example with their value).

Does that help?

@raquo
Copy link
Contributor

raquo commented Dec 25, 2022

Ah so value <-- selectedPathVar.signal does not work because the Select web component simply doesn't have a value property, unlike the native DOM <select> element. Yeah, usually web component libraries try to match the native DOM props / events, but sometimes they have quirks like this.

@sherpal I see that SAP UI5 Select has selectedItemId and selectedItemKey properties – maybe they're just using a different name for what would normally be the value property? If so, worth adding to the interface.

Also, tangential PSA – avoid using value in any kind of select-s (whether DOM-native or SAP) if the list of options changes over time – much more robust to use option's selected property in that case. See raquo/Laminar#94

@pme123
Copy link
Author

pme123 commented Dec 25, 2022

@sherpal
yes, thanks that works! This is my code now that works:

def ConfigurationPathsComp(
    basePath: String,
    selectedPathVar: Var[String],
    configuredPathsVar: Var[List[String]]
) =

  val configuredPaths =
    configuredPathsVar.signal.map(
      _.map(p =>
        Select.option(
          value := p,
          p,
          _.selected <-- selectedPathVar.signal.map(p == _)
        )
      )
    )
 Select(
          className := "configPathsSelect",
          children <-- configuredPaths,

          inContext { thisNode =>
            onChange.map(x => {
              println(s"${thisNode.ref.selectedOption.textContent}")
              thisNode.ref.selectedOption.textContent
            }) --> selectedPathVar
          } ,
        )

Let me know if there are better ways - as there are quite a few;).

@sherpal
Copy link
Owner

sherpal commented Dec 25, 2022

Indeed, they simply don't have a property value. They do have a selectedOption, but it's readonly (as reflected to be a def in the bindings), and they return the full html ui5-option element -- which I think is nice, because then we have all info, even custom data-stuff).
But from the doc, they don't have anything else.

@pme123 Your code is mostly ok, but rather than use the onChange within the inContext, you can use _.events.onChange.map(_.detail.selectedOption.textContent), it's there precisely to avoid it :D

@sherpal sherpal closed this as completed Dec 25, 2022
@sherpal
Copy link
Owner

sherpal commented Dec 30, 2022

I added an example in the demo to stress this use case

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants