PolymerElements / iron-input

An input with data binding
https://webcomponents.org/element/PolymerElements/iron-input
33 stars 45 forks source link

preventInvalidInput does not work when pasting text #77

Closed dsschnau closed 8 years ago

dsschnau commented 8 years ago

The description of allowedPattern is as follows:

Set this to specify the pattern allowed by preventInvalidInput. Bind this to the 's allowedPattern property.

I had assumed that the pattern combined with preventInvalidInput would check the value of the entirety of value. After digging into the code I discovered that when a user types a new character into an iron-input with these properties set up, paper-input validates not against the entirety of the new value but rather only the single character checked.

Along with this, when content is pasted into an iron-input, I expected that the entire string would be validated against, but instead I found that each individual character has the regex tested against it.

I understand that changing it would break a lot of things for other users but I personally don't think this is the best approach.

So I propose two things:

  1. the documentation is updated to clarify the current behavior
  2. provide a way to do regex validation against the entire value in an iron-input instead of individual characters
notwaldorf commented 8 years ago

Argh, you're totally right. On paste, preventInvalidInput doesn't do anything right. I'm a little worried about changing the entire logic of that method (especially since allowedPattern isn't really a regex, is more like a pattern, so if you validate it as a regex it's probably going to fail).

I'm also not 100% sure what the behaviour should be when you paste something incorrect -- should nothing happen? In that case, we could maybe add an event listener for the paste event, checking each of the characters of the pasted text against the method (kind of like if they were typed one by one), and if any are invalid, don't commit the text.

What do you think? This would leave the status quo as it is, and fix the broken paste behaviour.

notwaldorf commented 8 years ago

I've renamed the issue. I don't think the problem is with the code (since again, allowedPattern is not really a regex, so it won't match), but with the paste event

dsschnau commented 8 years ago

Firstly thanks for the detailed answer, very much appreciated.

I agree there isn't much problem with the code but it is important to clarify what allowedPattern really means in the documentation (with your blessing I'll gladly make a PR for it).

To answer your question about what should happen when pasting, I have two thoughts.

The first, more literal interpretation of the intent of allowedPattern, is that only the characters pasted that match the pattern are committed to the iron-input.

However, that might cause some very misleading things downstream. For example, a user pastes a model number (say "M49X") for some widget into an iron-input whose allowedPattern is [a-zA-Z] and is confused as to why only the letters are being pasted MX "What happened to the 49?".

The second, and what I think makes the most sense unless there's a good reason I'm not thinking of, leaving it the way it is (and clearly documenting it) would probably be the best idea.

notwaldorf commented 8 years ago

Documenting it then would be great. I was thinking that if you try to paste M49X you would actually get nothing (so none of the characters would be committed), but that's still a bit of a baffling interaction.

I think adding a note about how this prevents the user typing invalid input, but won't actually affect pasting would be amazing! Make sure to CC me on the PR so I can review it :)

Thanks!

notwaldorf commented 8 years ago

Fixed in https://github.com/PolymerElements/iron-input/pull/81