hlsolutions / jquery-autosuggest

An AutoSuggest jQuery plugin.
http://hlsolutions.github.io/jquery-autosuggest/
GNU General Public License v2.0
15 stars 3 forks source link

Add validation message for min and max tag length #24

Closed coryschires closed 11 years ago

coryschires commented 11 years ago

Thanks for maintaining this plugin! It's great, I've been using it for years.

I have a few sorta related issues:

  1. The minChars option works correctly but gives the user no feedback explaining why their tag was rejected. It seems obviously bad UI to enforce a validation without giving the user some sort of feedback.
  2. I would like a similar option for maxChars. I know it sounds ridiculous but our users sometimes paste in a long list of keywords, hit tab, and the plugin essentially creates one giant keyword. This seems like a generic enough problem that it should be solved at the library level. Similar to 1, the maxChars option should display some type of validation message to the user.

As far as displaying the validation messages, I was thinking we could just dynamically add/remove an span which would contain the error message. We could make the span class name configurable. That way the plugin could handle the error logic but the client app could still control the styles.

As for the error message text, I was thinking:

must be at least X characters
must be fewer than X characters

We could make the error text configurable as well; however, I think that might be overkill.

I'm totally willing to make these changes myself and submit a PR. I just want to make sure you're open to these ideas before doing the work.

knalli commented 11 years ago

Generally, yes! I'm currently not putting more in it because we.. well, simply do not use it any more. But a good PR is welcome any time.

I think I got your points (actually two?) but perhaps you can describe it a little bit more? Error message seems obvious to me, but not the thing with splitting and span.

coryschires commented 11 years ago

Thanks for getting back so quickly.

About the span, I was just trying to explain how I was planning on going about coding the validation messages. Perhaps I was giving too much implementation details.

But to clarify, I was thinking something like:

Given the autoSuggest input requires a tag to have at least 3 characters
And I input a tag of "AA"
And I press tab
Then I should see a message saying "must be at least 3 characters"
When I update the tag to be "AAA"
Then I should not see a message saying "must be at least 3 characters"
And I press tab
Then a tag of "AAA" should be added

Under the hood, I think the plugin should add a span to the DOM that looks something like:

<span class='error'>must be at least 3 characters<span>

Obviously this span would be added or removed from the DOM depending on the validity of the current tag length.

I was also thinking about making the class name (i.e. error) configurable. But, on second thought, let's not get carried away. error seems like a perfectly good name. If someone needs additional configuration, I could always add it later.

Make sense?

coryschires commented 11 years ago

Oh, I see. I'm not suggesting trying to split copy-n-pasted input into separate tags. I just want them to see that they are doing it wrong. They can split it up themselves.

That might also be a good idea. But I think that would/should be a separate ticket.

knalli commented 11 years ago

Ah, okay. Okay, but I would use a callback option for this. This options (like renderErrorMessage) can be overridden to style custom elements. A corresponding destroyErrorMessage would be required too.

So, your span generator is actually only an implementation of these both option callbacks.

coryschires commented 11 years ago

I see. So the plugin would define both those callbacks which, by default, would render the span I'm suggesting. But you could override them if you wanted.

Correct?

coryschires commented 11 years ago

Or were you thinking the callback should do nothing. Just pass some arguments and the client app would need to be explicit?

I'm good with either.

knalli commented 11 years ago

I see. So the plugin would define both those callbacks which, by default, would render the span I'm suggesting. But you could override them if you wanted.

Yes!

Just like the other callbacks we already have.

coryschires commented 11 years ago

Sounds good. I probably submit it later today or tomorrow.

Thanks again!