-
Notifications
You must be signed in to change notification settings - Fork 17
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
MOTECH-2732 Migrated module install Workflow #39
base: master
Are you sure you want to change the base?
Conversation
shanmukhm
commented
Jul 26, 2016
- Added a new directive for module installation form
- Also migrated 'motech-file-upload' directive to 'motech-common'
$scope.startOnUpload = function () { | ||
if ($scope.startUpload !== true) { | ||
$scope.startUpload = true; | ||
$('.start-on-upload').find('i').removeClass("fa-square-o").addClass('fa-check-square-o'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we remove the CSS manipulation from the controller and either do it in the markup or a directive link function?
@shanmukhm -- I'll admit I don't like either of the libraries you installed (and assume we were using them) malsup/form is pretty unsupported, so lets definitely drop that. I don't love JansyForm, but it works and its supported — and you wrapped the dependency that uses it, so we can change it later if need be (I also like how it looks) (1) There is a layout problem with smaller screens -- basically the submit button for the form is cut off by the screen (and we don't let people sideways scroll) I think the simplest solution would be: I personally like option (c) the best -- but I defer to you if you think making this implementation change is for the best. |
@nickdotreid Actually I thought it should be exactly same as the old one. That is why I used the old ones. I should've informed you before installing new libraries. I think I can try out option (c) and see if it works. |
@shanmukhm I've added a motech-file-upload directive in a separate PR as it was required for both PRs so please use it and resolve any conflicts if they occur. |
Can one of the admins verify this patch? |
@nickdotreid I just have a small question. Should the form be an inline-form or a normal form? |
@shanmukhm I think it will be more clear as a normal form |
fb810be
to
fe8869e
Compare