I'd really really love some feedback

Code Feedback
1

Hi, everyone.
Just finished working on my tribute page.
And I’d really love to have you guys peruse the project and hopefully give me some feedback on things I could have done better:

https://codepen.io/adekunleadedokun/pen/qBOvYVy

2

CSS suggestions

attention to detail with your tags. (ie body{ vs body {

avoid text values with css rules. (ie font-weight: bolder) use a number

some of your css rules have 0 spaces, some have 1, some have line breaks some do not.

avoid global css rules (you have p tag which affects ALL p tags. Give those p tags a class name and use that instead.)

avoid ids use classes instead
Stick to one font type, avoid using more than 2. I think you have like 4 different ones.
Make the bullets easier to read. It’s kinda hard to make out what you’re trying to tell us on that wood background. If information requires a reader any extra work to read, they wont do it.

HTML suggestions

Never use <br> use css instead for spacing
Move the text to json and out of the html
Your href in your h3 is wrong: https://https:// is twice

1 Like
3

Thanks
How do you move your text to json?

4

Well,. you wouldn’t have to make it JSON, but one step away from it so that you could easily store the text into a database and display it onto your page.

An example would look like this in Javascript:

const data = {
  timeline: [
    'Jarad Anthony Higgins (December 2, 1998 – December 8, 2019), known professionally as Juice Wrld (pronounced "juice world"; stylized as Juice WRLD), was an American rapper, singer, and songwriter, born in Chicago, Illinois.',
    'His song <em>"Lucid Dreams"</em> has been played on the music streaming platform Spotify over one billion times. <em>"Lucid Dreams"</em> and his earlier hit single <em>"All Girls Are the Same"</em> helped him gain a recording contract with Lil Bibby\'s Grade A Productions and Interscope Records.',
    '<em>"All Girls Are the Same"</em> and <em>"Lucid Dreams"</em> were two of five singles that were included in Higgins\' debut studio album <b>Goodbye & Good Riddance</b>, which went on to become certified Platinum by the Recording Industry Association of America (RIAA) on December 5, 2018. The album enjoyed positive critical reception, and contained three other singles: <em>"Armed and Dangerous"</em>,<em>"Lean wit Me"</em> and <em>"Wasted"</em>, all of which charted on the Billboard Hot 100.'
  ]
};

const init = () => {
  // builds lis
  const $lis = data.timeline.map((bullet) => {
    const $li = document.createElement('li');
  	 $li.classList.add('bullet');
     $li.innerHTML = bullet;
    
     return $li;
  });
  
  // attaches lis to ul
  const $ul = document.querySelector('ul.timeline');
  $ul.append(...$lis);
};

init();

Do you see how much easier it is to read? And plus it would be easier to see the format of your html structure. You always want to try and seperate data from the code.

1 Like
5

And the html part would be like this:

<div class="example">
  <ul class="timeline"></ul>
</div>

See how the lis are no longer there?

6

yeah, I get it
less code, more functionality

7
  • Technically, the black text on purple background does have enough contrast (at least for AA specs) but personally I still find it hard to read. I think the purple could be lightened up a little more (maybe consider going for AAA)
    https://webaim.org/resources/contrastchecker/
  • I think you can make the video much more responsive. Don’t set the width/height in the iframe tag. And then use CSS to make it scale responsively. For example, using Firefox, click the Picture-in-Picture icon to make it pop-out in a new window. You can see that the video can be much bigger than what you have it now, and the height/width dimensions are much different too.
  • I would put a max-width on the tribute-info section. Currently the line length will keep growing as wide as I can make my browser (and I can make it very wide). A lot of people find content hard to read when the line length is too wide.
  • The h3 heading at the bottom isn’t actually a heading, so it should be changed to a <p>.
  • The link text in that last sentence isn’t very descriptive. It should include his name. Consider changing it to something like “Check out The Young King’s Wikipedia entry to find out more about his life.” and “The Young King’s Wikipedia entry” would be the link text.
1 Like
8

@kerafyrm, this is out of scope for what user’s have been taught here thus far. They’ve just learned HTML and CSS. These projects are to get them to build something using what they’ve learned.