Please critique my Shigetaka Kurita tribute page

Hey! I just completed my first project on freeCodeCamp and am looking forward to sharing so I can get some opinions on what I did well and what I can do better.

The more feedback the better so bring it on!

If I may self-critique I would say that my code is a little on the verbose side. I also had to find workarounds to the freeCodeCamp requirements to achieve the design I was going for. I hope that’s allowed, please ignore the empty code that I put in those spots.

https://codepen.io/ferncode/pen/abdpmzX

hi @Fran

page looks good, great work.
i just want to remind you, that you forgot to closed the
@media only screen and (max-width: 600px) query.

try using the codepen css analyze tool, and you know what i mentioned.

1 Like

Didn’t even know that was a codepen feature, fixed that now. Thanks for taking the time to look at my page!

Hi Fran, overall the page looks good. I’m going to suggest a couple of things to make you aware of for future projects.

  • The white text on that color background does technically not have enough color contrast and is thus not accessible to people with color vision issues. You can check color accessibility on the following website:
    https://webaim.org/resources/contrastchecker/
  • When I narrow my browser all the way the two emojis start to drift left and the ascii one actually starts to get cut-off the page. While this is not a huge issue, for a perfectionist like me it would drive me crazy :slight_smile: When styling your pages, you should start with the browser as narrow as you can make it and style to make sure everything looks good. Then you can slowly widen the browser and add CSS break points if necessary to adjust the style for a wider view port. Set the break point using min-width and use ‘em’ units for the value (discussed below). Always start skinny and then work your way wider.
  • Another thing you will always want to check is that your page handles increases in text size gracefully. Currently, as I increase the text size the ‘Read More’ text on the button at the bottom breaks out of the button container. If you don’t know how to manually increase the text size, using Firefox, go to the ‘View->Zoom’ menu and activate ‘Zoom Text Only’. Then while holding down the Ctrl key scroll your middle mouse button to increase the text size. As mentioned above, if you use ‘em’ units for your CSS break points then your page will automatically be responsive to changes in text size. To ballpark the ‘em’ value just divide the pixel value by 16. So if you want to set a break point at 600px then you would actually set it at 37.5em.
  • Speaking of that button at the bottom, you’ve got it wrapped in an <a> tag. You’ll need to choose one or the other but not both. Because that is really a link to another page it should probably be an <a>, so you can lose the <button> completely. Also, you’ll want the text to be more descriptive then “Read More”. See the following for a discussion of making link text accessible:
    https://webaim.org/techniques/hypertext/
    Particularly the ‘Screen Readers and Links’ section.

Thank you so much for this comprehensive feedback :slight_smile:

  • I realized too late that I should have been writing my page mobile-first (or smallest width first) as you said, this is great advice that I’m definitely going to keep at the front of my mind for future projects.

  • Accessibility is important to me and I can’t believe I forgot to check my contrast ratios, I’ve gone ahead and fixed that. I also read the webaim.org article on links that you mentioned and I’ve updated my button to be more descriptive and took out the ‘button’ tag as you recommended.

  • What you said about text sizing makes some sense to me. I had no idea you could manually change font size using the scroll button on the mouse so that was helpful to learn. I did not realize I could set breakpoints in em (for some reason I thought it was reserved only for sizing text). I’ll definitely play around with that, but I do need to do some further learning/reading to get a better grasp on how exactly em units work since I only understand them as far as they can make text bigger or smaller right now. If you have any helpful ways of thinking about em please let me know.

Thank you again for your feedback and advice, highly appreciated!

Your page looks good @Fran. Some things to revisit;

  • Keep the test script when forking the pen (<script src="https://cdn.freecodecamp.org/testable-projects-fcc/v1/bundle.js"></script>).
    • The test script should be included, with all tests passing, when you submit your projects.
    • Your page passes 9/10 user stories. Click the red button to see which test(s) are failing and text to help you correct the issue. Hint: multiple class names are okay, not so with an id attribute
  • Codepen provides the boilerplate for you. It only expects the code you’d put within the body element in HTML. (No need to include the body tags). For anything you want to add to the <head> element click on the ‘Settings’ button, then HTML and add it into the ‘Stuff for <head>’ box.
  • Run your HTML code through the W3C validator.
    • Since copy/paste from codepen you can ignore the first warning and first two errors.
    • There are HTML coding errors you should address.
  • Since the dates are bold and there is sufficient spacing between each maybe think about getting rid of the bullets in the tribute info. (Just a suggestion)
1 Like