Need feedback for the tribute page

Project Feedback
#1

I just completed my first project.
This is my first time coding on my own (without hints from fcc) so i need your feedback on it
here’s the link : https://codepen.io/durden_20/pen/bGwYgKj

I think I’ve used some code in many place which isn’t necessary anyway if you can find any mistakes , suggestions or want me to make any improvements i would love to hear it.

Thank you.

#2

Cool, it looks pretty good.

A few thoughts…

I don’t know if I would have underlined the title (and others) - that’s part of the “language of the internet” that means “clickable”. I think the size and weight are enough to make it important.

		<blockquote id="quotation" cite="http://www.worldwildlife.org/who/index.html">
			<p>
				<b>"what i do is temporary,<br>
					but what i leave behind is<br>
					forever"<br>
					-Mark Fischbach</b>
			</p>
		</blockquote>

Yeah, I think the p and the b elements shouldn’t be there - whatever styling you want should be done with CSS and applied to all block quotes, that id, or a class.

		<u> Biography </u>

It’s a minor thing, but this always annoys me. Don’t add extra spaces in your elements like that. The whitespace in front of the element (indenting) is fine because it doesn’t count, but inside an element that is rendering text, it gets rendered to the screen. Many is the time I’ve had to spend time trying to figure out why something isn’t centered or aligned properly just to find out that it was a junior dev that did something like this. If you sincerely want those extra spaces around “Biography”, then that’s different. (In that case it could also be accomplished with CSS.) I only see you doing it on your h2s, but still, break yourself of this habit.

	border: solid 5px;
	border-color: grey;

Most people would shorten that to border: solid 5px grey;

But still, it looks good. Have fun on the next project.

1 Like
#3

Your page looks good @geeth9792. Some things to revisit;

  • Do not use the <br> element to force line breaks or spacing. That’s what CSS is for.
  • Accessibility is about being accessible to all users. Review the lesson about giving meaningful text to links.
1 Like
#4

Thanks for the feedback

I tried using margin for spacing instead of br it worked but when i resized the site, it changes the point of line break. so instead of using br i used p element is that fine?

and i don’t understand what wrong I’ve done in other things you’ve mentioned like:-

I think i chose correct word for link (Wikipedia entry) .

what do you mean by not accessible? Are you saying you can’t load the site?
because i used the correct link, and it works fine for me.
Thanks for replying.

#5

Thanks for the tips.

I’ve corrected most of the things.
And no extra white spaces … got it (To be honest i don’t know where it came from i didn’t add them.) :slight_smile:

#6

Yes that’s fine. Also, instead of using <br> elements to have each inline element on a new line, you can use or set container elements to be block-level elements so they’ll each take up the full width.

There is still a stray <br> element on line 28.

That’s why I provided the links for you to read. If you read them you will understand.
In short, screen readers do not read the entire page. Someone relying on a screen reader will not understand “wikipedia entry” because there’s not context.

1 Like
#7

Hey, congratulations, it looks great!! I really like the colors and the rounded corners – it feels very inviting.

One thing I would change – the Open Sans and Balsamiq fonts are nice together, but the body font (for me, it shows up as Times New Roman) is a bit jarring. I think continuing with Open Sans would be nice and would tie the page together better.

Actually it looks like you have some typos in the css that is causing default fonts to appear (“cusrsive” and “sans serif” missing the hyphen). However, using these generally font families still leaves the browser free to pick the exact font, which gives you less control. Also, my browser defaults on cursive to Comic Sans, which really shouldn’t be used by anyone outside of school teachers IMO!

But overall, I thought it looked really good, especially for a first project! Nice going!

1 Like
#8

Overall a very nice job. Just a few things:

  • For the blockquote, don’t use <p> to make new lines. You can use <span>s and set the display to block. But using <p> here is not semantically correct.
  • You’ve got a max-width on the list, which is good. And it is set in em units, which is awesome. Now do the same to the biography content. You also might want to think about doing that for the figure too.
  • Speaking of figures, the caption tag is <figcaption>. For the copyright information, use the <small> tag:
    https://developer.mozilla.org/en-US/docs/Web/HTML/Element/small
    I would probably place this information inside the <figcaption>.
  • Usually the <header> and <footer> are outside of <main>. In this case, I don’t even think you need a <header> since you aren’t using it for anything other than the <h1>. You are using the <footer> for traditional footer stuff so keep that, just move it out of <main>.
  • The <h3> at the bottom is a little wordy and probably not semantically correct. Personally, I would get rid of the last two <section> tags (you aren’t using <section> for Biography or Timeline so I’m not sure it makes sense to start using it here) and then add another <h2> heading above the blockquote so that each “section” has its own <h2>.
#9

Can you help me with one more thing,
these fonts are working fine on my PC but not in mobile devices
can you tell me if there’s a mistake in my code? i can’t find any.

#10

I don’t see an issue. What makes you think that fonts aren’t working with mobile devices?

#11