bug(select): floating label does not float if value is not string

[mgr34]

If <Select> value prop is not a string mdc-web will not add the mdc-floating-label--float-above class and the label will sit atop the text [0]. Select expects its value to be either a string | string[] | number. I believe it should only be string to conform to HTMLSelectElement[1]. This differs from the react select implementation which expands the interface to be string | string[] | number [2].

Specifically this occurs in the foundation handleChange method [3]. To determine whether it should call adapter.floatLabel it calls

const optionHasValue = value.length > 0;

which if it was a number value.length would return undefined and would evaluate to false. Additionally string[] would be misleading. First because the Select component does not support multiple values. Second, because e.target.value will only ever return the first selected value.

Why not file an mdc-web issue?

Mainly because this is the proper behavior of the HTMLSelectElement. the html5 spec indicates that the value of select should only be a string[1].

Proposed solution

Change value to only accept string. Additionally, I believe HTMLSelectElement only allows value on the DOMElement[4] and not the ContentAttributes[5]. React explicitly defines value [2].. Therefore, assuming I understand it correctly, the type signature for value should also be defined in the Props interface for both <Select> and <NativeControl>.

---

I spent too much time I don't have going down this rabbit hole. I don't regret it :)

[0] https://imgur.com/DLCJQFu
[1] https://html.spec.whatwg.org/multipage/form-elements.html#the-select-element
[2] https://github.com/DefinitelyTyped/DefinitelyTyped/blob/bdb87d49e550e600cb30658a9542bfe83337eaf9/types/react/index.d.ts#L2077-L2088
[3] https://github.com/material-components/material-components-web/blob/3cc4c6aa48ae8c1a307df542911e28a7808de41f/packages/mdc-select/foundation.js#L141-L162
[4] https://html.spec.whatwg.org/multipage/dom.html#concept-element-dom
[5] https://html.spec.whatwg.org/multipage/dom.html#concept-element-attributes

[moog16]

@mgr34 this is an interesting issue because it seems like HTMLSelectElement does not match HTML5 spec. According to HTML5 spec is should be a DOMString. Do I understand you correctly? If that's the case, we should probably open an issue against react definitelytyped.

I'm also leaning to agree with you because MDC Web's adapter.getValue method expects type string. So I'm thinking we should do this change.

[mgr34]

Hi @moog16,

I do not think it is an issue in react definitelytyped because they do extend the HTMLSelectElement interface to string | string[] | number. This is a modification made by the react developers and if I am reading their documentation correctly it was to make "it so that <input type="text">, <textarea>, and <select> all work very similarly - they all accept a value attribute that you can use to implement a controlled component."

    interface SelectHTMLAttributes<T> extends HTMLAttributes<T> {
        autoComplete?: string;
        autoFocus?: boolean;
        disabled?: boolean;
        form?: string;
        multiple?: boolean;
        name?: string;
        required?: boolean;
        size?: number;
        value?: string | string[] | number;
        onChange?: ChangeEventHandler<T>;
    }

the HTMLSelectElement should not have a value attribute. Insofar as that HTML5 validation is a thing [0] you will see an HTML5 validator throw an error if <select> is instantiated with a value attribute. This is because value is a DOM Interface [1] and not a content attributes [2]. I'm afraid I cannot clearly explain the difference at the moment. But the content attributes are the attributes you can set on the element.

Anyhow, I can prepare a PR in the next day or so changing value to only accept a string.

---

[0] -- I've spent too much time around XML fanatics blah blah blah. https://validator.w3.org/nu/#textarea
[1] https://html.spec.whatwg.org/multipage/dom.html#concept-element-dom
[2] https://html.spec.whatwg.org/multipage/dom.html#concept-element-attributes

[moog16]

@mgr34 I understand the differences between DomInterface & Attributes. They have the same syntax and look the same, but the browser and other frameworks treat them differently.

Since MDC Web has value strictly as string, I'm fine with changing it.

[mgr34]

Great--I'm glad I do not need to explain further. :) I was having a hard time putting it into words as I was rushing out the door yesterday evening.

I will prepare a PR shortly.

[moog16]

Awesome - thanks for doing all that research. You made a convincing argument.