6
\$\begingroup\$

I've got a goal and that goal is to become an Angular expert. So what better way of learning than to create an online store for pies, right? I've just finished the registration part and would like some feedback on it.

The registration process happens via Firebase's AngularFire, I decided to use Firebase because I've heard that it works great with Angular and I really want to become good at both of these frameworks. I use the AuthWithPassword feature as the authentication when logging in. And I store some personal info in a JSON format in Firebase to use at the checkout at a later point.

Now what I need some feedback on is the following:

  • I've split my controllers up into multiple ones to handle different logic across the application. Should I split this controller up even more since it's going to be even bigger once I add the login functionality to it? I've heard that this is the way to go but it also seems silly to put everything in a different controller because in the end it feels like it's going to get more messy than it has to.
  • Can I "angular-ify" my markup to make it better in any way?
  • Anything in general that I'm doing wrong or should improve to following best practices.

The registration template:

<div id="register-form-wrapper" class="rounded-corners box-shadow">
    <h1>Fill in your information</h1>
    <form method="post" id="register" name="register" novalidate>
        <table>
            <tr>
                <td><label for="firstname">Firstname:</label></td>
                <td><input type="text" name="firstname" alt="Enter your firstname" placeholder="Firstname" 
                           id="firstname" class="input rounded-corners box-shadow" value="Christoffer"></td>
                <td><span>*</span></td>
            </tr>
            <tr>
                <td><label for="lastname">Lastname:</label></td>
                <td><input type="text" name="lastname" alt="Enter your firstname" placeholder="Lastname" 
                           id="lastname" class="input rounded-corners box-shadow" value="Woody"></td>
                <td><span>*</span></td>
            </tr>
            <tr>
                <td><label for="street-address">Street address:</label></td>
                <td><input type="text" name="street_address" alt="Enter your street address"
                           placeholder="Street address" 
                           id="street-address" class="input rounded-corners box-shadow" value="Blabla 3"></td>
                <td><span>*</span></td>
            </tr>
            <tr>
                <td><label for="city">City:</label></td>
                <td><input type="text" name="city" alt="Enter your city" placeholder="City" 
                           id="city" class="input rounded-corners box-shadow" value="Stockholm"></td>
                <td><span>*</span></td>
            </tr>
            <tr>
                <td><label for="zip">ZIP/Postal code:</label></td>
                <td><input type="text" name="zip" alt="Enter your zip/postal code" 
                           placeholder="ZIP/Postal code"
                           id="zip" class="input rounded-corners box-shadow" value="12398"></td>
                <td><span>*</span></td>
            </tr>
            <tr>
                <td><label for="country">Country:</label></td>
                <td><input type="text" name="country" alt="Enter your country" placeholder="Country" 
                           id="country" class="input rounded-corners box-shadow" value="Sweden"></td>
                <td><span>*</span></td>
            </tr>
            <tr>
                <td><label for="email">Email:</label></td>
                <td><input type="email" name="email" alt="Enter your email" placeholder="Email" 
                           id="email" class="input rounded-corners box-shadow" value="testdata@gmail.com"></td>
                <td><span>*</span></td>
            </tr>
            <tr>
                <td><label for="password">Password:</label></td>
                <td><input type="password" name="password" alt="password" placeholder="Password" 
                           id="password" class="input rounded-corners box-shadow" value="mamma12"></td>
                <td><span>*</span></td>
            </tr>
            <tr>
                <td><label for="confirmed-password">Confirm password:</label></td>
                <td><input type="password" name="confirmed_password" alt="Confirm your password" 
                           placeholder="Confirm password" 
                           id="confirmed-password" class="input rounded-corners box-shadow" value="mamma12"></td>
                <td><span>*</span></td>
            </tr>
        </table>
    </form>
    <div id="error-wrapper">
        <h5 ng-show="isEmpty" class="error" ng-bind="errors[0]"></h5>
        <h5 ng-show="firebaseError" class="error" ng-bind="errors[1]"></h5>
        <h5 ng-show="emailTakenError" class="error" ng-bind="errors[2]"></h5>
        <h5 ng-show="passwordMismatchError" class="error" ng-bind="errors[3]"></h5>
        <h5 ng-show="isSuccessful" class="success" ng-bind="errors[4]"></h5>
    </div>
    <div id="form-btn-wrapper">
        <input type="button" form="register" name="register-submit" value="Register" 
               id="register-submit" class="btn" ng-click="validate()">
    </div>
</div>

The account management controller:

/** Controller that handles all logic regarding accounts **/

pieShopApp.controller('accountsCtrl', ['$scope', '$firebase', '$firebaseAuth', function ($scope, $firebase, $firebaseAuth) {

    $scope.firebaseUsers = new Firebase ('https://pie-shop.firebaseio.com/users/');
    $scope.firebaseSync = $firebase($scope.firebaseUsers);

    $scope.errors = ['Failed to create account, please check your information and try again.', 
                     'Something went wrong when trying to create your account, please try again later.', 
                     'Email is already taken, please choose another valid email.',
                     'The passwords do not match',
                     'Success!'
                    ];

    $scope.isEmpty = false;
    $scope.firebaseError = false;
    $scope.invalidEmailError = false;
    $scope.emailTakenError = false;
    $scope.passwordMismatchError = false;
    $scope.isSuccessful = false;

    $scope.validate = function() {

        var isValid = false;

        var userInfo = {
            firstname: $('#firstname').val(),
            lastname: $('#lastname').val(),
            streetAddress: $('#street-address').val(),
            city: $('#city').val(),
            zip: $('#zip').val(),
            country: $('#country').val(),
            email: $('#email').val(),
            password: $('#password').val(),
            confirmedPassword: $('#confirmed-password').val()
        }

        if (userInfo.password != userInfo.confirmedPassword) {
            $scope.passwordMismatchError = true;
        }

        for (var info in userInfo) {

            if (userInfo[info] === "") {
                $scope.isEmpty = true;
                return false;
            }

            isValid = true;
        }

        if (isValid) {
            $scope.registerNewUser(userInfo);
        }
    }

    $scope.registerNewUser = function(userInfo) {

        $scope.firebaseUsers.createUser({
            email    : userInfo.email,
            password : userInfo.password
        }, function(error, user) {
            if (error === null) {
                $scope.$apply(function() {
                    $scope.isSuccessful = true;
                    $scope.storeUserInfo(userInfo, user);
                })
            }
            else {

                $scope.$apply(function() {
                    switch (error.code) {

                        case 'EMAIL_TAKEN': $scope.emailTakenError = true; break;

                        case 'INVALID_EMAIL': $scope.invalidEmailError = true; break;

                        default: $scope.firebaseError = true; break;
                    }
                });
            }
        });
    }

    $scope.storeUserInfo = function(userInfo, user) {

        $scope.firebaseUsers = new Firebase ('https://pie-shop.firebaseio.com/users/' + user.uid);

        $scope.firebaseSync.$set({firstname: userInfo.firstname,
                                  lastname: userInfo.lastname,
                                  streetAddress: userInfo.streetAddress,
                                  city: userInfo.city,
                                  zip: userInfo.zip,
                                  country: userInfo.country
                                 });
    }
}]);
\$\endgroup\$
3
\$\begingroup\$

Nice project, I hope by helping out will eventually get some free pie? Anyway, one pretty big thing about why I like angular is the two way data binding. In this example you set up your form and referencing it with jQuery .val() to get the values and build your user object. Instead use the ng-model directive and you user object will create itself! :)

e.g:

<input type="email" ng-model="userinfo.email" alt="Enter your email" placeholder="Email" 
                       id="email" class="input rounded-corners box-shadow">

In your controller you will then have access to the userinfo obejct via $scope.userinfo, no need for jQuery!

Moving on to the validation and the displaying of error messages, instead of writing out every possible case in your markup and then displaying it via the controller I would let the controller do all the work and just tell the template to render errors. E.g I would create an array 'errors' which I would bind to the scope so it becomes accesible from the view. In the template I would simply do:

<h5 ng-repeat="error in errors" class="error"> {{ error }} </h5>

Your controller would then be responsible to push the error messages to the errrors array. But dont forget to reset it before you validate! You dont want last times error messages :)

Thats it for me!

\$\endgroup\$
  • \$\begingroup\$ I like the tips you gave, they've worked great so far. Only problem I have now is that my "for in" loop doesn't loop over the info anymore, even when I tell them not to be empty strings from the start. It only goes over the ones that actually contain something. Any idea how to fix this? \$\endgroup\$ – Chrillewoodz Jan 29 '15 at 12:13
  • \$\begingroup\$ I would suggest you use angulars validation directive to set which fields are required instead of writing your own (since its pretty much 'this field is required'). Take a look at this: ng-newsletter.com/posts/validations.html Or you could set up a plunker and Ill have a look at it! @Chrillewoodz \$\endgroup\$ – Erik Svedin Jan 30 '15 at 7:28

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.