Skip to content
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

kelsey #31

Open
wants to merge 24 commits into
base: master
Choose a base branch
from
Open

kelsey #31

wants to merge 24 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Jun 5, 2017

ROLODEX

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
What role does the Model play in Backbone? Similar to a model in rails
How do Backbone Views compare to Rails controllers? A backbone view is similar to a rails controller
How do Backbone Events work in comparison to DOM events? A backbone event is defined in the code vs. a DOM event is defined in the browser
What do you think of Backbone in comparison to raw JavaScript & jQuery? I think it's very confusing! Still don't feel like I have a great handle on this and may need more help on this later in the week.
Do you have any recommendations on how we could improve this project for the next cohort?

@CheezItMan
Copy link

ROLODEX

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Lot of commits, and good commit messages.
Comprehension questions Check
Functionality
Created a Contact Model Check
Created a Rolodex Collection Check
Created a ContactView which renders an individual contact Check
ContactView responds to a click event when the user clicks on the contact
RolodexView created which renders the lit of contacts Check
DOM Events handled for creating new Contacts Check
The RolodexView responds to custom Backbone event generated by ContactView to show the modal Nope! Instead you're having the ContactView render a new ContactDetailsView with the location hard-coded. Not the best or most flexible way to do so. You've tightly coupled the app, to quote Metz.
Styling, Foundation grid layout Check
All dynamic content is rendered using an Underscore template Check
Overall Let me just say... love the theme. You probably should look at your design because how you're doing the contact details is not quite the Backbone way. A better way would be with the trigger-listenTo method.

1 similar comment
@CheezItMan
Copy link

ROLODEX

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Lot of commits, and good commit messages.
Comprehension questions Check
Functionality
Created a Contact Model Check
Created a Rolodex Collection Check
Created a ContactView which renders an individual contact Check
ContactView responds to a click event when the user clicks on the contact
RolodexView created which renders the lit of contacts Check
DOM Events handled for creating new Contacts Check
The RolodexView responds to custom Backbone event generated by ContactView to show the modal Nope! Instead you're having the ContactView render a new ContactDetailsView with the location hard-coded. Not the best or most flexible way to do so. You've tightly coupled the app, to quote Metz.
Styling, Foundation grid layout Check
All dynamic content is rendered using an Underscore template Check
Overall Let me just say... love the theme. You probably should look at your design because how you're doing the contact details is not quite the Backbone way. A better way would be with the trigger-listenTo method.

@ghost
Copy link
Author

ghost commented Jun 7, 2017

Hey @CheezItMan - I met with Charles this afternoon and worked with him to get a better understanding of a few backbone concepts I was struggling with. I also updated it so that when the grayscale image turns to color it's an event! Feeling much better about this assignment. (p.s. @Hamled is awesome...and incredibly patient) 👍

@CheezItMan
Copy link

CheezItMan commented Jun 7, 2017 via email

@CheezItMan
Copy link

Cool effect using CSS and toggleClass to do the greyscale effect.

@CheezItMan
Copy link

I like how you now have the DetailsView organized. Nice! You've also broken things out into functions neatly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant