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

replace angular.uppercase with String.prototype.toUpperCase() (support/0.x) #978

Merged
merged 1 commit into from
Jun 25, 2018

Conversation

rthaut
Copy link
Contributor

@rthaut rthaut commented Jun 24, 2018

Description

Per @Anthropic's request in my PR for the master branch, this PR targets the support/0.x branch.

This PR replaces the use of angular.uppercase with String.prototype.toUpperCase(), as AngularJS has removed their helper functions.

Fixes Related issues

Checklist

  • I have read and understand the CONTRIBUTIONS.md file
  • I have searched for and linked related issues
  • I have created test cases to ensure quick resolution of the PR is easier
  • I am NOT targeting main branch
  • I did NOT include the dist folder in my PR

@json-schema-form/angular-schema-form-lead

@rthaut
Copy link
Contributor Author

rthaut commented Jun 24, 2018

@Anthropic I tested this on the support/0.x branch, BUT I had to make a few extra changes to get the tests to run on my end. I'm assuming you don't want those to be part of this PR (so they are not included here now), but you can see what I had to do here on my fork of the master branch.

Basically, many of the development dependencies are substantially out-dated, so I had to update them to get karma to actually run. Additionally, with Bootstrap 4 (compared to Bootstrap 3), the DOM structure for input elements is a little different, and the has-error and has-success classes are added to the input group, so the selector used in the test cases had to be adjusted. The alternative solution would be to lock Boostrap to v3.x in the bower.json file(s); I think that change would need to be made in json-schema-form/angular-schema-form-bootstrap, but I didn't actually track it down through the dependency tree.

Let me know if you want a PR that includes those fixes (or if you want them included on this PR), and, if so, which branch(es) you want me to target. Also, if there is a better way to discuss and/or handle this, I'm happy to oblige.

@Anthropic
Copy link
Member

@rthaut champion, thank you very much, you can ping me via Gitter IM and I can give you my email or just communicate there. I really appreciate your efforts. I'm working in the swappable validators branch and I'm stuck on a few things that have kept me away from touching any other branches (that and two separate month long holidays this year).
If you can put those changes in another PR I am more than willing to accept them (I always prefer separated PRs for clarity of purpose), not having the time to get the validation working on that branch has been such a pain stalling my ability to do quick fixes and they are building up for quite some time now.
I have had users ask about bootstrap 4, I don't really use it so wasn't sure what was involved, would like to discuss that with you and see if we can update the bootstrap repo at some point too.

@Anthropic Anthropic merged commit e4201b7 into json-schema-form:support/0.x Jun 25, 2018
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

Successfully merging this pull request may close these issues.

2 participants