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

3.0.0.RC1 - f:all - Invalid markup is being generated for numeric fields. Commas are added to type number. #301

Open
codeconsole opened this issue Apr 20, 2019 · 5 comments
Assignees

Comments

@codeconsole
Copy link
Contributor

codeconsole commented Apr 20, 2019

I have created a very basic grails 4.0.0.RC1 app to demonstrate the problem:

https://github.com/codeconsole/grailsnumberbug

package numberbug

class Bug {

    int num = 123456789

    static constraints = {
    }
}

<f:all bean="bug"/>

creates an input field
<input type="number" name="num" value="123,456,789" required="" id="num">

which is not valid markup in either Chrome or Safari.

The default behavior should be to not put commas in numbers due to the fact it breaks everything

@codeconsole
Copy link
Contributor Author

Not sure why you are formatting a number for an input value? I would think it would be best to change this

attrs.value = numberFormatter.format(propertyAccessor.value)

to

attrs.value = propertyAccessor.value

https://github.com/grails-fields-plugin/grails-fields/blob/master/grails-app/taglib/grails/plugin/formfields/FormFieldsTagLib.groovy#L729

	CharSequence renderNumericInput(BeanPropertyAccessor propertyAccessor, Map model, Map attrs) {
		Constrained constrained = (Constrained) model.constraints

		if (!attrs.type && constrained?.inList) {
			attrs.from = constrained.inList
			if (!model.required) attrs.noSelection = ["": ""]
			return g.select(attrs)
		} else if (constrained?.range) {
			attrs.type = attrs.type ?: "range"
			attrs.min = constrained.range.from
			attrs.max = constrained.range.to
		} else {
			attrs.type = attrs.type ?: getDefaultNumberType(model)
			if (constrained?.scale != null) attrs.step = "0.${'0' * (constrained.scale - 1)}1"
			if (constrained?.min != null) attrs.min = constrained.min
			if (constrained?.max != null) attrs.max = constrained.max
		}

		if (localizeNumbers && propertyAccessor?.value != null) {
			attrs.value = numberFormatter.format(propertyAccessor.value)
		}

		return g.field(attrs)
	}

@rvanderwerf rvanderwerf self-assigned this Apr 22, 2019
@rvanderwerf
Copy link

So it looks like you can set the property grails.plugin.fields.localizeNumbers = false to fix this but it might not format the number at all.

@rvanderwerf
Copy link

actually it will still format the view part and not the input area with grails.plugin.fields.localizeNumbers = false so I don't think this is a bug. This should probably be the default though, as you said formating input type number is invalid.

@codeconsole
Copy link
Contributor Author

Hi Ryan,

Thanks for identifying
grails.plugin.fields.localizeNumbers = false

That got my forms working again.

I don't have a bias on whether is should be true or false, but in any case, the input value should DEFINITELY NOT be formatted because it is invalid markup and fails in both Chrome and Safari.

I recommend either way changing

attrs.value = numberFormatter.format(propertyAccessor.value)

to

attrs.value = propertyAccessor.value

as previously mentioned.

@rvanderwerf
Copy link

yes it appears the only place that flag is used for that field, so I'm not sure why it's configurable at all if true just makes an invalid input box.

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

No branches or pull requests

2 participants