FCC Tribute Page - Feedback

Code Feedback
1

Hi all,

I’ve just finished my first project for freecodecamp and I’m looking for some feedback mainly around whether the code structure is correct and general aesthetics of the page.

I really feel like I’ve been able to learn quite a lot while doing this even though its the smallest project, its been fun :+1:

Thanks

https://codepen.io/andy-robertson/full/OqXGQL

2 Likes
2

Looks really good @Andy-R! There are a couple of changes I might suggest, but these are just my opinion. I would remove the top margin on your h1 tag and I would remove all margin on your body. That will push your header all the way to the top of the page and will remove the spacing around the edge of your page.

Also, if you replace the pen with full in your url it will open up without the code boxes open. so this: https://codepen.io/andy-robertson/full/OqXGQL instead of https://codepen.io/andy-robertson/pen/OqXGQL

But overall you did a great job. keep up the great work!

3 Likes
3

I made the changes and added a little padding to both the header and footer, it was strange as I hadn’t set a margin in the body but I was able to just use margin: 0; and set it to zero. You were right though it does look better, thanks for the advice!

1 Like
4

@Andy-R, nice looking page. Good job!

One thing, run your CSS code through a validator. codepen provides you with one for HTML, CSS and JS. Just click on the arrow in the upper right of the section and then click on the respective ‘Analyze’ link.
You have a duplicate assignment in CSS

3 Likes
5

Well spotted, thanks for the feedback :sunglasses:

2 Likes
6

@Andy-R

Beautiful professional looking project. Nice font selection.

The one thing I noticed is that the paragraph text shrinks too small in mobile view. It is hard to read at that point.
Oh, and increase the height of the footer (just my opinion).

2 Likes
7

Awesome project. I really loved the box shadow effect and the image of Albert Einstein. But can you increase the font size for mobile devices. I am using an iPhone 6 and it’s kinda hard to read . I had to zoom in to read the content and slide to the right to read the rest of the content.

2 Likes
8

Thanks for all the feedback!

I smoothed out and resized all the media query breakpoints for fonts so it should be much easier to read now at all screen sizes (hopefully), and I also added a little more padding to the footer :wink:

3 Likes
9

Love the design and for a tribute page it’s great, I really like it :slight_smile:

I’m new to HTML and CSS myself, so I won’t comment on the code, but there is a spelling mistake near the bottom:

  • He published over 300 academic papers and 150 non-scientific worlks

Keep up the great work :smiley: :+1:

2 Likes
10

Thanks for the feedback, damn typo, all fixed now. I also made a few more tweaks inside the media queries section for mobile devices :+1:

1 Like
11

I have these suggestions:

This will display a bigger images on smaller screens. Use max-width to stop it at the width you want.
For the image:

#image {
/*   max-width: 70%; */
  width: 90%;
  max-width: 1024px;
  display: block;
  height: auto;
  margin: auto;
  border-color: #514d3f;
  border-width: 3px;
  border-style: solid;
  padding: 2px;
  box-shadow: 0 10px 20px;
}

You set a lot of sizes in pixels. Setting pixel is not the greatest idea. It is better to use rem because pixels do not change in size.
For the width of the paragraphs:

#tribute-info {
  font-family: Cinzel, arial;
  font-size: 2rem;
  color: #514d3f;
/*   padding-left: 150px;
  padding-right: 150px; */
  padding: 0 .5rem;
  text-align: justify;
  text-justify: inter-word;
}

In your media queries you and using min-width and max-width together. In my experience you only need to use min-width if you are starting at the mobile size and max-width is for when you start your design at desktop view and want to shrink your page smaller.

Mobile first media query example:

@media (min-width: 400px) {
  /* your code */
}

@media (min-width: 600px) {
  /* your code */
}

@media (min-width: 1000px) {
  /* your code */
}

Going from desktop to mobile is like this:

@media (max-width: 1000px) {
  /* your code */
}

@media (max-width: 600px) {
  /* your code */
}

@media (max-width: 400px) {
  /* your code */
}
3 Likes
12

That’s a neat trick for images, after reading this I actually used it around the other way also with min-width on the signature image and it’s now perfectly readable on mobile.

I hadn’t thought through px vs rem (and em) so did some additional reading following this I went through and converted the page over.

The media query caught me out at first because after removing max-width from them only the 1000px shift worked until I realized the order they appeared in the CSS now mattered, so after a quick switch around it all worked as before.

A very informative post Brandon, thanks for taking the time to put it together :+1:

1 Like
13

Hi,

Great page man, it looks great! :clap:

A little tip about using Codepen for these projects. Everything that would normally go in the head section (if you had content in the head section that is :stuck_out_tongue_winking_eye:) doesn’t actually go in to HTML editor when you use Codepen. Instead go to Settings > HTML > and you will see Stuff for head . You don’t need to declare body tags either, think of the HTML editor as being the body of the HTML page. Hope that made sense.

Looking forward to seeing your next project :+1:

3 Likes
14

Thanks for pointing that out, I removed the redundant tags :wink:

1 Like
15

Man, don’t use caps for big text, it’s at least not the best practice. You can read here more about it: https://uxmovement.com/content/all-caps-hard-for-users-to-read/

Einstein image and overall design is good!

1 Like
16

Unfortunately, that is the regular Cinzel font as there is no lower case version but I will keep this in mind going forward. Thanks for the feedback :+1: