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

Numeric string detection #456

Merged
merged 1 commit into from
Mar 6, 2015
Merged

Numeric string detection #456

merged 1 commit into from
Mar 6, 2015

Conversation

RTC1
Copy link

@RTC1 RTC1 commented Mar 6, 2015

No option is selected if the value has been cast as a string, such as when from a form input, due to the new type comparison.

This keeps the type comparison in place, but casts strings as integers where appropriate.

claar added a commit that referenced this pull request Mar 6, 2015
Numeric string detection
@claar claar merged commit 9b949cf into formers:develop Mar 6, 2015
@claar
Copy link
Member

claar commented Mar 6, 2015

Thanks, Rob.

@barryvdh
Copy link
Contributor

barryvdh commented Mar 9, 2015

Hmm I think this probably broke stuff.

$value = "5";
dump(is_string($value)); // true
dump(is_numeric($value)); // true

See http://php.net/manual/en/function.is-numeric.php

And Laravel doesn't really cast proper values, so you are forcing one thing to int, the other you leave as string. Probably need to cast both strings as int, or us is_int

@barryvdh
Copy link
Contributor

barryvdh commented Mar 9, 2015

Sorry this seems to fix it. It was probably #447 that gave problems, but this commit isn't tagged yet.

@RTC1
Copy link
Author

RTC1 commented Mar 9, 2015

Might be worth trying this fix to see if it resolves your issue with #447.

@barryvdh
Copy link
Contributor

barryvdh commented Mar 9, 2015

Yes it did.

@barryvdh
Copy link
Contributor

barryvdh commented Mar 9, 2015

Could this fix be tagged? Without the latest commit, all my dropdowns don't keep their value when editing :(

@claar
Copy link
Member

claar commented Mar 9, 2015

Tagged 3.5.6

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.

3 participants