Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for HTML5 input types search and tel #1348

Closed
wants to merge 2 commits into from
Closed

Add support for HTML5 input types search and tel #1348

wants to merge 2 commits into from

Conversation

xificurk
Copy link
Contributor

Both type=tel and type=search inputs are essentially the same as standard type=text. The main difference is semantic.

@fprochazka
Copy link
Contributor

Looks allright 👍

@dg
Copy link
Member

dg commented Jan 11, 2014

There is 23 types of input (text, password, checkbox, radio, button, submit, reset, file, hidden, image, datetime, datetime-local, date, month, time, week, number, range, email, url, search, tel, color) and the count is not final. I think there is no benefit to have 23 different methods, especially not for absolutely same types from framework view.

addText('tel')->setType('tel') is for me understandable, maybe we can add constants (addText('tel')->setType(Form::TELEPHONE') or addInput('tel', Form::TELEPHONE')), what do you think?

@Majkl578
Copy link
Contributor

No, not constants, at least not on Form...

@xificurk
Copy link
Contributor Author

@dg Your point is valid only for the three types in this PR - text, search, tel. From the framework point of view, all of them should behave the same way. But the same thing cannot be said about the rest. So, the real question here is whether or not to add methods for telephone and search.

@mishak87
Copy link
Contributor

All controls wrap some behavior or additional logic and hence have custom methods for adding. Search and telephone are just shortcuts for renderer. As such can be in own Html5Forms helper that extends Form via mixins.

By name of this pull I would expect SearchControl and TelephoneControl with pattern validation, sanitization and other logic that would be useful. This pull is IMO pointless. Is like adding addWysiwyg() that would just to do this:

$control = $this->addTextarea();
$control->getControlPrototype()->class = 'tinyMCE';
return $control;

@dg
Copy link
Member

dg commented Jan 12, 2014

@mishak87 👍

@xificurk
Copy link
Contributor Author

@mishak87 Your example differs in one important detail. Textarea with class tinyMCE is meaningless, unless you bind some scripting to that class on the frontend, thus your method addWysiwig will be most probably different than someone else's. On the other hand, inputs with type=tel and type=search are part of HTML5 specification and they have well defined meaning and UA (especially on touch screens) treat them differently than type=text.

I have a couple of reasons why I would like to see this in Nette:

  • Better readability. I really think that $form->addTelephone('landline') is better than $form->addText('landline')->setType('telephone')
  • IDE auto-completion when I start typing $form->add. This is motivated by more important issue than laziness - it prevents you from making stupid mistakes like the one above - it's type=tel, not type=telephone, not type=phone...
  • I always thought of setType on TextInput as a temporary workaround until a proper support for other input types is added. It's prone to errors as demonstrated above. Furthermore, I don't think it belongs to the public API of TextInput, especially since there is no check for the value you're setting, so $form->addText('invalid')->setType('checkbox') is allowed.

I'm aware that all of those are pretty weak arguments, it's more of category "nice to have" than "must have", but is there any real reason why not to add them?

@mishak87
Copy link
Contributor

@xificurk Is there any issue with mixins I suggested other then: "My IDE is stupid as hell and I don't want to write plugin that will feed them to autocomplete"?

Disclaimer: All IDEs are inherently stupid since they focus on what programmer needed years ago instead of what (s)he might need in the future.

@pavelkouril
Copy link
Contributor

I'm for adding "addSearch", "addTelephone", etc., because they ARE different types of inputs than classic text input.

Sure, it will be tons of added methods, but each of them will have different semantical meaning.

@mrtnzlml
Copy link

Bump... Btw why addTelephone and not addTel?

@fprochazka
Copy link
Contributor

@mrtnzlml because shortcuts are anti-pattern.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants