ntntnlstdnt / codelab-sinatra-erb

https://github.com
0 stars 0 forks source link

Code Lab Feedback #1

Open ntntnlstdnt opened 9 years ago

ntntnlstdnt commented 9 years ago

Hey @chrisvfritz, can you take a look at this? It's hosted here and meets the following project criteria:

chrisvfritz commented 9 years ago

I see a few things that could be fixed here:

First, the image always shows in 100x150 pixels, no matter what the actual aspect ratio of the picture is. To fix this, I would remove the width="100" and "height="150" from your img tag and instead specify with CSS something like this:

img.profile-pic {
  max-width: 100px;
}

That will make sure that the image doesn't grow beyond 100 pixels wide and allows the height to be dynamic, so it will always have the correct aspect ratio!

Second, while the data in the profile technically shows up on the page, there's no context to let us know what any of this information means - it's just bullet points. This isn't something you have to change, as it's technically correct, but if you'd like, I would add a little more context.

chrisvfritz commented 9 years ago

Almost perfect now! You included the CSS for img.profile-pic, but isn't a profile-pic class on your img element, so that CSS is not applying. Fix that and it'll be ship-worthy! :smiley:

ntntnlstdnt commented 9 years ago

I have made the corrections and improved the indentation.

chrisvfritz commented 9 years ago

Looking good! :shipit: