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

ion-input ignores the disabled attribute when created using a FormControl in a Reactive Form #9834

Closed
kelvindart opened this issue Dec 30, 2016 · 15 comments · Fixed by #9994
Closed
Assignees
Milestone

Comments

@kelvindart
Copy link
Contributor

kelvindart commented Dec 30, 2016

Ionic version: (check one with "x")
[ ] 1.x
[x] 2.x

I'm submitting a ... (check one with "x")
[x] bug report
[ ] feature request
[ ] support request => Please do not submit support requests here, use one of these channels: https://forum.ionicframework.com/ or http://ionicworldwide.herokuapp.com/

Current behavior:
When you create a form using the FormBuilder.group(...) method and assign an ion-input to a FormControl, it seems to ignore the disabled attribute set in the state object, documented here: https://angular.io/docs/ts/latest/api/forms/index/FormControl-class.html, and the ion-input is enabled regardless.

On a regular HTML input this seems to work as expected, so I am thinking it's something in the ion-input not correctly being set.

N.b. when I set the disabled attribute directly in the mark-up on ion-input it does follow the correct behaviour:

<ion-input type="text" formControlName="ioninput" disabled></ion-input>

But this is undesired (since I cannot set it programatically) and I receive the following console warning from Angular:

console warning

Expected behavior:
For the ion-input to be disabled, if set in the state object passed in as a parameter to the FormControl constructor.

Brief illustration of the issue:

issue

Steps to reproduce:
Plunkr: http://plnkr.co/edit/qFRbCc2FI6BDJ7vMp5I7?p=preview

Related code:

this.fg = fb.group({
  ioninput: [{ value: 'Ion Input...', disabled: true }],
  input: [{ value: 'HTML Input', disabled: true }]
});
<form>
  <ion-list [formGroup]="fg">

    <ion-item>
      <ion-label stacked>Ion Input</ion-label>
      <ion-input type="text" formControlName="ioninput"></ion-input>
    </ion-item>

    <ion-item>
      <input type="text" formControlName="input" />
    </ion-item>

  </ion-list>
</form>

Other information:

Ionic info: (run ionic info from a terminal/cmd prompt and paste output below):

Cordova CLI: 6.4.0 
Ionic Framework Version: 2.0.0-rc.4
Ionic CLI Version: 2.1.18
Ionic App Lib Version: 2.1.9
Ionic App Scripts Version: 0.0.48
ios-deploy version: 1.9.0 
ios-sim version: 5.0.8 
OS: macOS Sierra
Node Version: v7.2.1
Xcode version: Xcode 8.2.1 Build version 8C1002

Edit: Fixed the broken link to the animated gif illustrating the issue.

@kelvindart kelvindart changed the title ion-input ignores the disabled attribute when created using a FormControl ion-input ignores the disabled attribute when created using a FormControl in a Reactive Form Dec 30, 2016
@kelvindart
Copy link
Contributor Author

I guess a workaround would be to bind disabled to an input property, e.g.:

<ion-input formControlName="ioninput" [disabled]="isDisabled"></ion-input>

However, if we are to use truly Reactive forms this would be less ideal, and we still receive the Angular console warning above, so I believe this to still be a bug. 😄

@jgw96 jgw96 added the v2 label Jan 3, 2017
@jgw96
Copy link
Contributor

jgw96 commented Jan 3, 2017

Thanks for the very detailed issue, helps with debugging a ton! We will take a look into this, most likely ion-input is just not passing on the disabled attribute to the actual input that it wraps.

@kelvindart
Copy link
Contributor Author

Happy to help :) I figured the more info the better. Great - doesn't sound too too complex (I hope :P).

@mhartington mhartington added this to the 2.0.0-rc.5 milestone Jan 3, 2017
@brandyscarney brandyscarney modified the milestones: 2.0.0-rc.6, 2.0.0-rc.5 Jan 12, 2017
brandyscarney added a commit that referenced this issue Jan 12, 2017
- test modified to disable username and comments on `input/form-inputs`
and then toggle them via a button

see: http://g.recordit.co/RkN510TcHk.gif

fixes #9834
@brandyscarney
Copy link
Member

Thanks for the issue! I submitted a PR for this here: #9994 and I released a nightly version including the fix: [email protected]

Could you try it out and let me know if you find any issues? :)

@kelvindart
Copy link
Contributor Author

Wonderful! :) Thanks so much.

I will update to the nightly and run locally, reporting back my findings.

Thanks for the fast turnaround - great job!

adamdbradley pushed a commit that referenced this issue Jan 13, 2017
- test modified to disable username and comments on `input/form-inputs`
and then toggle them via a button

see: http://g.recordit.co/RkN510TcHk.gif

fixes #9834
@zakton5
Copy link
Contributor

zakton5 commented Jan 16, 2017

Has this also been fixed for other controls such as ion-toggle, ion-textarea, etc.?

@kelvindart
Copy link
Contributor Author

kelvindart commented Jan 16, 2017

Thanks so much for the fix on ion-input @brandyscarney - I can confirm this indeed works in the nightly. 😬

@kelvindart
Copy link
Contributor Author

@zakton5 - it appears one of Mike Hartington of Ionic has raised a bug: #9998. I guess it's a generic ion-... control issue.

@jtushar53
Copy link

jtushar53 commented Jan 17, 2017

hi @brandyscarney , i can confirm the issue still exist on latest nightly [email protected]

here is repo

ionic info :-
Your system information:
Cordova CLI: 6.4.0
Ionic Framework Version: 2.0.0-rc.5-201701131937
Ionic CLI Version: 2.2.1
Ionic App Lib Version: 2.2.0
Ionic App Scripts Version: 1.0.0
ios-deploy version: Not installed
ios-sim version: Not installed
OS: Windows 10
Node Version: v6.9.2
Xcode version: Not installed

@brandyscarney
Copy link
Member

@zakton5 @kelvindart Yes, it should be working for the other controls now: #10006

I'm not sure if a nightly was released after merging that though, so I just released a new one: [email protected]

@brandyscarney
Copy link
Member

@jtushar53 Angular doesn't recommend mixing the disabled attribute and formControl, so this is behaving as designed. See the comment on this issue for more information: angular/angular#11271 (comment)

You would need to do something like this:

this.loginForm = fb.group({
  "username": [{value: "", disabled: true}, Validators.required],
  "password": ["", Validators.required]
});

@jtushar53
Copy link

@brandyscarney thanks its working now. my concern was to enable disable control whenever i want
i acheived this using below code
this.loginForm.controls['username'].enable() or this.loginForm.controls['username'].disable();

brandyscarney added a commit that referenced this issue Jan 18, 2017
- test modified to disable username and comments on `input/form-inputs`
and then toggle them via a button

see: http://g.recordit.co/RkN510TcHk.gif

fixes #9834
@matejamateusz
Copy link

I don't know if it is connected or not to the issue but is it fixing also the 'clearInput' issue when there is formControlName attribute? Thanks in advance #9077

mhartington pushed a commit that referenced this issue Jan 20, 2017
* docs(navctrl): move content from api doc to top-level nav doc

* docs(action sheet): edit and clarify docs

* docs(action sheet): expose ActionSheet and edit docs

* docs(action sheet): make link path relative and refine docs

* docs(action sheet): fix typo

* docs(action sheet): clean up table formatting

* docs(alert): refactor, add examples, and edit API doc

* docs(alert): fix typos

* docs(alert, action sheet): move page transition doc under advanced

* docs(avatar): add usage example and edit doc

* docs(button): refactor docs and add content from component doc

* docs(card): refactor and incorporate samples from component doc

* docs(checkbox): minor edit

* docs(chip): Add note about MD

* docs(icon): refactor and integrate examples from component doc

* docs(app): fix typo

* docs(config): refactor and edit

* docs(datetime): edit

* docs(gesture): add gesture docs

* docs(gesture): further edit doc

* docs(button, datetime, icon): standardize use of attribute vs input property

* docs(button): change wording from attribute to directive

* docs(input): edit docs and move label examples to Label api doc

* docs(input): add ion-item to code sample

* docs(input): add inset iunput sample

* docs(label): edit doc

* docs(label): add descrition of attributes

* docs(list): refactor and edit doc

* docs(list): add basic usage example

* docs(toolbar, header, footer): refactor and edit docs

* docs(tabs): edit and incorporate examples from old component doc

* docs(navctrl): migrate majority of content to top-level navigation controller doc

* chore(img): fix typos

* docs(virtual scroll): remove instance members

* docs(virtual scroll): increase header sizes

* docs(menu): add related links to all menu docs

* docs(nav): update related docs for all nav docs

* docs(navbar): edit and hide some private instance members

* docs(content): add no-bounce to doc

* docs(fab): fix fab demo

closes ionic-team/ionic-site#947

* docs(note): edit

* fix(list): add border to last item in MD list (#9679)

* fix(list): fixes #9619

* chore(list): make my sass legit

* chore(list): remove from ios and wp css

* fix(scroll): handle low duration in scrollTo

* fix(input): only add padding right if it has clear input

fixes #9865

* docs(nav-controller): fix escape char in Lifecycle table

the pipe before `Promise` adds a new column to the table which makes it push the description to a narrow column. Tried to fix it with replacing the pipe by the HTML ASCII pipe char.

* docs(menu-toggle): fix broken link

path correction in description 
old -> nav/NavBar
new -> navbar/Navbar

* docs(demos): add demos from old component docs

* refactor(package.json, demo template, gulp): Update demo build for new multi-demo api docs

* refactor(demos): update demos for new multi-demo api docs

* docs(api): update api docs for new demo file structure

* refactor: improve tree shaking abilities

Internal refactor completed in order to improve tree shaking and dead
code removal. The public API, with an exception to ion-slides, has
stayed the same. However, internally many changes were required so
bundlers could better exclude modules which should not be bundled.
Ultimately most changes resorted to removing references to `window` or
`document`, or a module that referenced one of those.

BREAKING CHANGES

ion-slides was refactored to remove the external dependencies, and
rewritten in TypeScript/ES6 modules to again improve tree shaking
abilities.

* chore(build): run a 'clean' before running 'validate'

run a 'clean' before running 'validate'

* docs(menu): fix typos in menu-controller.ts (#9923)

Typo errors.

* docs(slides): fix docs

* fix(clickblock): add NavOptions.minClickBlockDuration

* feat(clickblock): ability to set a minimum duration for click block

* fix(clickblock): add NavOptions.minClickBlockDuration

* chore(clickblock): comment out test css

* test(app): fix failing clickblock tests (#9938)

* doc(navParams): update links to push and nav
Closes ionic-team/ionic-site#957

* test(clickblock): add new tests for minDuration

* docs(slides): deprecation notice typo's (#9932)

* fix(platform): only set isPortrait when the width/height is set

* test(tabs): fix ngc error in advanced test

* docs(input): add example for ion-textarea

* docs(hide-when): add hide-when demo back

* docs(demos): update colors for API demos

* chore(ionic): release rc.5

* docs(menu): update menu docs
Closes ionic-team/ionic-site#951

* docs(changelog): unsmarten quotes on sw-toolbox dep (#9966)

* docs(avatar): fix typo (#9965)

place to placed

* docs(nav): fix typos in comments for NavControllerBase and ViewController (#9953)

* docs(nav): fix typo

* docs(view): fix typo

* docs(slides): update slides docs (#9968)

Change the `pager` description.
Change the `paginationType` input type.

* chore: add void return value for some callback signatures (#9967)

* chore(noImplicitAny): add noImplicitAny to tsconfig

* feat(slides): expose more options (#9992)

* feat(slides): expose more options
Closes #9988
Exposes slidesPerView and spaceBetween. Also documents how to change
unexposed options

* docs(slides): update advanced usage

* chore: fix incorrect execution of node binaries (#9962)

* chore: fix incorrect execution of node binaries

* Currently gulp runs the `ngc` by manually calling the `./node_modules/.bin/ngc` file (by assuming it's a node script)
  This is not always correct, because often package managers like (npm or yarn) create bash files for the package binaries.

* Using `resolve-bin` to properly determine the path to the *real* node script and then we can assume it's a node file.

Fixes #8341

* Remove extra newline

* feat(slides): add swiper controller (#9983)

* chore(slides): fix errors with noImplicitAny

* fix(checkbox): set disabled state from FormControl

* fix(datetime): set disabled state from FormControl

* fix(range): set disabled state from FormControl

* fix(select): set disabled state from FormControl

* fix(toggle): set disabled state from FormControl

* feat(radio): add disabled to radio-group and support disabled formcontrol

fixes #9998

* feat(slides): add option for paginationBulletRender

Closes #10002

* fix(input): pass disabled down to input when it is set from form

- test modified to disable username and comments on `input/form-inputs`
and then toggle them via a button

see: http://g.recordit.co/RkN510TcHk.gif

fixes #9834

* fix(item-sliding): don't error or allow swipes with no options

Added items with the following use cases:

1. Sliding item without options
2. Sliding item with one set of dynamic options that toggle
3. Sliding item with two options, one dynamic

Removing my code will cause errors in all of the above examples.

Fixes #9914

* fix(nav): fixes #9558 (#9767)

* chore(e2e): rename app-module.ts to app.module.ts for consistency

references #10023

* fix(tap): allow document to be tap polyfilled

Closes #9726

* docs(slides): document spaceBetween and slidesPerView

* Update README.md

* Fix url is not correct after selecting tabs

Update deeplinker's updateNav after tabSwitchEnd so deeplinker gets the
right active navchild.

* fix(slides): allow auto to be passed
Closes #10000. Closes #10037

* docs(tabs): fix description of tabsHideOnSubPages
Closes #10061

* docs(popover): add import to example code
Closes #10053

* docs(nav-controller): fix param name and type

* fix(tabs): ionChange event is dispatched after the switch

* docs(popover): expose popover docs and reconcile with PopoverController

* docs(modal): expose modal docs and reconcile with ModalController

* docs(toast): expose toast docs and reconcile with ToastController

* docs(demos): add missing demos - copied from preview app

* docs(demos): include demos in docs that didn't have one

* fix(alert): input missing id attribute from input options

fixes #9457

* fix(modal): overlay-zindex is not changed in back direction

fixes #9409

* fix(content): unsubscribe from viewCtrl observables after content ins… (#10050)

* fix(content): unsubscribe from observables on destroy

* fix(content): scroll is initialized before subscribing

fixes #9593
fixes #10045

* fix(content): unset viewCtrl subscribers on destroy

* test(icon): app module was being ignored by my global gitignore

* chore(scripts): update e2e prod build to work with ionic-app-scripts (#10083)

* chore(e2e): WIP to add files needed for app-scripts

* chore(e2e): WIP one e2e test building but with errors

* chore(e2e): move e2e.prod to working with app-scripts

move shared functions into util, add support for debug flag when
running e2e.prod / demos.prod which gets passed to app-scripts

* chore(build): fix app-scripts build for all e2e and demos

* chore(e2e): update ionic-angular import path

* chore(build): update dev paths to work with prod

* chore(e2e): get watch working with e2e prod

* docs(scripts): update README for running e2e and demos

closes #8411
closes #10023

* docs(config): fix demo for config page not updating. #9413 (#9418)

* docs(config): update demo for config, add more logs for app-scripts

closes #9413

* docs(demos): fix bugs and integrate multiple demos on api docs

* chore(virtual-scroll): set IterableDiffer to any

Adding this to avoid an angular version issue.

* chore(swiper): fix reference for closure advanced

* chore(scripts): update app-scripts to run via node, remove snapshot clean

* refactor(package.json): remove unneeded script
@wagner18
Copy link

wagner18 commented Mar 22, 2017

So, enable() and disable() works to programmatically control field state when using reactive forms, however @jtushar53, I don't know if you were aware about this, but for those who aren't

When using the disable() FormControl method it will invalidate the whole form.

So if you have to control the state of a field but still need to keep the form as VALID you will have to use some workaround to do so.

There is a discussion on that matter, take a look to more details.
https://github.com/angular/angular/issues/11271

@ionitron-bot
Copy link

ionitron-bot bot commented Sep 3, 2018

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Sep 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants