Skip to content

Forms: Selectbox allow item with null caption [ID => null] #4

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
Merged
dg merged 1 commit into from
Apr 18, 2014
Merged

Forms: Selectbox allow item with null caption [ID => null] #4

dg merged 1 commit into from
Apr 18, 2014

Conversation

@simara-esports
Copy link
Contributor

@simara-esports simara-esports commented Apr 4, 2014

No description provided.

@JanTvrdik
Copy link
Contributor

JanTvrdik commented Apr 4, 2014

Why not just use empty string?

@simara-esports
Copy link
Contributor Author

simara-esports commented Apr 4, 2014

Sorry for the second commit, I'll try to remove it..

Well, it's a bug.
If you'll try to pass data from Selection::fetchPairs(), where one row have an empty name (NULL), you'll see a very strange error message and you'll ask WAT? :-)

mishak87
mishak87 reviewed Apr 4, 2014
View reviewed changes
tests/Forms/Controls.SelectBox.loadData.phpt Outdated
Copy link
Contributor

@mishak87 mishak87 Apr 4, 2014

Choose a reason for hiding this comment

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

=> not ,
Edit: Actually the second item is irrelevant.

@mishak87
Copy link
Contributor

mishak87 commented Apr 4, 2014

It would make more sense to sanitize options in Select. Here null value is a duplicate to empty string.

@simara-esports
Copy link
Contributor Author

simara-esports commented Apr 7, 2014

@mishak87 Typo, it wasn't meant to be 3 items.

@simara-esports
Copy link
Contributor Author

simara-esports commented Apr 15, 2014

Can You please accept or reject this pull request? If you'll reject, please fix the bug described in the test case.

@dg
Copy link
Member

dg commented Apr 15, 2014

I'll merge it, but please squash it to single commit and use uppercased NULL in tests.

@simara-esports
Forms: Selectbox allow item with null caption [ID => null]
349cad9 
Forms: test fix, test for ChoiseControl

Forms: Coding standard fix

NULL should be uppercase
@simara-esports
Copy link
Contributor Author

simara-esports commented Apr 16, 2014

Done, thanks

@dg
Copy link
Member

dg commented Apr 18, 2014

Thx !

dg added a commit that referenced this pull request Apr 18, 2014
@dg
Merge pull request #4 from simara-esports/master
23be8ee 
Forms: Selectbox allow item with null caption [ID => null]
@dg dg merged commit 23be8ee into nette:master Apr 18, 2014
@RobinManoli RobinManoli mentioned this pull request Feb 12, 2020
Closed
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

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@simara-esports @JanTvrdik @mishak87 @dg