Feedback on my "tribute page"

Hey guys,
I just started coding and did a ‘‘tribute page’’ for Michael Jordan (in the hype of the Last Dance documentary).

I would like some inputs on the overall structure of the coding and if it works.

https://codepen.io/vzamith/full/eYpbXYp

1 Like

So you’ll want to use CSS media query break points to make your page more responsive. When I narrow my browser window the lists still stay in the two column format. At some point they need to covert to a single column format. I also get some overlap with the header at the top of the page. The images at the bottom on the other hand are rearranging for a narrower view port. Maybe instead of going directly from one row with four columns to one column with four rows you throw in 2x2 format as well?

Okay, makes sense. I will try do that and refresh it in Codepen.

@bbsmooth Do you mind checking it again? I did some alterations to it, I think now it’s working better.

When get smaller I just hide it the subtitle of the 1st image, otherwise gets too crowded.

Very nicely done. Responds very well to changes in browser width. Now I am going to challenge you to respond just as nicely to changes in text size. If you don’t know how to change 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 make the text size larger. You’ll see right away that the right side of the page gets cutoff and there is no horizontal menu bar so you can’t scroll over to see the missing content.

The way to deal with this is to use ‘em’ instead of ‘px’ for your break points. Right now you have break points at 800px and 600px. To ballpark the em units, divide each by 16 (that assumes a default font size of 16px). So change 800px to 50em and 600px to 37.5em. Then while keeping your browser width the same try increasing/decreasing the font size to see what happens.

Great Page! I have a couple questions if you do not mind, please.
Have you run the FCC tester for the page? If so did you pass all 10?
Why am I asking, cause if you did, then I misunderstood some of the instructions for the project - - which would be great to be honest!
One note, not sure if it is effecting your results are not but in your CSS line 86 you misspelled “transform” :slight_smile:

Again super job on the page! :slight_smile:

Thanks for the feedback! I’m familiar with the zoom (I use Chrome but it works similarly).

One question is, to be also responsive to change in font size, should I merely change the Media Querie (from px to em ) or created a new one that functions around font-size?

I just upload it on the FCC Challenge, apparently went through (it got the check it in the /learn tab). I believe that as long as you meet the User Stories it should work.
From that you can spin it how you like it. I did like this to play a bit with flexboxes and responsive layout.

Ps. Thanks for notice on the typo of the CSS. :slight_smile:

1 Like

Yes, just change the existing break points from px to em. Setting your breakpoints in em will automatically make them responsive to both changes in the browser width and font size changes. You get two for the price of one.

2 Likes

Not sure if it was just my scrolling/loading before or not, but it looks like you now have a black background below the title area, which looks much cleaner than it did before, also allows faster and easier reading :slight_smile: which means I noticed something…MJ attended UNC-CH or University of North Carolina at Chapel Hill (you do not have to include the at Chapel Hill as that is the main campus and is referred to as UNC :slight_smile: ) to my knowledge there is no North Carolina College .

Yes, I am a proofreader :slight_smile:

1 Like

The black background on the title was probably due to the loading, but now I’ve inserted and I think I prefer like that.

And I’ve corrected the “North Carolina College”, thanks :slightly_smiling_face:

1 Like

Yes, it worked. I’ve used the 16 rate conversion from px to em, seems to have worked fine. Thanks for the tip and the time to help me.

Looks real good. Isn’t that nice, now your page is automatically responsive no matter what the view port width or text size.

A few other issues:

  • Don’t use ‘vw’ (of ‘vh’) for font size. Doing this makes the size of the text solely dependent upon the view port dimensions. It takes away the ability of the user to control the text size.
  • I think your heading levels could be fixed up a little. All of those h4’s should probably be h2’s. The current h2 and h3 should not be headings at all, or you could add “The greatest to ever play the game” to the h1.
  • The alt text for the four images at the bottom is the same for every image. I’m guessing you just forgot to customize them.
  • The blue link at the bottom is too hard to read on the black text, it definitely needs to be a lighter color. You can check color contrast accessibility at
    https://webaim.org/resources/contrastchecker/
  • That link is also not quite as descriptive as it could be. Maybe change the sentence to something like
"Visit <a>Michael Jordan's Wikipedia page</a> to learn more about his career.

I agree with those. Regarding the headings I was making the mistake of using the headings tag to also give bigger font-sizes, which I’ve read is a mistake, since headings have a purpose. I’ve created class to them instead and left only h1 and h2.

For the responsive text I found a solution that mixed em + vw, so you can set a minimum for the text size and still make it variable with the vw size. I believe it worked.

Thank you for the helpful tips!

Welcome to the forums @victor.zamith. Your page looks good. 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>).
    • Your page passes 5/10 user stories. Click the red button to see which test(s) are failing and text to help you correct the issue.
    • The test script should be included, with all tests passing, when you submit your projects.
  • 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 coding errors you should address.
  • Review the lesson about giving meaningful text to links.

Edit: On a side note, don’t include the style tags in CSS when using codepen. It will cause issues.

2 Likes

Thank you for the welcome. Since is my first project I missed out on the script to test the code. Now runs 10/10.

I’ve also checked the bugs in W3C, fixed them all. Just in the CSS it shows me some Parse error, but I believe is from copying to Codepen. Is that right?

Changed the link, it wasn’t acessible, also included some :hover and etc, to make it even more accesible.

One final question, I’ve already upload the codepen link (which is still the same) into the FCC, should I redo it? Or is gonna work fine?

Thanks.

No. I believe you need calc and the expression in parens. Go ahead and Google it to be sure.

It’ll be fine as long as you updated the same pen. If you created another pen then you’d have to resubmit the link.

1 Like

:point_up_2: this is what I was originally asking you about @victor.zamith with the testing …

@Roma where in our code should the test script be placed? I have not tested my projected yet with the script…I have run it through W3C validator and it passes, however FF accessibility does not like my text/background contrast so I am working on that currently :slight_smile:

Thanks :slight_smile:

1 Like

If using codepen the script element can be the first line in the HTML section.
If you’re coding elsewhere that requires you to create the HTML boilerplate then the test script (being JS) would go right before the closing body tag.

2 Likes

Thanks everyone for the feedback and help, I’ve been learning a lot those few days.

I did finish my Form page, in case you are interest in checking it out.