ionic-team / ionic-framework

A powerful cross-platform UI toolkit for building native-quality iOS, Android, and Progressive Web Apps with HTML, CSS, and JavaScript.
https://ionicframework.com
MIT License
51.09k stars 13.51k forks source link

bug: Input type number is not respected #7121

Closed davidruisinger closed 6 years ago

davidruisinger commented 8 years ago

Type: bug

Ionic Version: 2.x

Platform: all

Steps to reproduce:

1) Create an ion-input with type="number" 2) Add [(ngModel)] and (ngModelChange) to get the Value of the input 3) Check the type of the value using typeOf(value) 4) Type in a number into the input => Type is always a string

Expected behavior: ... => Type is a number.

See this plunk as an example: https://run.plnkr.co/plunks/GHBFad/

awe-sim commented 8 years ago

The Alert component exhibits the similar behavior. Set an input type to number in a Alert component, the input value is still a string and the data returned to the button handler function also contains a string.

Ideally, the input value property should be string | number and not just string. And the data returned to the handler should have the appropriate type as well.

It is really cumbersome to do something like this:

let alert = new Alert({
    inputs: [{
        name: 'amount',
        type: 'number',
        value: myAmount.toString() // <-- SHOULD NOT REQUIRE toString()
    }],
    buttons: [{
        text: 'OK',
        handler: (data) => {
            console.log(typeof data.amount); // <-- SHOULD NOT OUTPUT 'string'
        }
    }]
});
marpstar commented 8 years ago

We're seeing similar issues with type="number". In the example Plunkr as well as our code, the number input fails to use the numeric keypad on iOS. Haven't tested Android.

Using <ion-input type="text" pattern="\d*"> does launch the numeric keypad on iOS.

zakton5 commented 8 years ago

Same as #6570. This is still an issue. Could anyone from the team respond?

zakton5 commented 7 years ago

Hey guys. Any progress on this?

dpsoft commented 7 years ago

workaround:

Template:

 <ion-input 
    type="number" 
    [ngModel]="progress" 
    (ngModelChange)="progress = convertToNumber($event)">
  </ion-input>

Function:

public convertToNumber(event):number {  return +event; }

hope this helps.

mirkonasato commented 7 years ago

The original plunk doesn't seem to work any more.

Here's a new one with Ionic 2.0.0-rc.3: http://plnkr.co/edit/EjsP3h?p=preview

Note that this works as expected with plain Angular: http://plnkr.co/edit/rIr9jVkUnIfW1FZIbROx?p=preview

mirkonasato commented 7 years ago

The underlying issue is with setting the type dynamically, in input.ts.

That doesn't work with plain Angular either, the value becomes a string after editing it: http://plnkr.co/edit/mG5F636NLpkmj09UPWkd?p=preview

Angular's NumberValueAccessor directive has input[type=number] as its selector. I guess that's only evaluated when the component is created, and at that point the type is not yet set?

brandyscarney commented 7 years ago

I confirmed this is still an issue with latest.

ctc1995 commented 7 years ago

Hey everybody, How can i solve this problem? Any help?

manucorporat commented 7 years ago

http://stackoverflow.com/questions/35791767/html-input-type-number-still-returning-a-string-when-accessed-from-javascript

not even the DOM consider type="number" when returning the type. The DOM always returns a string, I think this is a won't fix.

joshgarwood commented 7 years ago

Please do not close this :) It was even included in the 2.3 release milestone.

This is almost more of an issue with the keyboard than a data type issue. For type="number" I would expect the numeric keyboard to appear, but this does not happen.

zakton5 commented 7 years ago

@manucorporat If you test this using Angular2 you will see that a number is returned. Is it possible to just leverage Angular's logic to get the same result?

brandyscarney commented 7 years ago

I think there are a few issues going on in this one issue. Here's my attempt at a recap (let me know if anything I said is wrong):

  1. Original issue by @flavordaaave - using <ion-input type="number"> the value is returning string type instead of number. I can't get the original plunker to work currently, thanks @mirkonasato for the plunkers, can reproduce with: http://plnkr.co/edit/EjsP3h?p=preview
  2. @awe-sim - Alert is only accepting / returning string values in inputs
  3. @marpstar / @joshgarwood - numeric keyboard is not appearing with <ion-input type="number">

So our issue is that we pass input type dynamically here: https://github.com/ionic-team/ionic/blob/master/src/components/input/input.ts#L103

Which is broken and reported by this Angular issue: https://github.com/angular/angular/issues/13243

Reopening this since it works without the dynamic binding.

Note to team - it may be blocked by Angular but it's worth investigating. Maybe we should separate the alert / keyboard issue out.

zakton5 commented 7 years ago

Looks good @brandyscarney. Thanks for the nice summary!

SwenVogel commented 7 years ago

Any progress on this topic?

n2lose commented 7 years ago

Seems we still need to find some workaround to fix it temporary

felschr commented 7 years ago

@n2lose For me the workaround by @dpsoft works. I'm still hoping for a proper fix though.

shootdaj commented 7 years ago

The workaround works for ngModel, but is there a similar workaround for using reactive forms? If you specify a form control, there is no way to intercept the value ion-input sets in the form. Anyone know of a solution?

zakton5 commented 7 years ago

@shootdaj You may be able to do something like

this.form.get('control').valueChanges.subscribe((newVal: string) => {
  this.form.get('control').setValue(+newVal);
});

There may be a better workaround though.

shootdaj commented 7 years ago

@zakton5 Thanks for your help. I've tried this approach in the past, it ends up being really cumbersome because you have to handle all these form controls separately and keep them in sync with your form. This is fine when you have a form with fixed fields, but it gets much more tricky when you are dynamically building a form. It's not really a scalable solution. Ideally, you should be able to intercept the value the HTML control is sending to the form and override it and you should be able to set that interception function in your form definition itself. But AFAIK, Angular does not support that. It also causes some weird unexpected behavior when you have validation around your reactive form since it is first setting the value to the string (causing validation to fail) and then to the number.

Also, I believe your code would result in an infinite loop since setting the value (.setValue) will cause valueChanges to fire, which will set the value, which will fire valueChanges, and so on. So you have to use emitEvent = false : this.form.controls['control'].setValue(value, {emitEvent: false}). emitEvent: false causes valueChanges to not fire. This is the part where the validation issues come in because I think the validation requires the event to be emitted? Not 100% sure about that but I've noticed strange behavior with this method.

zakton5 commented 7 years ago

@shootdaj Good catch with the emitEvent: false. That is all correct. Basically, this needs to be fixed and there is no good, scalable workaround. This should really be higher on ionic's priority list

santiq commented 7 years ago

Any update on this? I'm really worry about it!!!

zakton5 commented 7 years ago

Nope, when is there ever an update from the ionic team?

shootdaj commented 7 years ago

I got around this issue (not this exact issue, but an issue with the same underlying problem) by creating a custom control that wraps the ionic control. This custom control essentially intercepts the value using ngModel and sets it to a form control passed in to the custom control.

The problem I was dealing with was that I wanted to use a range of 0-1 (decimal values) for ion-range but ion-range doesn't support decimal values. It only supports integer values. So I created this custom control:

TS: https://github.com/shootdaj/ZoneLighting/blob/develop/UI/UI/src/components/zl-range/zl-range.ts HTML: https://github.com/shootdaj/ZoneLighting/blob/develop/UI/UI/src/components/zl-range/zl-range.html

And it is used like this: https://github.com/shootdaj/ZoneLighting/blob/develop/UI/UI/src/components/dynamic-input/dynamic-input.html#L4

The dynamic-input component passes in a reference to a form control to zl-range which is used by zl-range as the form to change the value for. When the user drags the range slider, the [(ngModel)] sets the selectedValue which in turn sets the form value to the incoming value divided by 100 (converting the range of 1-100 to 0-1).

To solve this issue, you can create a similar control but instead of dividing the value you can just set it directly as the type=number should ensure that the ngModel passes in a number type and not a string. Even if it passes string, you can convert the string to a number and then set it to the form.

Note: I didn't paste the actual code because the code formatting was being thrown off by the single quotes and it appeared staggered.

rollermy commented 7 years ago

I just multiplied the value by 1 "1" 1 // = 1 "1.0" 1 // = 1.0 For reference: https://stackoverflow.com/a/33544880/4309496

matoos32 commented 7 years ago

Still an issue with Ionic 3 + Angular 5. typeof on the event object is giving "string" using @dpsoft 's convertToNumber($event) technique.

wormling commented 6 years ago

This issue causes us trouble all over our system and we still don't have a reasonable solution. Ionic 3.9.2 + Angular 5.0.1

mhartington commented 6 years ago

Hi there! So while we appreciate the feedback, the original cause of this issue has been linked to already

https://github.com/ionic-team/ionic/issues/7121#issuecomment-287143709

This is an internal issue on Angular's side, so a fix would need to come from them. Im going to be locking this issue, as any constructive work needs to happen on angular's side.

Thanks!