-
Notifications
You must be signed in to change notification settings - Fork 14
Kodys angular books #8
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
base: master
Are you sure you want to change the base?
Changes from all commits
d64d490
158f32d
2e8d9c3
5251bd4
82f2e87
ab5c71b
c0777b7
6e105bc
1a2242b
375588c
ab701f1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| node_modules |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| <!DOCTYPE html> | ||
| <html ng-app="booksApp"> | ||
| <head> | ||
| <title>books</title> | ||
| <link rel="stylesheet" type="text/css" href="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.7/css/bootstrap.min.css"> | ||
| <link rel="stylesheet" type="text/css" href="/public/styles/styles.css"> | ||
|
|
||
| <script src="https://ajax.googleapis.com/ajax/libs/angularjs/1.6.2/angular.min.js"></script> | ||
| <script src="http://ajax.googleapis.com/ajax/libs/angularjs/1.6.2/angular-route.js"></script> | ||
|
|
||
| <script src="/public/scripts/app.js"></script> | ||
| <script src="/public/scripts/controllers/booksController.js"></script> | ||
| <script src="/public/scripts/controllers/oneBookController.js"></script> | ||
| </head> | ||
| <body> | ||
| <h2>Bunch O' Books</h2> | ||
| <div ng-view></div> | ||
| </body> | ||
| </html> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| { | ||
| "name": "angular-books-crud-lab", | ||
| "version": "1.0.0", | ||
| "description": "<!-- Location: SF -->", | ||
| "main": "index.js", | ||
| "dependencies": { | ||
| "angular-route": "^1.6.2", | ||
| "bower": "^1.8.0", | ||
| "mongoose": "^4.8.4" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think you actually use these dependencies, and you should've been using budo rather than node. Why do you have a package.json file? |
||
| }, | ||
| "devDependencies": {}, | ||
| "scripts": { | ||
| "test": "echo \"Error: no test specified\" && exit 1" | ||
| }, | ||
| "repository": { | ||
| "type": "git", | ||
| "url": "git+https://github.com/klawton1/angular-books-crud-lab.git" | ||
| }, | ||
| "author": "", | ||
| "license": "ISC", | ||
| "bugs": { | ||
| "url": "https://github.com/klawton1/angular-books-crud-lab/issues" | ||
| }, | ||
| "homepage": "https://github.com/klawton1/angular-books-crud-lab#readme" | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| angular | ||
| .module("booksApp", ['ngRoute']) | ||
| .config(config); | ||
|
|
||
| config.$inject = ['$routeProvider', '$locationProvider']; | ||
| function config( $routeProvider, $locationProvider) { | ||
| $routeProvider | ||
| .when('/', { | ||
| templateUrl: '/templates/books.html', | ||
| controller: 'BooksController', | ||
| controllerAs: 'bc' | ||
| }) | ||
| .when('/books/:id',{ | ||
| templateUrl: '/templates/onebook.html', | ||
| controller: 'OneBookController', | ||
| controllerAs: 'obc' | ||
| }) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. semi-colon player! |
||
|
|
||
| $locationProvider.html5Mode({ | ||
| enabled: true, | ||
| requireBase: false | ||
| }); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| angular | ||
| .module("booksApp") | ||
| .controller("BooksController", BooksController); | ||
|
|
||
| BooksController.$inject = ['$http', '$routeParams']; | ||
| function BooksController( $http, $routeParams) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file is using single-space indentation; most of the rest of your files use two-space indentation. Consistency would be nice, and a single space is not enough. |
||
| var vm = this; | ||
| vm.books = []; | ||
| vm.filter = "author"; | ||
| $http({ | ||
| method: "GET", | ||
| url: "https://super-crud.herokuapp.com/books", | ||
| }).then(function Success(json){ | ||
| vm.books = json.data.books; | ||
| }) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. where's the semi-colons, dude? |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| angular | ||
| .module("booksApp") | ||
| .controller("OneBookController",OneBookController); | ||
|
|
||
| OneBookController.$inject = ['$http', '$routeParams', '$location']; | ||
| function OneBookController( $http, $routeParams, $location) { | ||
| var vm = this; | ||
| vm.book = {}; | ||
| vm.editBook = { | ||
| title: "", | ||
| image: "", | ||
| author: "", | ||
| releaseDate: "", | ||
| } | ||
| $http({ | ||
| method: "GET", | ||
| url: "https://super-crud.herokuapp.com/books/" + $routeParams.id, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should use template string rather than concatenation |
||
| }).then(function Success(response){ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why does this function get an uppercase name? Those names are usually reserved for constructor functions. should be success(response) |
||
| vm.book = response.data; | ||
| }) | ||
| vm.makeCopy = function(book){ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. unsure why this was needed, but it doesn't break anything. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can see why this was necessary, but would give it a different name; something like "selectBookToEdit" or "makeEditable" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It even looks like you're trying to call this from your frontend as makeEdit rather than makeCopy :( |
||
| for(key in vm.editBook){ | ||
| vm.editBook[key] = book[key] | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. missing semicolon |
||
| } | ||
| } | ||
| vm.submit = function(book){ | ||
| $http({ | ||
| method: "PUT", | ||
| url: "https://super-crud.herokuapp.com/books/" + book._id, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. concat vs. template |
||
| data: vm.editBook | ||
| }).then(function(res){ | ||
| vm.book = res.data; | ||
| $location.path('/'); | ||
| }) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. missing semicolon |
||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. CAN I PLEASE HAVE A WHITESPACE?! |
||
| vm.delete = function(){ | ||
| $http({ | ||
| method: "DELETE", | ||
| url: "https://super-crud.herokuapp.com/books/" + vm.book._id | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. concat vs. template string |
||
| }).then(function(res){ | ||
| $location.path('/'); | ||
| }) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| .book-cover{ | ||
| height: 275px; | ||
| width: 200px; | ||
| } | ||
| .editing{ | ||
| display: inline; | ||
| } | ||
| .title{ | ||
| width: 550px; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. watch out for hard coded px! This will break on smaller screens |
||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. look up https://bestmotherfucking.website/ for styling tips, but great job styling |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| <div> | ||
| <select ng-model="bc.filter"> | ||
| <option ng-model="bc.filter">author</option> | ||
| <option ng-model="bc.filter">title</option> | ||
| </select> | ||
| <div class="book" ng-repeat="book in bc.books | orderBy: bc.filter" > | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. filter feature?!?! Way to go above and beyond the call of duty, soldier. |
||
| <div> | ||
| <img class="book-cover" ng-src="{{book.image}}"> | ||
| </div> | ||
| <div class="editing"> | ||
| <h3 class="title"><a ng-href="/books/{{book._id}}">{{book.title}}</a></h3> | ||
| <h5>{{book.author}}</h5> | ||
| <h5>{{book.releaseDate}}</h5> | ||
| </div> | ||
| <hr> | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. add a whitespace for god's sake! |
||
| </div> | ||
| </div> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| <div class="book"> | ||
| <div> | ||
| <img class="book-cover" ng-src="{{obc.book.image}}"> | ||
| </div> | ||
| <form class="editing" ng-submit="obc.submit(obc.book)"> | ||
| <h3 class="title"><span ng-hide="edit">{{obc.book.title}}</span><input type="text" name="title" ng-show="edit" ng-model="obc.editBook.title"></h3> | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These are each a lot of things on one line. I try never to have more than one tag in a line; I'd break this up into like 4 lines. |
||
| <h5><span ng-hide="edit">{{obc.book.author}}</span> <input type="text" name="author" ng-show="edit" ng-model="obc.editBook.author"></h5> | ||
| <h5><span ng-hide="edit">{{obc.book.releaseDate}}</span> <input type="text" name="releaseDate" ng-model="obc.editBook.releaseDate" ng-show="edit"></h5> | ||
| <button class="btn btn-default" ng-show="edit" ng-click="edit=false" type="submit">Save</button> | ||
| </form> | ||
| <button class="btn btn-default" ng-show="edit" ng-click="edit=false; obc.makeEdit(obc.book)">Cancel</button> | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Where you do 2 things in these ng-click callbacks, it makes more sense to include that line in the function you're calling (i.e., just say |
||
| <button class="btn btn-default" ng-click="obc.makeCopy(obc.book); edit=true" ng-hide="edit" >Edit</button> | ||
| <hr> | ||
| <button class="btn btn-default" ng-click="obc.delete()">Delete</button> | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. try to read this and only breathe in only on whitespaces, you can't can you? CAN YOU?! |
||
| </div> | ||
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.
add a whitespace line for better reading, brosky