First Tribute Page on Carl Sagan! Any Feedback is Appreciated!


Great work. The text is responsive but the image is not.

1 Like

Ill see to that problem thankyou!

@saimtiazm I just saw this! I added it here and I’ll review it as soon as I get a chance, hopefully sometime later today. :sunny:

For your portfolio project, feel free to update the first post of the portfolio project topic yourself with a link to your project so we know it’s ready for review or message me when it’s posted and I’ll add it to the first post so others know it’s ready.

@saimtiazm The page looks nice. Great work! :tada: Here are some ideas for improvement:

  • Lines 17-23 have a paragraph element:

    <p>- Carl Edward Sagan (November 9, 1934 – December 20, 1996) was an American astronomer, cosmologist, astrophysicist, astrobiologist, author, science popularizer, and science communicator in astronomy and other natural sciences.<br>
     <br>- He is best known for his work as a science popularizer and communicator. His best known scientific contribution is research on extraterrestrial life, including experimental demonstration of the production of amino acids from basic chemicals
     by radiation.<br>
     <br>- Sagan assembled the first physical messages sent into space: the Pioneer plaque and the Voyager Golden Record, universal messages that could potentially be understood by any extraterrestrial intelligence that might find them. Sagan argued the
     now accepted hypothesis that the high surface temperatures of Venus can be attributed to and calculated using the Greenhouse Effect.</p>

    It works the way you have it, however, I think it will be more descriptive if you use the unordered list (<ul>) element and list item elements (<li>) since the content is an unordered list. Then, instead of selecting the paragraph element in your CSS, you can select the unordered list and list item elements to get the style you want.

  • I see you have some JavaScript:

    document.getElementsByTagName("h1")[0].style.fontSize = "80px";
    document.getElementsByTagName("h3")[0].style.fontSize = "40px";
    document.getElementsByTagName("p")[0].style.fontSize = "18px";
    document.getElementsByTagName("p")[1].style.fontSize = "20px";

    I’m relatively new to JavaScript so from what I can tell, this code changes the font size of various HTML tags. Why did you decide to do this via JavaScript instead of doing it in CSS?

  • Your image doesn’t have an alt attribute. It’s very important for every image to have an alt attribute, even if it’s empty alt="". Here’s more information about alt attributes and their importance to accessibilty.

  • I like your color choices, however, your anchor element color (the default) doesn’t contrast well with the background color. This tool is really helpful when choosing colors so that as many people as possible can see them and read the content.

  • I notice you’re using inline styling for your image. While this does work, some argue against it because it blurs the separation between design and content. Here’s an article that goes deeper into this if you’re interested.

  • Like @juanritos mentions, the image is not responsive. To fix this, you can set the width of the image to 100% and the height of the polaroid class to auto.

Let me know if I can help you fix any of the above issues. Other than that, you did well here! :sunny: