material-ui icon indicating copy to clipboard operation
material-ui copied to clipboard

[material-ui][TextField] InputProps vs inputProps confusion

Open eyn opened this issue 6 years ago • 27 comments

The TextField (nor Input) does not pass the step property through to the input as a step attribute.

  • [X] I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior

<Textfield type="number" step="0.01" /> renders a <input type="number" step="0.01" />

Current Behavior

<Textfield type="number" step="0.01" /> renders a <input type="number" />

Steps to Reproduce (for bugs)

  1. Go to https://codesandbox.io/s/lyxw254vz9
  2. Click on the up/down arrows on right of textbox and see differences in behaviour

Context

Your Environment

Tech Version
Material-UI 1.0.0.rc0
React 16
browser Safari - Not possible to insert 0.xx number and pass validation. Chrome accepts 0.xx number and validates it but step is still by unit

eyn avatar May 14 '18 12:05 eyn

Happy to do a pr for this if you're happy having step added to TextField (and Input)?

eyn avatar May 14 '18 12:05 eyn

didn't test, but I guess you should use <TextField inputProps={{step: 0.01}} /> instead? It#s a prop on Input instead of TextField.

update: sry, just checked your sandbox :D you already came up with that. What#s the problem with the inputProps solution? I guess there are more props we don't pass down automagically: https://developer.mozilla.org/en-US/docs/Web/API/HTMLInputElement so it might be consistent to not do it at all.

sakulstra avatar May 14 '18 13:05 sakulstra

@eyn We try to find a tradeoff between the popular properties applied on the input and the bundle size of the component. step hasn't made it yet.

oliviertassinari avatar May 14 '18 15:05 oliviertassinari

I have added the waiting for users upvotes tag. I'm closing the issue as I'm not sure people are looking for such abstraction. So please upvote this issue if you are. We will prioritize our effort based on the number of upvotes.

oliviertassinari avatar May 14 '18 17:05 oliviertassinari

Agree it's a balance and step probably falls outside that line at the moment. Just wanted at least a discussion. Any thoughts about a helpful message in development mode for properties that are valid on <input /> but not passed on by TextField or Input.

eyn avatar May 15 '18 07:05 eyn

Any thoughts about a helpful message in development mode for properties that are valid on but not passed on by TextField or Input.

@eyn I think that it's a good idea 👍

oliviertassinari avatar May 15 '18 07:05 oliviertassinari

So… Is there actually no way to specify the steps attribute? :disappointed:

leMaik avatar Jan 24 '19 17:01 leMaik

@leMaik You can with the inputProps property: https://codesandbox.io/s/lyxw254vz9

<TextField inputProps={{ step: "0.01" }} type="number" />

oliviertassinari avatar Jan 24 '19 23:01 oliviertassinari

I think that @eyn's idea is brilliant. We could add warnings about this issue at runtime so people don't have to find this issue on GitHub.

oliviertassinari avatar Jan 24 '19 23:01 oliviertassinari

@oliviertassinari Oh, with a small i. :sweat: A warning would have helped me a lot, indeed. I'll look into adding them later. :+1:

leMaik avatar Jan 26 '19 21:01 leMaik

we've been running into this problem over and over again(with min/max/step) :D and eventually ended up writing a custom NumberField which deflattens these props. Would it make sense to expose such a component within material-ui?

sakulstra avatar May 02 '19 15:05 sakulstra

@sakulstra Do you have an example of how it works? It could be interesting.

oliviertassinari avatar May 03 '19 11:05 oliviertassinari

Pretty sure it isn't :sweat_smile:

We're using this within formik so i tried to extract the not super exciting gist of what we do ;)

const NumberField = ({step, min, max, inputProps, ...rest}) => {
  let inputProps = Object.assing({}, inputProps);
  if (step) inputProps.step = step;
  if (min) inputProps.min = min;
  if (max) inputProps.max = max;
  return (
    <TextField inputProps={inputProps} {...rest} />
  )
}

There's probably a more elegant solution to this, bu we basically whitelisted some props which currently aren't forwarded to inputProps automatically and manually forward them.

sakulstra avatar May 03 '19 11:05 sakulstra

@sakulstra Thanks for sharing the implementation. It's a problem we are exploring in #14580.

There is one pragmatic option we have considered: we could deprecate the inputProps. Instead of forwarding the props to the root element, we could forward them to the inner input element. You almost never need to target the wrapping div (we solve the problem with a ContainerComponent and ContainerProps props for the ListItem).

However, there is one downside to it. It breaks the convention almost all the other components are following. What's the purpose of a convention? Create predictability and intuitive APIs. I'm not sure we actually have an intuitive API here. I see a lot of people struggling with the inputProps existence. We would solve:

  • #14555 The aria label are not applied on the right element
  • #11377 🔺
  • #12805 Just to put this somewhere, it seems like InputProps and inputProps are not the same:
  • #10064 eslint duplicate error InputProps and inputProps
  • #16952 eslint duplicate error InputProps and inputProps
  • #15070 greater 10 or less than zero by setting the min and max in InputProps. But its not working
  • #13183 Mehh., inputprops is they way
  • #12369 Input of type file ignores the accept prop`
  • #12365 InputProps are not spread on the right element
  • #11427 How to apply attribute on the native input?
  • #9480 InputProps do not get applied to input
  • #7768 maxlength attribute is not working
  • and more…

cc @eps1lon

oliviertassinari avatar May 03 '19 18:05 oliviertassinari

@oliviertassinari thanks for the references, looking trough this, it seems like your approach makes a lot more sense! Looking forward to this change ;)

sakulstra avatar May 04 '19 16:05 sakulstra

As a last resort, if the choice is pragmatic and intuitive, I would choose to break convention I suppose. Form in general have always been a pain point with special complexities so they might unique in a sense. Seems like it solves a whole host of problems that would be nice to eliminate.

jhoffmcd avatar May 08 '19 03:05 jhoffmcd

I'm still having trouble understanding the difference between inputProps vs InputProps

image TextField.d.ts

  1. I noticed that autoComplete="new-password" mentioned here works when I have "inputProps" but it does not when I have InputProps
  2. endAdornment works when I have InputProps and not when I have inputProps
  3. if I have both (which wouldn't make sense based on the screenshot attached) it slows down the browser significantly.

davext avatar Apr 01 '21 17:04 davext

@davext The component/DOM structure of TextField is:

  • FormControl
    • InputLabel
    • Input
      • input
    • FormHelperText

it should clarify it

oliviertassinari avatar Apr 01 '21 18:04 oliviertassinari

@oliviertassinari

I discovered a better trick for the autoComplete to be disabled.

Instead of the label in the AutoComplete to be the name of the field, I take it out, and an input field with a hidden slash and it totally disabled Chrome's auto complete. Here's a code example:

<InputLabel>First Name<Typography hidden>/</Typography></InputLabel> <Autocomplete...

davext avatar Apr 02 '21 03:04 davext

I'm still having trouble understanding the difference between inputProps vs InputProps

index c33fab4291..7a582b6b84 100644
--- a/packages/material-ui/src/TextField/TextField.js
+++ b/packages/material-ui/src/TextField/TextField.js
@@ -302,7 +302,7 @@ TextField.propTypes /* remove-proptypes */ = {
    */
   inputProps: PropTypes.object,
   /**
-   * Props applied to the Input element.
+   * Props applied to the Input component.
    * It will be a [`FilledInput`](/api/filled-input/),
    * [`OutlinedInput`](/api/outlined-input/) or [`Input`](/api/input/)
    * component depending on the `variant` prop value.

Could this diff help? It would at least add a differentiation between the component vs element.

mnajdova avatar Jun 25 '21 12:06 mnajdova

@mnajdova I'm not sure what the right terminology is. From my perspective, props are first applied to the element, then made available in the component's render method.

At this point, I would either propose:

  1. A better documentation https://github.com/mui-org/material-ui/issues/11377#issuecomment-812101655
Capture d’écran 2021-06-25 à 15 59 59
  1. Renaming inputhtmlInput to match the htmlFor (React), htmlColor (Icon component) props convention.

We can likely do these two changes.

oliviertassinari avatar Jun 25 '21 14:06 oliviertassinari

The current convention triggers the lint rule react/jsx-no-duplicate-props and we have no way around it.

vicary avatar Jul 27 '21 06:07 vicary

Prevent duplicate properties in JSX (react/jsx-no-duplicate-props)

   .....
    InputProps={InputProps}
    inputProps={inputProps}
   ......
    />

This problem was signaled here since 2018: ESlint give error duplicate error.

Is there a solution to fix this issue ? We are using v5.

mahady-manana avatar Oct 25 '21 11:10 mahady-manana

Hey everyone, I'm working on this alongside the https://github.com/mui/material-ui/issues/41281 work. My current view on how to solve the confusion relying on the slots pattern is:

InputProps

To be deprecated in favor of slotProps.input

inputProps

For this one, I see two options to deprecate in favor of:

  1. slotProps.htmlInput: Differentiates the name from slotProps.input and follows React's htmlFor pattern. Pro: keeps the input element available without accessing nested slots.
  2. slotProps.input.slotProps.input: More confusing than option 1, but might be logical for users familiar with the slots pattern.

I prefer option 1., but wonder about your opinion or if you see other options.

DiegoAndai avatar Apr 17 '24 14:04 DiegoAndai

I opened my proposal as a PR ☝🏼

DiegoAndai avatar Apr 29 '24 20:04 DiegoAndai

  1. slotProps.htmlInput: Differentiates the name from slotProps.input and follows React's htmlFor pattern. Pro: keeps the input element available without accessing nested slots.

I like this better, hoisting the API that makes sense on top makes sense, however this will fail if the Input slot is changed and it has a different API for the HTML input. I don't think this is very likely to happen, but at least the second option does not suffer from this issue.

mnajdova avatar May 01 '24 05:05 mnajdova

this will fail if the Input slot is changed and it has a different API for the HTML input.

I think option 2 suffers from this as well. If the custom input does not support slotProps.input, then TextField's slotProps.input.slotProps.input wouldn't work either.

DiegoAndai avatar May 02 '24 16:05 DiegoAndai