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

[Form] fixed allow render 0 numeric input value #10053

Merged
merged 1 commit into from Jan 20, 2014

Conversation

Projects
None yet
6 participants
@dczech
Contributor

dczech commented Jan 17, 2014

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #10052
License MIT
Doc PR
@jakzal

This comment has been minimized.

Show comment
Hide comment
@jakzal

jakzal Jan 17, 2014

Member

What about the twig template?

Member

jakzal commented Jan 17, 2014

What about the twig template?

@@ -1,5 +1,5 @@
<input
type="<?php echo isset($type) ? $view->escape($type) : 'text' ?>"
<?php if (!empty($value)): ?>value="<?php echo $view->escape($value) ?>"<?php endif ?>
<?php if (!empty($value) || is_numeric($value)): ?>value="<?php echo $view->escape($value) ?>"<?php endif ?>

This comment has been minimized.

@Tobion

Tobion Jan 17, 2014

Member

why is the condition needed at all? wouldn't value="" not mean the same as without the attribute?

@Tobion

Tobion Jan 17, 2014

Member

why is the condition needed at all? wouldn't value="" not mean the same as without the attribute?

This comment has been minimized.

@stof

stof Jan 17, 2014

Member

the issue is that is_numeric does not cover the case of value="" but value="0"

@stof

stof Jan 17, 2014

Member

the issue is that is_numeric does not cover the case of value="" but value="0"

This comment has been minimized.

@Tobion

Tobion Jan 17, 2014

Member

i asked why we dont remove the if condition completely

@Tobion

Tobion Jan 17, 2014

Member

i asked why we dont remove the if condition completely

This comment has been minimized.

@stof

stof Jan 17, 2014

Member

IE doing weird stuff for empty option values

@stof

stof Jan 17, 2014

Member

IE doing weird stuff for empty option values

@stof

This comment has been minimized.

Show comment
Hide comment
@stof
Member

stof commented Jan 17, 2014

@dczech

This comment has been minimized.

Show comment
Hide comment
@dczech

dczech Jan 17, 2014

Contributor

@ALL stof explained all doubts, thx

Contributor

dczech commented Jan 17, 2014

@ALL stof explained all doubts, thx

@fabpot

This comment has been minimized.

Show comment
Hide comment
@fabpot

fabpot Jan 20, 2014

Member

Good catch, thanks @dczech.

Member

fabpot commented Jan 20, 2014

Good catch, thanks @dczech.

fabpot added a commit that referenced this pull request Jan 20, 2014

bug #10053 [Form] fixed allow render 0 numeric input value (dczech)
This PR was merged into the 2.3 branch.

Discussion
----------

[Form] fixed allow render 0 numeric input value

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

Commits
-------

82f98c4 [Form] fixed allow render 0 and 0.0 numeric input values

@fabpot fabpot merged commit 82f98c4 into symfony:2.3 Jan 20, 2014

1 check passed

default Success: Travis, fabbot
Details

@dczech dczech deleted the dczech:2.3 branch Jan 20, 2014

@webmozart

This comment has been minimized.

Show comment
Hide comment
@webmozart

webmozart Jan 20, 2014

Contributor

I would appreciate if the comparison was for "0" instead of doing an is_numeric() check.

Contributor

webmozart commented Jan 20, 2014

I would appreciate if the comparison was for "0" instead of doing an is_numeric() check.

@dczech

This comment has been minimized.

Show comment
Hide comment
@dczech

dczech Jan 20, 2014

Contributor

I considered this option too but in this case I had to assume that form_widget_simple couldn't get integer value. It was safer to use is_numeric check

Contributor

dczech commented Jan 20, 2014

I considered this option too but in this case I had to assume that form_widget_simple couldn't get integer value. It was safer to use is_numeric check

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