Added optional Select input binding

[minettiandrea]

First of all thank you very much for the great product. I'm a very happy udash user! And using it extensivly on my open source project https://github.com/Insubric/box

For my projects I'm using quite a lot the native HTML5 form validation, an in general with udash it works smoothly, I just come across a minor limitation on the select element, using required on the select element with the current implementation does not trigger any validation and we have no way to put a null option, but there are use cases where this is needed.
For reference see MDN select required documentation https://developer.mozilla.org/en-US/docs/Web/HTML/Element/select#attr-required.

In order to overcome this limitation my proposition is to add a optional method on io.udash.bindings.inputs.Select with that signature:

def optional[T](
    selectedItem: Property[Option[T]], options: ReadableSeqProperty[T], labelNoValue:Modifier
  )(label: T => Modifier, selectModifiers: Modifier*): InputBinding[Select]

basically is the same of the apply just the selectedItem is optional and we need a label when no value is selected.

The usage is pretty straightforward.

Thanks

Andrea

[ddworak]

Thank you for your kind words, it's always great to see Udash successfully used outside of our org :)

Regarding the PR, I'm not yet convinced that it improves upon the current API so much - I think I may be missing some use case. Let's assume:

val fruitProperty = Property[Option[Fruit]](None)

Then the current Select could be used like:

Select(
  fruitProperty, Seq(Apple, Orange, Banana).map(_.option).prepended(None).toSeqProperty
)(maybeFruit => maybeFruit.mapOr("Label no value", _.name))

and the new, utility one:

Select.optional(
  fruitProperty, Seq(Apple, Orange, Banana).toSeqProperty, labelNoValue = "Label no value"
)(_.name)

Sure, we get to avoid boxing in .option and prepending explicitly, but if the renderer for labels is any more complex, you'll have to call it separately in each arg list. Maybe if you show your example, we can find a nice interface to cover it.

[minettiandrea]

Hi @ddworak ,
thank you for replying

my example it's no more complex from what you wrote, the need of adding the optional method was for the rendered output, with the current implementation you will have something like:

<select>
   <option value="0">Label no value</option>
   <option value="1">Apple</option>
   ...
</select>

but you won't be able to use HTML5 required since "Label no value" has 0 in the value attribute

with my implementation you'll get:

<select>
   <option value="">Label no value</option>
   <option value="0">Apple</option>
   ...
</select>

so if "Label no value" is selected and the element is required the validation fails as expected.

But I agree that we could just change the underlying implementation.
Here (L21:SelectBinding.scala) :

 val el = option(value := idx.toString, label(opt)).render

if opt is None we can set the value as empty string instead the index, but I don't know if that could break things somewhere else, of course the onChange method should be changed accordingly.

If you like that idea I can update the PR with the implementation

[ddworak]

Oh now I see. I think you were right to create a separate method then - it's a different behaviour - we just need more clarity for the user as to what the differences are.

[minettiandrea]

Thank you for reviewing my code.

I did the changes you suggested and added a test case.

[ddworak]

Thank you for these improvements. I've left 2 minor comments and I think we'll be ready to ship after that.

[minettiandrea]

Done, when you think it could be available on maven central?

[ddworak]

LGTM! Thank you for nice contribution :) I'll push the M17 tag right after merge, so it'll be available as soon as the central processes the artifact, usually a couple hours.