Add inputAs prop to TextField to customize how the underlying input is rendered

[justinanastos]

This adds an inputAs prop that accepts either a string that includes keyof JSX.IntrinsicElements, which means any normal DOM element like input or button. You can also pass a React element labelAs={<input className="border-red" />}

[jgzuke]

Q: Does this let you pass a custom component or only built in DOM elements and component instances? Something like as=Link from engine (components/local-link not react routing Link)?

[justinanastos]

Q: Does this let you pass a custom component or only built in DOM elements and component instances? Something like as=Link from engine (components/local-link not react routing Link)?

@jgzuke Correct. You can't pass as={Link}. I thought I described that in the docs and the TypeScript types show that too. Can you suggest a better way to communicate this?

[justinanastos]

/rebase

[jgzuke]

@justinanastos I think its fine. Maybe calling out specifically that it wont work might be worthwhile, instead of just not including it in the list of things that do work. I was more just confirming though, since I thought one of the original reasons for as were so we could use Links as buttons more easily

[evans]

Excited about this landing! I have a question and a suggestion that I think will make the code more readable. Leg's get this shipped!

[evans]

Curious why this PR touches onBlur and what otherProps used to do?

[justinanastos]

I tried to sneak this in because I thought it would go through fast. I'm going to pull it into another patch PR. I explicitly don't want to accept otherProps because, this being a layout component, it's not obvious where those props will be spread to.

[justinanastos]

Now I remember why I did it this way; these two work together because there are formatting changes that will cause merge conflicts and I wanted to avoid that.

[evans]

I read the api documentation for cloneElement and createElement to understand what is happening here. From what I understand, cloneElement does a shallow copy of the props by default, so we need to merge the classNames explicitly. Will this have unintended consequences or semantics around class precedence that would require documentation?

The other case uses createElement without merging class names, since there are no props for inputAs when it is a string, such as div or input. It took me about 10 minutes to read through the React docs and understand it. I suggest you add a comment describing the cases here and why the prop merging is different, so hopefully that we don't need to context switch out of the source to understand the behavior

[justinanastos]

Will this have unintended consequences or semantics around class precedence that would require documentation

Great question. No, the order of the class names in class="a b c" has no effect on precedence. Precedence is determined solely by specificity, which has clearly defined rules in the css spec.

[justinanastos]

I suggest you add a comment describing the cases here and why the prop merging is different, so hopefully that we don't need to context switch out of the source to understand the behavior

I very much value the suggestion and am happy that you read through the docs before bringing up the talking point.

First, understanding the difference between an element and component is critical. A component is the definition of something you will render, either a React.FC (functional component) or a class that extends React.Component. An element is the result of a render by either of two methods (note these two are identical; the first will be transpiled into the second by our build tools):

• jsx

  <a href="apollographql.com">link</a>
  

• React.createElement

  React.createElement('a', {href: 'apollographql.com'}, 'link')

Note: jsx is transpiled into React.createElement for use in browsers.

---

As you know, createElement and cloneElement are very different things. cloneElement takes an element, meaning something you've already rendered, and augments it with additional props; hence the classNames merging. createElement takes either an intrinsic jsx element (like "div") or a component and renders it into a react element.

While this not trivial, I think it's more valuable to look at the React docs if someone wants to know how it works rather than explaining it in a comment. In my opinion, I value that you spent the time to read about createElement and cloneElement and I think you got far more out of that than any comment I could leave here. I also think it's better to read the docs because then in the next component where we do the same thing (Buttons, for example), you already know how they all work.

[justinanastos]

@jgzuke

I thought one of the original reasons for as were so we could use Links as buttons more easily

That's correct. Turns out it's not possible. In retrospect; I wish we'd agreed to disagree and commit and use the render prop method instead of spending so much time trying to make the as={Link} case work; especially since it still doesn't.

[justinanastos]

@jgzuke My mistake; I forgot we weren't talking about buttons and the as prop anymore. We could allow the consumer to pass a React.ComponentType, but I don't see any value in doing that as it can't accept any additional props unless you use the element option.

[evans]

Woot! Thanks for the explanation 🎉

[jgzuke]

@justinanastos thanks for explaining where we ended up with that!

[justinanastos]

@evans and @jgzuke ; the questions you asked are exactly the kind of questions I hope people ask. There are things that are only obvious to me because I wrote the code; things that are not obvious are great places to start conversations. Sometimes I'll say it's best to read some docs, other times I'll say you're totally right this needs an explanation. Thank you!

[justinanastos]

🚀 PR was released in v1.7.0 🚀