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

bugfix: empty string is valid for numericopt #2075

Merged
merged 1 commit into from Aug 27, 2017

Conversation

micgro42
Copy link
Collaborator

This fixes a bug that the empty string was considered invalid for config entries with type numericopt and a min or max restriction.

Bug Description:

Steps to reproduce

  1. create plugin with config-option $conf['foobar'] = ''; in default.php and $meta['foobar'] = array('numericopt', '_min' => 50); in metadata.php
  2. go to config-manager and change that value to 100
  3. save
  4. change that value back to '' (empty string)
  5. try to save

actual result: error message that the entered value is invalid

expected result: save is successful, because numericopt value is allow to be an empty string

There was a bug that the empty string was considered to be invalid if
also a max or min value was defined for a config field of type
numericopt.
@mention-bot
Copy link

@micgro42, thanks for your PR! By analyzing the history of the files in this pull request, we identified @mperry2, @Chris--S and @Klap-in to be potential reviewers.

@splitbrain
Copy link
Collaborator

Hmm. Does that make sense? When I have a numeric config option and specify it should be at least 50, why should I allow an empty value?

@micgro42
Copy link
Collaborator Author

It could be useful to have no value to mean "deactivated" or "auto". The current use case is that the edittable-plugin allows specifying a default-width for the columns. It is sensible to require a minimum width for that default, because a column-width of 5px doesn't make much sense. However I still want to give the user the option (which is also the default value of that config-option) to use handsontable's automatic column-width calculation. That would be (re-)enabled by setting the config value to ''.

Also, if I wouldn't want to allow the empty string, wouldn't it make more sense to just use the numeric type?

@splitbrain
Copy link
Collaborator

You're right. I mixed up numeric and numericopt.

@splitbrain splitbrain merged commit fb8e820 into master Aug 27, 2017
@splitbrain splitbrain deleted the allowEmptyStringNumericOpt branch August 27, 2017 06:16
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.

None yet

4 participants