adamgedney / fullsailsit-in

Full Sail Sit-in Mobile Device Deployment Class project
0 stars 0 forks source link

All of the things I found. #1

Open rachelhigley opened 10 years ago

rachelhigley commented 10 years ago

Menu

I think that you can rewrite your menu to not need a service at all. Since this is more extensive I will tell you about it in class.

totalSitins

You should never use firebase without angularFire. Trust me it makes it harder. I'm not sure exactly what you are doing here but I think that you can do this when they join a class rather then when they go to the classCtrl or DateCtrl.

ClassCtrl

Cookies

You shouldn't need to use cookies. You can use the authorized user from firebase to get this data. Since Firebase handles sessions you will know if a user is already logged in and get the information from there.

Getting class names

Can you not have your API return a array?

Also I don't think you need your classNames array. In your view instead of {{classData.classNames[$index]}} do {{$rootScope.classHash[classData.classAcronyms[$index]]}} (or something along those lines)

DateCtrl

(see notes from ClassCtrl)

$scope.cancelButton, $scope.buttonWidth

is it important to scope these, can they just be static in your code could you dynamicly show two differnt buttons (ng-show, ng-hide) rather then using the same button for different functions? This is more of a preference thing.

var acro = $routeParams.a

is this necessary? Can you make your route param more semantic then a and then just use something $routeParams.acronym everywhere?

Get dates

Seems like this is not a good way to do this. I don't know how your data is coming back but can't your do this as an object rather then a bunch of different arrays? so your object would look something like this:

$scope.classDates = [
{
    day: 
    date:
    time: 
    inst:
}]; 

Then when you repeat you can do date in classDates and your binded variables can look like date.day; then your confirm can look like confirm(date); and that would simplify your modal a lot.

Simple Logic

So, when you only have simple logic to do on click you can do this in the view, you can decide how separate you want your data to be though. So something like $scope.cancel could just be ng-click="showModal = false; showConfirmation = true;" again this is kinda up to you on if you want put that there. I'm okay with it because both of those things are changing the view not really larger logic.

$http

For $http did you try using there $http.get and $http.post methods? Might help to solve your data issues.

tallySitins

There you go again not using angularFire :) Why are you using a timestamp here?

I don't know if you are using this? seems to be only consoles?

loginCtrl

$window.location

Don't use $window.location, use $location.path keeps your in angular and is a much better way.

checkUser

you really hate angularFire don't you? :)

We are going to have to talk about simplifying your whole user flow to make this super easy and much cleaner. Right now you are pulling down all 10,000 user just to find your one.

setUserCookies

see note about cookies in ClassCtrl

getDatetime

Why do you care about the added? and if it is so important just use the one line for a time stamp.

logoutCtrl

When we rewrite we are going to get rid of this entire controller.

main

Tell Chris to let you get rid of karma so you delete this stupid file :)

menuCtrl

not sure the different between this and menuSlider. Either way we will rewrite it.

Views

Why are you including the header/footer on every page rather then putting it outside in the index.html and not having to worry about it?

home.tpl

can't you just change the route to use loginCtrl rather then main?

attended.tpl

instead of ng-show="!showConfirmation" you can do ng-hide="showConfirmation"

adamgedney commented 10 years ago

You're wonderful! Some of those files I'm not using, and yes.... I hate angularfire. :) Let's go over this in lab :)

thank you thank you thank youuuuu!