akeeba / fof

Rapid Application Development framework for Joomla!™ 3 and 4
0 stars 0 forks source link

detectedType should be integer #594

Closed Eighke closed 8 years ago

Eighke commented 8 years ago

Typo in the detection, I was wondering why numeric fields were not generated. :)

nikosdion commented 8 years ago

There are two different field types, Integer and Numeric. I really wanted to use Numeric, not Integer. Float, doubles (i.e. decimals!) and currency (also decimal) cannot be possibly expressed as Integer fields. A price of €3.36 would become €3. That's wrong and dangerous (it screws up your data when you save).

Eighke commented 8 years ago

Right, I agree. But currently every Numeric (Int, bigint, etc) is detected as Numeric. And Numeric detectedType is used no where.

Do we have Float field type ? I will check and try to improve it a bit, I have some free time today.

Eighke commented 8 years ago

Well I just checked... The detected Integer is using Text field in the edit view anyway. So it will not change anything for our database. :)

Anyway, I separated in 2 detectedType, Numeric (float, double, curency) and Integer. For the edit view both are using the same field (Text). But for the browse view, I switched the Numeric detectedType to the Numeric field.

Sound good?

nikosdion commented 8 years ago

We need to use the Numeric field type for floats and Integer for integers. While they may sound like simple text fields there are both server validation rules and client-side HTML5 field types which act differently.

Eighke commented 8 years ago

It's not possible currently. We have 2 problems.

First, the Integer field of FOF is extending JFormFieldInteger. And it is not returning a input type="number", but a genericlist of a range of value...

And because we have no range it will result on a useless select with no option. :/

Second, the Numeric field of FOF is extending Text... So I'm not sure for the sever validation, but I'm sure the client-side HTML 5 will be just a type="text".


Currently Joomla provide a field for Integer and Numeric (type="number") : JFormFieldNumber.

So I see 2 choices:

What do you think?

nikosdion commented 8 years ago

Ugh, right. We're getting to the point where we have to rewrite large parts of JFormFields so let's not go there. I'll accept your PR as is.

nikosdion commented 8 years ago

Actually, I'm going to do the change in your original PR, not accept the PR as-is. Not enough coffee yet.

Eighke commented 8 years ago

Ahah. I just need the scaffolding to work. Looks good now, thanks. :)