Kodys angular books #8
Conversation
| }).then(function Success(response){ | ||
| vm.book = response.data; | ||
| }) | ||
| vm.makeCopy = function(book){ |
There was a problem hiding this comment.
unsure why this was needed, but it doesn't break anything.
There was a problem hiding this comment.
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.
It even looks like you're trying to call this from your frontend as makeEdit rather than makeCopy :(
| } | ||
| .title{ | ||
| width: 550px; | ||
| } |
There was a problem hiding this comment.
look up https://bestmotherfucking.website/ for styling tips, but great job styling
| <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.
filter feature?!?! Way to go above and beyond the call of duty, soldier.
| display: inline; | ||
| } | ||
| .title{ | ||
| width: 550px; |
There was a problem hiding this comment.
watch out for hard coded px! This will break on smaller screens
| <script src="/public/scripts/app.js"></script> | ||
| <script src="/public/scripts/controllers/booksController.js"></script> | ||
| <script src="/public/scripts/controllers/oneBookController.js"></script> | ||
| </head> |
There was a problem hiding this comment.
add a whitespace line for better reading, brosky
| templateUrl: '/templates/onebook.html', | ||
| controller: 'OneBookController', | ||
| controllerAs: 'obc' | ||
| }) |
| url: "https://super-crud.herokuapp.com/books", | ||
| }).then(function Success(json){ | ||
| vm.books = json.data.books; | ||
| }) |
| <h5>{{book.author}}</h5> | ||
| <h5>{{book.releaseDate}}</h5> | ||
| </div> | ||
| <hr> |
| <button class="btn btn-default" ng-show="edit" ng-click="edit=false; obc.makeEdit(obc.book)">Cancel</button> | ||
| <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.
try to read this and only breathe in only on whitespaces, you can't can you? CAN YOU?!
| vm.book = res.data; | ||
| $location.path('/'); | ||
| }) | ||
| } |
mnfmnfm
left a comment
mnfmnfm
left a comment
There was a problem hiding this comment.
Overall looks good! I'm guessing you have a bug where editing isn't quite working as you'd hoped, because you call the function two different things in two different places. But your angular is generally well-written.
| } | ||
| $http({ | ||
| method: "GET", | ||
| url: "https://super-crud.herokuapp.com/books/" + $routeParams.id, |
There was a problem hiding this comment.
Should use template string rather than concatenation
| $http({ | ||
| method: "GET", | ||
| url: "https://super-crud.herokuapp.com/books/" + $routeParams.id, | ||
| }).then(function Success(response){ |
There was a problem hiding this comment.
Why does this function get an uppercase name? Those names are usually reserved for constructor functions. should be success(response)
| }) | ||
| vm.makeCopy = function(book){ | ||
| for(key in vm.editBook){ | ||
| vm.editBook[key] = book[key] |
| }).then(function Success(response){ | ||
| vm.book = response.data; | ||
| }) | ||
| vm.makeCopy = function(book){ |
There was a problem hiding this comment.
I can see why this was necessary, but would give it a different name; something like "selectBookToEdit" or "makeEditable"
| vm.submit = function(book){ | ||
| $http({ | ||
| method: "PUT", | ||
| url: "https://super-crud.herokuapp.com/books/" + book._id, |
| <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.
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.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.
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 obc.makeEdit(obc.book), make all your ng-show listen to obc.edit, and then in your definition of makeEdit set this.edit = false; similar in other places).
| }).then(function Success(response){ | ||
| vm.book = response.data; | ||
| }) | ||
| vm.makeCopy = function(book){ |
There was a problem hiding this comment.
It even looks like you're trying to call this from your frontend as makeEdit rather than makeCopy :(
| "dependencies": { | ||
| "angular-route": "^1.6.2", | ||
| "bower": "^1.8.0", | ||
| "mongoose": "^4.8.4" |
There was a problem hiding this comment.
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?
| .controller("BooksController", BooksController); | ||
|
|
||
| BooksController.$inject = ['$http', '$routeParams']; | ||
| function BooksController( $http, $routeParams) { |
There was a problem hiding this comment.
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.
3 participants
No description provided.