Swe8 instructors create

[dsteen338]

This PR resolves #8.

Contains the code necessary to create a course object.

[dsteen338]

Could someone lend some feedback about the labels that I have there right now? I feel like they're not the best but I don't know what to change them to

[dsteen338]

I definitely clicked the wrong button and closed this one by accident sorry hahahaha

[jsmith]

Usually with mdbootstrap forms, I either use a placeholder or a label but not both. What you have in the actual text fields looks good!

[dsteen338]

Updated View

[brandon1024]

@dsteen338 Forgive me if this is a silly question, but isn't a date picker modal supposed to show up when I click on Start or End dates? Doesn't seem to be showing up for me, and it's preventing me from entering a new course.

Also, I'm not sure if this is something that is possible in the time we have, or if we want to do this at all, but it would be super cool if in the instructor view, when we click create course, it shows this form directly in the instructor view. Thoughts? Kinda like this:

Instructor view:

Instructor view after button clicked:

Can I have feedback from @jacsmith21 and @mwalz1 ?

[dsteen338]

@brandon1024 yes, it should be. What's your browser and it's version?

Also, is the timepicker modal appearing?

[brandon1024]

@dsteen338 I'm using Chrome v61.0.3163.100

The timepicker modal is appearing now.. Did you change something or was this some weirdness on my machine?

Also, have you by chance modified anything to do with the nav bar / footer? They looked messed up here, but look fine on develop and other branches.

[jsmith]

I love the look of that form in the instructor view btw! Is that not how it looks at the moment?

[jsmith]

Is there anything left to do before this PR can be merged?

[dsteen338]

@brandon1024 I don't think I touched it.... @jacsmith21, @MackenzieJones and I just tried for an hour to get form input validation working and unfortunately we're having trouble getting the view to easily resolve the validation messages. I think we could kind of make a hacky workaround to display each error at the top of the page though.... Or we could just merge it and create an issue to add form validation at some other point. Thoughts?

EDIT: Alternatively, we could look into some javascript validator so an invalid form wouldn't even reach the server

[jsmith]

Ah that's a good question. I would say merge the PR and open up a new issue as the validation messages aren't essential at the moment.

Also, I like the idea of JavaScript validation!

[brandon1024]

Ahh ok. I'm getting some weirdness on my local where sometimes it shows the date picker, and sometimes it doesn't.

I think perhaps it's best to hold this issue back until the next release. We can make do for now, and it might be best to avoid merging issues into develop this early. Thoughts?

Sorry guys I feel like I'm being a hardass :see_no_evil: Just want everything to be thorough and 100%

[dsteen338]

If we don't need it, then it would be ok. Did you check for console errors when the modal wasn't appearing?

[brandon1024]

Yeah I just looked. Was going to create an issue for it:

[dsteen338]

@brandon1024 I think it's a race condition.... Could you see if moving the script to the bottom of the template file fixes it?

[jsmith]

Putting the JS at the bottom seemed to get rid of the error for me!

[jsmith]

@dsteen338 The datepicker wasn't showing up for me either until I turned off by browsers cache! Since this PR changed the file it's getting all confused

I say we merge the PR since the date picker bug is now resolved 👍

[dsteen338]

@jacsmith21 I'll have to move the script down and then we can merge

[dsteen338]

I'm confused.... currently on develop the fields on the Course object have been completely changed, where did that come from?

[jsmith]

That was me who made changes! I had to change the course object for the template editor.

There shouldn't be too many changes though, mostly additions!

[dsteen338]

@jacsmith21 so should I take your change of making the id a long and then keep both for everything else?

[jsmith]

Yeah keep the long! The CrudRepository actually expects the ids to be long variables

[dsteen338]

OK I think this one is done now! We added the updated version of the css/js files and integrated into the instructor view so you can actually click "add a course" and it'll take you to the form (just a new page... for now) and after saving it'll return you to the instructor view. We also added rows and columns to the form so when you create a course it'll actually have seats and be editable. It doesn't actually create the seat objects though, is that necessary at this point?

Anyway this should be re-reviewed I think

[dsteen338]

@brandon1024 We're trying to get the embedded form to work and we fixed the issue with the submit button acting weird.

[brandon1024]

These changes look good to me! :) @dsteen338 thumbs up this comment if this PR is ready to merge, and I'll merge it in

[dsteen338]

@brandon1024 I added a cancel button to close the form. I was going to use the btn-secondary but it's a purpleish-pink and I think that it's too strong visually to be a secondary button (personal opinion) so I went with blue-gray. See here:

I also incresed the width of the form by 10% so the names because I was finding I had to look over too far to see them

Do those changes seem good?

[brandon1024]

@dsteen338 I really like that, especially the button color. Could we maybe put the cancel button on the left?

[dsteen338]

@brandon1024

[dsteen338]

Dunzo

[brandon1024]

Testing.

[dsteen338]

Oh I actually forgot one last thing... I was thinking of making it so the page didn't refresh after submitting ..

[brandon1024]

No worries!

[brandon1024]

Small UI bug! Simple css z-index fix

[brandon1024]

Closed by accident

[dsteen338]

@brandon1024 That happens because the z-index of the sidebar is 1... I'm not exactly sure how I could modify the overlay be on top of that because I don't directly create the element itself, it's all done by the javascript. It looks like @tonerm43 commited the css to make the z-index of the sidebar 1, any insights why?

EDIT: removing the z-index: 1 on the sidebar doesn't seem to have any adverse affects for now...

[brandon1024]

@dsteen338 I recall Mackenzie added the z-index to make the course list appear above the box shadow on the nav bar. I’d have to look at it though, can’t remember fully. If removing the z-index on the nav bar doesn’t break anything, and everything looks good, then we can do that. Otherwise we will just have to add z-index: 2 to the time picker, which I don’t think would be too hard, even through js.

[dsteen338]

I guess the picker holder inherits (even though I couldn't find that property defined anywhere) the z-index, so adding the greater z-index to the div.instructor-body class fixed it I believe.

[brandon1024]

Looks good! Merging.