I sure this is crap, but it's my first try so lay off ;)

Code Feedback
1

I’d like to hear what folks have to say. Thanks in advance for your feedback.

2

Its not crap. I like your simple approach. But i guess color choices for header could be better. Always use colors which are easy on eyes (like material colors). e.g. your intro paragraph looks very pretty.

2 Likes
3

I used the blue, red, and white because of the Russian flag, and gold for his medals. Thank you for your feedback, I really appreciate it!

4

Hey there,

Looks decent for a first project. The one thing that stands out is that the text-box is centered by giving it a fixed margin (left and right 500px) This can work fine, but on my smaller screen it makes the box look all squished. Ideally you’d avoid hard-coded px values.

Centering elements can get pretty difficult but you can look at giving it a percentage margin along with a minimum-width or maybe using css flexbox to make it more responsive.

Outside of that it looks good, try to challenge yourself and build a more complex site next, something with a menu or multiple images. Keep up the good work

1 Like
5

Hi @pizza_danger,

HTML inspector:

  • The <ul> element cannot be a child of the <ul> element.

MDN documentation:
<ul>: The Unordered List element - HTML: HyperText Markup Language | MDN

Permitted content
zero or more <li> elements, eventually mixed with <ol> and <ul> elements.

<ul class="text-box">
  <li>...</li>
  <li>...</li>
  <li>...</li>
  <li>...</li>
  <li>His Greco-Roman Wrestling achivements include:</li>
  <ul>
    <li>3 Olympic Gold Medals</li>
    <li>9 World Championships</li>
    <li>12 European Championships</li>
  </ul>
  • If you want a nested list:
<ul>
  <li>first item</li>
  <li>second item     
  <!-- Look, the closing </li> tag is not placed here! -->
    <ul>
      <li>second item first subitem</li>
      <li>second item second subitem
      <!-- Same for the second nested unordered list! -->
        <ul>
          <li>second item second subitem first sub-subitem</li>
          <li>second item second subitem second sub-subitem</li>
          <li>second item second subitem third sub-subitem</li>
        </ul>
      </li> <!-- Closing </li> tag for the li that
                  contains the third unordered list -->
      <li>second item third subitem</li>
    </ul>
  <!-- Here is the closing </li> tag -->
  </li>
  <li>third item</li>
</ul>
  • first item
  • second item
    • second item first subitem
    • second item second subitem
      • second item second subitem first sub-subitem
      • second item second subitem second sub-subitem
      • second item second subitem third sub-subitem
    • second item third subitem
  • third item
---

Is better not use the style attribute:

<p style="color: #ff0000">To learn more about Aleksandr Karelin ... </p>

Because, you have multiple styles sources (That’s make the page more difficult to review.):

  • The CSS tab
  • Every element on the html with the style attribute

MDN documentation:
style - HTML: HyperText Markup Language | MDN

The style global attribute contains CSS styling declarations to be applied to the element. Note that it is recommended for styles to be defined in a separate file or files. This attribute and the <style> element have mainly the purpose of allowing for quick styling, for example for testing purposes.

---

Cheers and happy coding :slight_smile:

1 Like