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

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

Merged
merged 1 commit into from
Jan 20, 2014
Merged

Conversation

dczech
Copy link
Contributor

@dczech 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
Copy link
Contributor

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 ?>
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

i asked why we dont remove the if condition completely

Copy link
Member

Choose a reason for hiding this comment

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

IE doing weird stuff for empty option values

@stof
Copy link
Member

stof commented Jan 17, 2014

@dczech
Copy link
Contributor Author

dczech commented Jan 17, 2014

@ALL stof explained all doubts, thx

@fabpot
Copy link
Member

fabpot commented Jan 20, 2014

Good catch, thanks @dczech.

fabpot added a commit that referenced this pull request Jan 20, 2014
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
@dczech dczech deleted the 2.3 branch January 20, 2014 08:49
@webmozart
Copy link
Contributor

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

@dczech
Copy link
Contributor Author

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants