[Textfield] Allow label be a component

[Kuirak]

I'd like to use some other components (e.g. FormattedText from react-intl) for the label on a text field.

If i set the id to a value it works with the current code: <Textfield label={<FormattedText id="MyLabel"/>} id="foo" />

Maybe it could be changed that customId is generate by something like lodash uniqueId and the propTypes allow components as label.

[tleunen]

The problem with a automatically generated id is that you cannot really guarantee that you id will be the same between the server rendering and the client rendering... That's why it was based on the label.

For the fact to allow custom component for the label, I don't have anything against it, except I'm not a big fan if this syntax. It doesn't seem very "React". Maybe I'm wrong. One other thing could be to have labelComponent prop. It would give something like this:

<Textfield labelComponent={FormattedText} label="MyLabel" id="foo" />

Might be a bit better. I should ask some other React devs to see what would be the best.

[Kuirak]

Actually I like this syntax with passing a fully valid component as prop. It is basically the same syntax you use for the <Header title={<Icon name="logo"/>} />.

It helps knowing where the passed component will be used in the actual component.

In my opinion either label or labelComponent should be set but not both.

[tleunen]

I'll go with the component inside label ;)

[Chun-Yang]

Any one is working on this? I can give it a shot.

My use case is for Checkbox. I need the label to contain links.

[tleunen]

Not that I'm aware of ;-)

[leifoolsen]

Is the generated id correct?

When I inspect the first textfield component in the examples page I see the following output.

<div class="mdl-textfield mdl-js-textfield is-upgraded" data-upgraded=",MaterialTextfield" style="width: 200px;">
<input class="mdl-textfield__input" id="textfield-Text" name="">
<label class="mdl-textfield__label" for="textfield-Text">Text...</label>
</div>

The third textfield it has the same id-value. Would i not be better to generate a random id, e.g. id="textfield-Text-someuniquerandomcharsequence"?

For screenreaders to work properly it is required to correlate label and input with a unique id.

[tleunen]

Auto generating things is not that great because of server side rendering, you would need to have the same id on server than client to prevent a re-render.

I agree using the label as "id" is not great, but you can pass your own id to the Textfield and this id will be used instead. I wanted a way to easily create a textfield without forcing to specify an id. But this might change in v2.

[leifoolsen]

If the only purpose of an id is correlating label and input it may be desirable to automate this task. Maintenance of identifiers can quickly become messy, especially when the input fields are dynamically generated.

[tleunen]

Agreed. but how would you generate the id in order to have the same generated one between multiple render (server/client)?

[leifoolsen]

My knowledge of React is, for now, relatively limited. We does not use server side rendering. Inside the render() method I use this this to generate a random id:

const randomString = ( n=12 ) => Array( n+1 ).join((`${Math.random().toString(36)}00000000000000000`).slice(2, 18)).slice(0, n);

const id = 'textfield-'+randomString();

[tleunen]

You can have a pretty good random string using just Math.random().toString(36).substr(2).

But the problem with this is that you won't get the same id when rendering multiple times, so this is an issue. No everybody uses server rendering but this is still something to keep in mind while building a library...

[leifoolsen]

As i mentioned, the only purpose of the id is to correlate label and input for screen readers. If the id varies between page loads does not matter, as long as the label for= correlates with the input id=. For ids we need to track we do not use a random generated id.

[leifoolsen]

... btw. Thanks for a great lib :+1:

[tleunen]

I agree with you for the purpose of them, but at the same time we have to find a way to not trigger a second render when going from server side render to client side render ;)

[leifoolsen]

I do not have much knowledge about serverside rendering. But if the id and correlating for= is set by the server, then the client side maybe could check if the id already exists?

[leifoolsen]

Regarding the generated id: Wold it not be better to NOT generate an id? The absence of an id does not affect MDL. As it is now, we'll potentially end up with several duplicate id values in a page.

[tleunen]

An id is required to have a full a11y coverage.

[leifoolsen]

I know. But if no id is given in markup, then I think it is better to leave it out. Having (potentially) duplicate id-values will not validate either :)

... this is not really a big problem, as you would normally set identifier in markup.