Tribute Page (to N.K. Jemisin) feedback please :)

[jinwithwings]

Hey FCC community, this is my first post - yet another new coder seeking feedback on my first completed project! It’s a tribute page to fantasy author N.K. Jemisin, and it’s pretty simple but I’m fairly proud of it.

NK Jemisin Tribute Page

Would love any feedback you might have on ways to improve, streamline, or clarify the code. My CSS organization is still a bit wobbly…

[Philip_B]

Hi, it looks nice!

I think you can adjust the wrapping of your flex a little bit.

For example, if you narrow your page, the box goes inside the text.

So I added; -row to you #tribute-info

(display: flex-row;)

and already your text stayed within your box, but its not perfect yet…

[jwilkins.oboe]

Hi @jinwithwings !

I think your page looks good.

Just a couple of small things I noticed.

For google, fonts you would place it in the head section and not the header section.

      <link rel="stylesheet" href="https://fonts.googleapis.com/css2?family=Archivo">

When it comes to working with codepen, there is a section to place google fonts and other links for the head.

@jinwithwings

My CSS organization is still a bit wobbly…

I think overall it is pretty easy to follow.

You could get rid of this empty selector here

main {
}

You have a little bit of repetition here with the font-weight and font-family

h2 {
  font-size: 26px;
  font-family: "Archivo", sans-serif;
  font-weight: 300;
}

h3 {
  font-size: 20px;
  font-family: "Archivo", sans-serif;
  font-weight: 300;
}

You could use a class instead for those css properties to cut out the repetition.

Congrats on your first project!

[jinwithwings]

Ooh, thank you for catching that! I am guessing I need to probably do something with flex-wrap… will look more this. Appreciate the feedback.

[jinwithwings]

Thanks for the encouragement and feedback! I appreciate you pointing out the head/header discrepancy, and the suggestion to use a class for h2 and h3 (the latter of which I added last minute, haha). I’ll work on those modifications to the code

[jinwithwings]

Hi there! I couldn’t find a flex property that would keep the border outside the content, so I ended up just adding an overflow (hidden) to #tribute-info. It’s not the most elegant solution, however. What do you think?

If you (or anyone else) have any other thoughts or ideas for actually making the text shrink accordingly with the border no matter what, I’d love to hear them! Thanks again.

[lasjorg]

The ul content is overflowing the container, it doesn’t really matter if you hide it or not.

One option would be to use an auto margin and a max-width instead of the fixed margin you have now. At least then the overflow doesn’t happen until the page is pretty narrow.

#tribute-info {
  display: flex;
  justify-content: center;
  border-color: black;
  border-width: 10px;
  border-style: double;
  padding: 20px 20px 30px 15px;
/*   margin: 0px 100px 20px 100px; */
/*   overflow: auto; */

  margin: 0 auto;
  /* adjust as you see fit */
  max-width: 980px;
}

You might also set word-break: break-word on the ul element.

[jinwithwings]

Thanks so much! That’s a way better alternative to getting the results I was looking for. Also, thanks for sharing with me the word-break property

[Philip_B]

Would it help putting a min-width of (100px or another) ?

[lasjorg]

Then the container will overflow the page instead of the content overflowing the container. But it’s still an overflow, so you can pick whichever you like.

But at that size (with the settings I posted) it’s fine if the page has some overflow. Below 320px it’s not uncommon to have a bit of overflow.

I might suggest changing the padding on the body to use the vw unit (viewport) or adding a media query to lower the padding at smaller screen sizes.

[jinwithwings]

@lasjorg @Philip_B I appreciate the additional suggestions!