8000 [Form][Twig] Add an additional input html5 attribute if with_secondes parameters in setted by konandrum · Pull Request #10203 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Form][Twig] Add an additional input html5 attribute if with_secondes parameters in setted #10203

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

[Form][Twig] Add an additional input html5 attribute if with_secondes parameters in setted #10203

wants to merge 2 commits into from

Conversation

konandrum
Copy link
Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
License MIT

@konandrum
Copy link
Author
< 8000 h3 class="f5 text-normal" style="flex: 1 1 auto">

Oups, i saw a big mistake, i have to change the with_minutes by with_seconds !

@fabpot
Copy link
Member
fabpot commented Mar 26, 2014

What's the status of this PR?

@konandrum
Copy link
Author

Fixed, you can merge

@fabpot
Copy link
Member
fabpot commented Mar 27, 2014

Can you also fix the PHP template and add some unit tests?

Tobion
Tobion reviewed Mar 27, 2014
View reviewed changes
src/Symfony/Bridge/Twig/Resources/views/Form/form_div_layout.html.twig
@@ -148,6 +148,9 @@
{% block time_widget %}
{% spaceless %}
{% if widget == 'single_text' %}
{% if with_seconds %}
{% set attr = attr|merge({'step': 1 }) %}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should merge the attributes the other way round, so people can override it.
Also have you considered

The following control allows any time in the day to be selected, with any accuracy (e.g. thousandth-of-a-second accuracy or more): <input name=favtime type=time step=any>

http://www.whatwg.org/specs/web-apps/current-work/multipage/common-input-element-attributes.html#attr-input-step

@tucksaun tucksaun mentioned this pull request Apr 24, 2014
Closed
@fabpot
Copy link
Member
fabpot commented May 16, 2014

Closing in favor of #10777
A5C8

@fabpot fabpot closed this May 16, 2014
fabpot added a commit that referenced this pull request May 16, 2014
@fabpot
bug #10777 [Form] Automatically add step attribute to HTML5 time widg…
0762bae 
…ets to display seconds if needed (tucksaun)

This PR was squashed before being merged into the 2.3 branch (closes #10777).

Discussion
----------

[Form] Automatically add step attribute to HTML5 time widgets to display seconds if needed

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #9976, #10203
| License       | MIT
| Doc PR        | none

Same issue as #9976 and #10203: when you add a `time` field to a form with options `single_text` (so HTML5) and `with_seconds`, the generated input does not contain the `step` attribute, therefore the browser does not show them, leading to an error at the submit because of an invalid format.

Compared to #9976/#10203:
* Unit testable
* Available directly in the component
* Available in other templating format than twig
* Still able to customise the step attribute by hand

Commits
-------

a379298 [Form] Automatically add step attribute to HTML5 time widgets to display seconds if needed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Form TwigBundle
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@konandrum @fabpot @Tobion
0