CSC495-2014 / TeamworkEnglewoodGit

GNU General Public License v2.0
5 stars 15 forks source link

Updated pop up modal and ajax code #163

Closed kwpembrook closed 10 years ago

kwpembrook commented 10 years ago

The updated pop up modal code along with some added CSS to show that the project name/cell is clickable. Still working on the function to organize the projects by date.

kwpembrook commented 10 years ago

The pop up was functioning correctly except for we were still having issues with it actually checking and cloning the projects but that has to do with the server and not this code as of right now. However I will look into it more to double check.

mboie commented 10 years ago

Couldn't those +s be the cause of that issue though? If you are sending it to the incorrect route, it won't ever get to the git clone method and will always return a failure. You see what I am saying?

Do you know for a fact that you are actually making it into the controller and method that you want to? That would tell you if the +s are a problem.

Gotta go to a final now, I would definitely suggest looking into that though because it may end up solving a lot of your troubles.

kwpembrook commented 10 years ago

After some testing the +s are just adding the username and projectname strings that are passed to the pop up call into the urls. They are not the problem because they are correctly passing the username and project name into the url.

mboie commented 10 years ago

Alright. I originally pointed it out because I remembered it being mentioned in the other pull request. if it works and @apotheos has no problem with you using it, I am certainly okay with it as well. Just figured I would mention it.

I have to sign off for a bit now but I will check in again at 4.20 or so to see if you added anything else to the pull request before the 5.00 pm deadline.

kwpembrook commented 10 years ago

Thanks, but this was the only way we were able to get it to work properly when testing other ways. I will be adding some more changes and will also text you once I do so that you will know for sure.

mboie commented 10 years ago

Looks like you fixed the previous issues with your latest commit so I can sign off on this.

mboie commented 10 years ago

I actually just noticed something that is going to need to be changed at some point, I think.

I commented on it in the code.

kwpembrook commented 10 years ago

yes but it does not work with all of my testing without the capstone because that is what it is currently

mboie commented 10 years ago

Would redirecting to the editor page, using the route, work? I believe Will is doing something similar to get to the projects page after checking the authorization.

kwpembrook commented 10 years ago

The JS was not even compiling when we would insert a redirect to the editor page route in his function and disabled the whole pop up aspect as a whole. We tried many ways of doing this and the one in there is the only way we found to work.

mikeholler commented 10 years ago

@mboie, I agree with the capstone thing. There are better ways to do this and it will not work for Teamwork Englewood when they go to install it. It is possible to handle URLs dynamically, so we should investigate that in the future. I'm not going to merge this into upstream/master yet, since it will break in any installation that does not have capstone in the URL.

I will however create a branch upstream/demo that we can share together tomorrow, and I'll merge this into that.

kwpembrook commented 10 years ago

Alright that sounds good and I am willing to help with any further changes so that we can hand off the final product in the best possible shape we can.