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

Consistent ionic NgModel components #8578

Closed
9 tasks
manucorporat opened this issue Oct 8, 2016 · 11 comments · Fixed by #10898
Closed
9 tasks

Consistent ionic NgModel components #8578

manucorporat opened this issue Oct 8, 2016 · 11 comments · Fixed by #10898
Assignees

Comments

@manucorporat
Copy link
Contributor

manucorporat commented Oct 8, 2016

We should revisit:

  • checkbox
  • date-time
  • input
  • text-area
  • radio-group
  • searchbar
  • segment
  • selector
  • toggle

and events like (ionChange), (ionSelect) in across the framework: Tabs, SplitPane, SlidingItem...

Each one has a different policy when calling the callbacks.

  • Some does not call onChange() when the value is changed from writeValue
  • Some does fire the callbacks with the default value (others not)
  • In general all these components are doing the same thing but from an implementation detail they are not consistent.

Examples:
Toggle: https://github.com/driftyco/ionic/blob/master/src/components/toggle/toggle.ts#L230-L246
Range: https://github.com/driftyco/ionic/blob/master/src/components/range/range.ts#L620-L649

I propose to create a new design document to describe how an ionic component should work through a base interface and a standarized behavior, we could even create "base unit tests" shared between all the inputs.

ping
@adamdbradley
@brandyscarney

I think this should be approached before ionic 2.0 final is released. Probably in 2.0rc2

@adamdbradley
Copy link
Contributor

I started using abstract classes here for each of our input types, we can use this to come up with a standard. https://github.com/driftyco/ionic/blob/master/src/util/form.ts#L66

@jgw96 jgw96 removed this from the 2.0.0-rc.4 milestone Dec 2, 2016
@manucorporat
Copy link
Contributor Author

#9472

@manucorporat
Copy link
Contributor Author

manucorporat commented Dec 5, 2016

Ionic inputs should not break if userland code crashes. i.e. we should wrap calls to user's code with try{} catch

Pseudocode:

dispatchEvent() {
   try {
     this.userland();
   }catch() {
     // restore state and rethrow 
   }
}

@manucorporat
Copy link
Contributor Author

#10532

@manucorporat
Copy link
Contributor Author

manucorporat commented Mar 15, 2017

All inputs to implement ionFocus and ionBlur
Related #10761

@manucorporat
Copy link
Contributor Author

manucorporat commented Mar 18, 2017

All events in ionic should behave the same:
(ionSelect), (ionChange), (ionInput)...

manucorporat added a commit that referenced this issue Mar 21, 2017
manucorporat added a commit that referenced this issue Mar 21, 2017
manucorporat added a commit that referenced this issue Mar 23, 2017
manucorporat added a commit that referenced this issue Mar 23, 2017
manucorporat added a commit that referenced this issue Mar 26, 2017
manucorporat added a commit that referenced this issue Mar 26, 2017
manucorporat added a commit that referenced this issue Mar 28, 2017
@manucorporat
Copy link
Contributor Author

ng-valid handling inconsistency: #10952

@manucorporat
Copy link
Contributor Author

#11005

@manucorporat
Copy link
Contributor Author

#7806

@manucorporat
Copy link
Contributor Author

#8935 (comment)

@ionitron-bot
Copy link

ionitron-bot bot commented Apr 5, 2018

Thanks for the issue! This issue is being closed due to inactivity. 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.

Thank you for using Ionic!

@ionitron-bot ionitron-bot bot closed this as completed Apr 5, 2018
@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Apr 5, 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.

5 participants